Skip to content

With typescript: true the __svelte_meta.loc reports invalid location because script tag is changing number of lines/columns #8360

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

Closed
FluffyDiscord opened this issue Mar 6, 2023 · 9 comments · Fixed by #8778

Comments

@FluffyDiscord
Copy link

FluffyDiscord commented Mar 6, 2023

Describe the bug
Enabling typescript preprocess breaks __svelte_meta.loc reporting on elements, since the script tag is being processed and the amount of lines and columns no longer relates to the original file.

Unfortunately this renders any debugging tools useless.

Logs
No error logs

To Reproduce
I have prepared two identical components, one written with lang="ts", the other one in plain JS.

  1. Clone https://github.com/FluffyDiscord/svelte-preprocess-offset-bug
  2. Run yarn install or npm install
  3. Run yarn dev or npm run dev
  4. Open browser at http://localhost:5173/ or the specified Vite url found in console
  5. Left click on the svelte inspector helper in top right corner
  6. Hover over shifted location element, it will report that the span is on the sixth line, which is wrong
  7. Now hover over correct location element, it will report that the span is on the seventh line, which is correct

Invalid reported position, Svelte thinks the span starts at line 6 column 5, because of the preprocess.
image

Correctly reported position of the span at line 7, column 5.
image

In this simple example case the difference is just a single line. In larger Svelte components with longer script tags with more TS definitions the difference can be as much as few dozen lines and columns (function parameter types, interface/type definitions, inline variable/property types).

Expected behavior
The preprocess does not shift or change the line or column size. It should fill up the removed lines with empty line character \n and fill up removed columns with empty space character or JS comments /* */ in DEV mode. This is not required for PROD and maybe even preferable to be that way to confuse lurkers :)

Stacktraces
None needed

Information about your project:
None needed

Additional context
This might also apply to other tags like style or module (not tested, since I don't use module and I put style at the bottom)
There is a workaround and that is to put script tag at the bottom of the svelte file. But that goes against the Svelte idea. I don't want to have move hundreds of script tags in my files down to bottom, it should not be required to do so.

@FluffyDiscord FluffyDiscord changed the title With typescript: true the __svelte_meta.loc because script tag is changing number of lines/columns With typescript: true the __svelte_meta.loc reports invalid location because script tag is changing number of lines/columns Mar 6, 2023
@dummdidumm dummdidumm transferred this issue from sveltejs/svelte-preprocess Mar 8, 2023
@dummdidumm
Copy link
Member

Mhm that's an interesting issue for which I'm not sure if the fix should be on the tooling side (like the inspector) or built into Svelte. There theoretically could be other tools depending on the current position pointing to the preprocessed Svelte version, which would mean we need another field. Transfering this to the Svelte repo since and cc'ing @dominikg because of Svelte inspector.

@dominikg
Copy link
Member

dominikg commented Mar 8, 2023

Not exactly sure what inspector could do here, it can only work with the metadata attached to the elements.

The output sourcemap of svelte should always work all the way back to the input, not the preprocessed version of the input.
For that reason preprocessors should emit sourcemaps for their transforms and when svelte receives an input sourcemap, it merges them. vite-plugin-svelte does pass the input sourcemap, so it looks like either svelte-preprocess emits the wrong sourcemap or the merge algorithm inside svelte isn't working as intended.

@dummdidumm
Copy link
Member

dummdidumm commented Mar 8, 2023

Another potential culprit (haven't checked) would be __svelte_meta.loc not taking into account source maps at all.

@dominikg
Copy link
Member

dominikg commented Mar 8, 2023

looks like vitePreprocess has the same "off by one line" problem. It might be the initial newline after the opening script tag not being accounted for 🤔

@dominikg
Copy link
Member

dominikg commented Mar 8, 2023

adding an interface to the script block

    interface Foo {
        bar: string;
    }

that gets removed by the preprocessor leaves the offset at the same 1 line difference, so loc does take at least some of it into account.

@FluffyDiscord
Copy link
Author

FluffyDiscord commented Mar 8, 2023

adding an interface to the script block

    interface Foo {
        bar: string;
    }

that gets removed by the preprocessor leaves the offset at the same 1 line difference, so loc does take at least some of it into account.

Larger svelte TS components have bigger line and column differences. The "one line off" is just a coincidence in this very simple example case.

Edit: updated the issue to mention it

@dominikg
Copy link
Member

dominikg commented Mar 8, 2023

it looks like reducing a svelte+typescript component to one line still puts the template at line 2

<script lang="ts">let type:string="TS"</script><div><span>{type}</span></div>

puts the span in meta.loc at line 2 col 15

@dominikg
Copy link
Member

dominikg commented Mar 8, 2023

svelte-inspector does modify loc to add 1 to line and col, this exists from the very beginning of it sveltejs/vite-plugin-svelte@2777585#diff-30de01dc2bdc94cb6f848bf28a3971204dd2349a77fbdea0c093aa1844992567R40

don't excactly remember why that was added, wild speculation has me accounting for the cause of this error accidentally by trying to go from 0 based to 1 based counts.

If we remove that, it becomes an off by two problem for the presented case.

Using vite-plugin-svelte's advanced raw import query, here is a stackblitz that shows source, preprocessed source and output js after the rendered component.

Note the add_location calls in the output js, those create the __svelte_meta props. You can edit lib/TS.svelte to see them change. There's definetely more going on than just inspector adding a line inadvertedly.

https://stackblitz.com/edit/vitejs-vite-stxval?file=svelte.config.js,src%2FApp.svelte,src%2Fmain.ts,src%2Flib%2FTSComponent.svelte&terminal=dev

@dominikg
Copy link
Member

dominikg commented Mar 8, 2023

looks like @dummdidumm is correct. the meta.loc is obtained from position in preprocessed source, not accounting for input sourcemaps at all.
via

this.locate = getLocator(this.source, { offsetLine: 1 });

getLocator is from https://github.com/Rich-Harris/locate-character

dummdidumm added a commit that referenced this issue Jun 21, 2023
We need to use a different method for getting the meta info because `locate` is used to help construct the source map that references the preprocessed Svelte file. If we would now add source maps to that `locate` function it would go the the original source directly which means skipping potentially intermediate source maps which we would need in other situations.

fixes #8360
closes #8362
dummdidumm added a commit that referenced this issue Jun 22, 2023
We need to use a different method for getting the meta info because `locate` is used to help construct the source map that references the preprocessed Svelte file. If we would now add source maps to that `locate` function it would go the the original source directly which means skipping potentially intermediate source maps which we would need in other situations. Sadly we can't map the character offset because for that we would need to the original source contents which we don't have in this context.

fixes #8360
closes #8362
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants