Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

line numbers inaccurate #83

Closed
jeberly opened this issue Nov 3, 2021 · 6 comments · Fixed by #153
Closed

line numbers inaccurate #83

jeberly opened this issue Nov 3, 2021 · 6 comments · Fixed by #153
Labels
bug Something isn't working
Milestone

Comments

@jeberly
Copy link

jeberly commented Nov 3, 2021

Describe the bug
We have a fairly large production Svelte app and was trying to migrate off rollup to esbuild. Thanks for your plugin. We are running into an issue I was hoping you can give some hints too. We are running esbuild with watch and I am creating fake errors to test the error output so we can hook up an overlay screen. The issue I am running into, is the line numbers for svelte errors are off or strange.

  1. error in js
{"errors":[{"location":{"column":17,"file":"src/components/AppRoot.svelte","length":15,"line":79,"lineText":"import Left from \"./Left.svelte\";","namespace":"","suggestion":""},"notes":[],"pluginName":"esbuild-svelte","text":"Transform failed with 1 error:\nsrc/components/Left.svelte:321:7: error: Expected identifier but found \".\""}],"warnings":[]}

AppRoot.svelte imports Left.svelte and the error i introduced does exist at Left.svelte:321:7 but it is in the "text" area, not the "location"

  1. error in markup. An error I introduce in markup section at line 404, shows this output. The error line reported is 323, which isn't close to actual error location
{"errors":[{"location":{"column":10,"file":"src/components/Left.svelte","length":0,"line":323,"lineText":"321:           {room.name}\n322:         </a>\n323:         {##if roomID === room.id}\n               ^\n324:           <div\n325:             class=\"nav-action\"","namespace":"file","suggestion":""},"notes":[{"location":{"column":17,"file":"src/components/AppRoot.svelte","length":15,"line":79,"lineText":"import Left from \"./Left.svelte\";","namespace":"","suggestion":""},"text":"The plugin \"esbuild-svelte\" was triggered by this import"}],"pluginName":"esbuild-svelte","text":"Expected if, each or await"}],"warnings":[]}
  1. An error in css. Actual error is at line 625, output incorrectly shows 544
{"errors":[{"location":{"column":3,"file":"src/components/Left.svelte","length":0,"line":544,"lineText":"542: \n543: <style>\n544:   ..account-alert {\n        ^\n545:     background-color: var(--rml-warning-bg);\n546:     padding: var(--rml-padding-default);","namespace":"file","suggestion":""},"notes":[{"location":{"column":17,"file":"src/components/AppRoot.svelte","length":15,"line":79,"lineText":"import Left from \"./Left.svelte\";","namespace":"","suggestion":""},"text":"The plugin \"esbuild-svelte\" was triggered by this import"}],"pluginName":"esbuild-svelte","text":"Identifier is expected"}],"warnings":[]}

To Reproduce
Unfortunately I can't recreate this issue with a blank Svelte app.

Here is my build script though in case it helps.

const result = await esbuild
  .build({
    define: {
      __env_vars__: cfg,
    },
    entryPoints: ['src/app.js', 'src/room.ts'],
    bundle: true,
    outdir: outDir,
    assetNames: 'assets/[name]-[hash]',
    logLevel: 'debug',
    minify: false,
    sourcemap: true,
    // splitting: true,
    watch: {
      onRebuild(error) {
        clients.forEach((res) =>
          res.write(
            'data: ' +
              JSON.stringify({ update: true, error: error, path: appPath }) +
              '\n\n',
          ),
        );
        clients.length = 0;
        console.log(error ? JSON.stringify(error) : '...success...');
      },
    },
    format: 'esm',
    target: 'es2017',
    loader: {
      '.svg': 'text',
    },
    // metafile: true,
    plugins: [
      esbuildSvelte({
        preprocess: [
          typescript({
            sourcemap: true,
          }),
          sveltePreprocess({
            sourceMap: true,
            typescript: false,
          }),
        ],
      }),
    ],
  })
  .catch(() => process.exit(1));

Expected behavior
Location data would always be present in esbuild output on failure and point to correct line in file.

Environment (please complete the following information):

"esbuild": "^0.13.12",
"esbuild-svelte": "^0.5.7",
  • Svelte preprocessors used (if any):
    "svelte-preprocess": "^4.9.8",
    "svelte-preprocess-esbuild": "^2.0.0"

Thanks in advance!

@jeberly jeberly added the bug Something isn't working label Nov 3, 2021
@EMH333
Copy link
Owner

EMH333 commented Nov 4, 2021

Interesting! Thanks for the detailed report. I'm adding my preliminary reply here but potentially expect more this weekend (the rest of this week is very busy for me).

My initial thoughts would be that esbuild/svelte/esbuild-svelte is reporting the error location after it does preprocessing (for typescript etc.), so that screws up the line numbers?

Not sure how that works with your js example error though.

Rough example (haven't tested, and doesn't include imports/lang declaration)

<script>
interface LabeledValue {
  label: string;
}
 
function printLabel(labeledObj: LabeledValue) {
  console.log(labeledObj.label);
}

onMount(()=>printLabel({label: "Hello world"}));
</script>

<div> whatever, lets just say error here </div>

If the interface code gets removed during preprocessing then that shifts all the line numbers up by 3? Then when an error is found during actual compilation then it uses the incorrect numbers. It would surprise me if Svelte developers hadn't handled that though...

Also possible that my recent preprocessor sourcemap change screwed things up somehow, maybe try esbuild-svelte: v0.5.6?

I'll let you know if I find the problem or think of anything else

@jeberly
Copy link
Author

jeberly commented Nov 4, 2021

Thanks for the quick reply. I just did some more tests.

  1. by downgrading to esbuild-svelte: v0.5.6 line numbers still off
  2. removed svelte-preprocess-esbuild and let sveltePreprocess do ts transpilation. Numbers reported were very low like line 3 instead of line 250

I wish I could reproduce this issue in a way I could easily share.

EDIT: perhaps it is related to evanw/esbuild#604 and https://github.com/evanw/esbuild/blob/4e25a16bbcf77a6f6714b09fa9401b5029ff00bd/CHANGELOG.md#unreleased

You must not be using esbuild as a bundler. When bundling, esbuild needs to assume that it's not seeing a partial file because the bundling process requires renaming symbols to avoid cross-file name collisions.

EDIT2: I opened an issue here, not sure if the response is new information to you, but thought I would post here evanw/esbuild#1746

@EMH333

This comment was marked as outdated.

hyrious added a commit to hyrious/esbuild-plugin-svelte that referenced this issue Apr 21, 2022
Still don't know what happened here, the esbuild's example plugin --
https://esbuild.github.io/plugins/#svelte-plugin
does have `start.line - 1` to get the lineText, but that was
wrong in my current test.

Maybe related:
EMH333/esbuild-svelte#83
hyrious added a commit to hyrious/esbuild-plugin-svelte that referenced this issue Apr 21, 2022
Still don't know what happened here, the esbuild's example plugin --
https://esbuild.github.io/plugins/#svelte-plugin
does have `start.line - 1` to get the lineText, but that was
wrong in my current test.

Maybe related:
EMH333/esbuild-svelte#83
@hyrious
Copy link

hyrious commented Apr 21, 2022

I guess this was definitely a hidden issue in svelte itself…

My fresh commit: hyrious/esbuild-plugin-svelte@5b88066

@EMH333
Copy link
Owner

EMH333 commented Dec 2, 2022

After much delay, I think I'm able to recreate this and have added a test in 3617764. My original prediction was correct, but it looks like this is squarely on this plugin to handle, not Svelte.

The solution could get complex because it needs to undo the work done by the preprocessors to find the original line of source. This likely involves some sourcemap work so I'll need to figure out the best way to approach this

Update: The svelte language-server handles it properly but includes several things specific to VSCode that I would need to strip out. I need to investigate what other bundler plugins do and if they even handle this properly at all.

@EMH333
Copy link
Owner

EMH333 commented Sep 5, 2023

This has been released as a part of v0.8.0. Thank you for your patience! Please open new issues if you discover any bugs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants