Sourcemaps offset #286

Maggi64 · 2020-12-05T16:54:48Z

Describe the bug
In my current project and in the example repo below the sourcemaps are offset by a few lines.
This seems to be related to the style tag:

To Reproduce

clone repo (https://github.com/Maggi64/sapper-sourcemap-offset)
npm run dev
look at chrome dev tools

Expected behavior
Correctly linked sourcemaps. Otherwise debugging issues becomes quite hard.

Information about your project:

Your browser and the version: (Chrome 87)
Your operating system: (Windows 10)
Rollup & svelte-preprocess are updated to the newest versions (see repo package.json)
All other packages used are also on the newest version

I hope this is the correct place for this issue. Help and suggestions where that problem exactly comes from are appreciated.

Edit: Even without scss preprocessing this issue still persists. Adjusted example repo.

The text was updated successfully, but these errors were encountered:

benmccann · 2020-12-07T17:42:11Z

@milahu I know you had done some investigation into this in sveltejs/svelte#5584 (comment). Not sure if it's something you'll be able to take a look at, but if so I'd be happy to help review

milahu · 2020-12-07T18:28:47Z

thanks for the bug : )

a minimal reproduction is: add this to every svelte and js file

console.log('this is file.xyz line 10'); // TODO keep line number in sync

and then look at the JS console

with svelte-preprocess on

with svelte-preprocess off

same result with older versions of svelte, 3.0.1 for example

still not sure where exactly the problem is ..
from the sapper template i could not reproduce yet

@dummdidumm do you know more?
ben said you saw the off-by-one bug too

dummdidumm · 2020-12-07T19:09:43Z

No I don't know more, but I think it's inside Svelte because inside language-tools we are able to map the warnings correctly.

milahu · 2020-12-07T20:30:27Z

small bug in svelte-preprocess:

the regex for <template>.*</template> is too greedy
so the input <template>T1</template><template>T2</template>
is wrongly replaced with T1</template><template>T2

these work as expected in svelte:

	async function preprocess_tag_content(tag_name: 'style' | 'script', preprocessor: Preprocessor) {
		const get_location = getLocator(source);
		const tag_regex = tag_name === 'style'
			? /<!--[^]*?-->|<style(\s[^]*?)?(?:>([^]*?)<\/style>|\/>)/gi
			: /<!--[^]*?-->|<script(\s[^]*?)?(?:>([^]*?)<\/script>|\/>)/gi;

also, <template src="./input.template.html"></template> is simply removed
i expect the contents of input.template.html should be injected?

milahu · 2020-12-08T00:02:12Z

edit: nope

possible cause:

<!-- This file is generated by Sapper — do not edit it! -->

is added on top of every *.svelte file that is passed to svelte.compile

edit: no, this comes from node_modules/@sapper/internal/App.svelte

to debug, i patched node_modules/svelte/compiler.js like

 	function compile(source, options = {}) {

+console.log('compile source >>>>'+source+'<<<<');

injecting that comment without changing lines / columns
and without creating syntax errors is not trivial .. simply remove?
maybe use filenames like component.autogenerated.svelte

good night ; )

milahu · 2020-12-08T08:43:54Z

new suspect:
in svelte.preprocess, this

<style>
    h2 {
        font-size: 15px;
    }
</style>

becomes this

<style>
    h2 {
        font-size: 15px;
    }

/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW....4iXX0= */</style>

before the /*# there should be no newline

backtrace:

svelte-preprocess/dist/autoProcess.js
  calls transformer 'globalStyle'

svelte-preprocess/dist/transformers/globalStyle.js
  postcss_1.default(plugins).process(content

postcss/lib/lazy-result.js
  let map = new MapGenerator(str, this.result.root, this.result.opts)
  let data = map.generate()

postcss/lib/map-generator.js
  let eol = '\n'
  if (this.css.includes('\r\n')) eol = '\r\n'
  this.css += eol + '/*# sourceMappingURL=' + content + ' */'

so .. blame postcss! postcss/postcss#1486

a quickfix could be made with https://github.com/ds300/patch-package

case closed : )

dummdidumm · 2020-12-08T09:20:47Z

This happens for me when I use a TS preprocessor only so that can't be it. Also the PostCSS issue should have been fixed ( #251 ).

milahu · 2020-12-08T09:27:22Z

so that can't be it

should be the same problem

sourcemaps are injected at end-of-file
seems that consumers expect the /*# sourceMap... at start-of-line
so a newline must be added ..
but that change (added line + added columns) is not included in the sourcemap

dummdidumm · 2020-12-08T10:23:45Z

I'm not sure that's true. If I throw an error inside the script, that error has the wrong line. If the sourceMap-stuff is added inline, then it's added to the end of the script tag, so it should not affect things before that.

For reference, here is how language-tools does the sourcemapping:

Find style/script start/end position
Preprocess stuff
Find style/script start/end position in transformed file
Don't merge source maps in to one, keep them seperate

When an original position is requested, find out if it's inside script/style.
If yes:

Find out if the position relative to the generated fragment. For example if script starts at line 2, the relative position is line-2.
Use the sourcemap of the script/style to find the original position
Find out the position relative to the whole original file. For example if the original position within the script is at line 2, and the script starts at line 2, it's 2+2.

milahu · 2020-12-08T21:19:27Z

fixed the postcss bug in postcss/postcss#1487
and guess what .. it works : D

problem was:
appending of the annotation-comment line /*# sourceMappingURL=.... */
was not reflected by the sourcemap
so in the result, only N-minus-one lines were mapped
producing these off-by-one errors

Maggi64 · 2020-12-09T01:14:41Z

@milahu In some cases lines are not offset N-1 lines, it actually seems to depend on the complexity of the style tag

milahu · 2020-12-09T03:05:45Z

postcss adds one 'bad' line for every preprocessor
in your case postcss calls globalStyle and scss preprocessors

an easier solution is sveltejs/svelte#5754
to make svelte more fault-tolerant
rather than fix all possible preprocessors

Maggi64 · 2021-01-02T22:53:49Z

I updated my example repo to the newest version including that sourcemap pr.
The issue still persist sadly.

milahu · 2021-01-03T09:17:38Z

found the problem

the preprocessor returns sourcemap not in result = { code: '...', map: HERE }
but 'inlined' into result.code = attached as 'magic comment line' at end-of-file
in that case result.map == undefined

/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW ... vgICB9XG4iXX0= */

this magic comment is currently ignored by svelte, but its parsed by the browser
proof: when i remove the magic-comment-last-line, then i get correct mappings

solution: in svelte, detect if preprocessor added the magic-comment-line at end-of-file,
then remove that line, and parse sourcemap from there
in theory, multiple sourcemaps can be attached this way

@dmitrage do you wanna fix this?

benmccann · 2021-01-03T13:53:49Z

Or should we fix the preprocessor instead to return the map in code.map? That seems like it might be the cleaner solution than making Svelte core understand non-standard behavior

milahu · 2021-01-03T14:07:08Z

its a de-facto-standard, so id say we make svelte more tolerant. im working on it, the only challenge is parsing multiple attachments, and eventually merging them with result.map, which is ambiguous since the time-order of the maps is unclear --> needs a new config in the preprocessor interface

dummdidumm · 2021-01-03T14:55:36Z

What do you mean by "multiple attachments" and "time-order is unclear"? The order in which script/style/markup is preprocessed is fixed at the moment.

milahu · 2021-01-03T14:58:52Z

a preprocessor can return sourcemaps in result.map or in result.code or in both. when in both, the order is ambiguous, and we need a new flag like result.attachedSourcemapIsLast = true | false. its an edge case, but should be supported

multiple attachments:

var code = 'here';
/*# sourceMappingURL=data:application/json;base64,....map1.... */
/*# sourceMappingURL=data:application/json;base64,....map2.... */

edit: too complicated. now focussing on the simple cases

milahu · 2021-01-03T19:04:00Z

@Maggi64 can you test my patch in https://github.com/milahu/svelte/tree/parse-attached-sourcemap?

edit: done, problem solved : )

dmitrage · 2021-01-03T22:59:30Z

@dmitrage do you wanna fix this?

@milahu sorry, will be almost off in the next few days.

now focussing on the simple cases

I'd vote for keeping this direction.

Edge cases with multiple/mixed inline/external maps are rare enough and preprocessor side has more context of how to deal with them. Adding api sugar for such seems excess and increasing maintenance burden.

In contrast, extracting map from single inline comment with no external returned is unambiguous, easy to implement and sounds common.

CherryDT · 2021-01-04T12:53:16Z

I'm noticing this also with a prependData for my SCSS, it causes errors inside the style block to appear at the wrong line (probably the line inserted by prependData is counted even though it doesn't exist in source):

Here you can see the error for $aaaprimary applied to the unrelated word transform.

Maggi64 · 2021-01-05T11:06:05Z

@milahu thanks for helping me out with this issue. 👍

Tried your branch, it indeed works for the example repo and seemed to fix the sourcemap for most cases.

But when i turn on scss preprocessing it is offset again sadly.

Try this:

Also updated my example repo.

egargan · 2021-01-05T13:07:20Z

Not sure if this will be coming from the same issue (this discussion seems to revolve around the style block), but sourcemaps for TS script blocks also aren't correct, with or without a style block in the same component.

Reproducing is as easy as cloning the template repo, running that scripts/setupTypeScript.js script, npm install, then add a few random lines to the App.svelte's script block. Should be obvious that the lines don't match up.

E.g.

Non-TS block

TS block

milahu · 2021-01-05T14:07:18Z

But when i turn on scss preprocessing it is offset again sadly.

where can i see that? i dont see any sourcemaps at all in the css files (sorry im new to sapper)

not

sourcemaps for TS script blocks also aren't correct ... Reproducing is as easy as cloning the template repo, running that scripts/setupTypeScript.js script ...

when i run npm run build i get the warning

(!) Plugin typescript: @rollup/plugin-typescript: Typescript 'sourceMap' compiler option must be set to generate source maps.

looks like a bug in the template config

edit: not. sourceMap: !production in rollup.config.js enables sourcemaps only for dev builds

Maggi64 · 2021-01-05T14:56:02Z

@milahu I just looked at chrome dev tools again. With the scss preprocessor the lines in the <script> part are offset again.

milahu · 2021-01-15T09:09:41Z

one problem is: sourcemap support in rollup-plugin-svelte is not yet released
workaround: npm i -D https://github.com/sveltejs/rollup-plugin-svelte.git

milahu · 2021-01-18T09:03:44Z

@Maggi64 @CherryDT @egargan

problem solved?

egargan · 2021-01-18T09:23:57Z

Good spot! Installing rollup-plugin-svelte directly from GitHub does make a difference, not a good one though. Using the same example from my last comment, this is what I see now.

I'm fairly sure everything's configured correctly. As I said I'm just using the template repo and running the scripts/setupTypeScript.js script. Sourcemaps for non-TS <script> blocks still work as expected.

milahu · 2021-01-18T14:01:51Z

too long, dont read

looks like svelte-preprocess fails to parse the sourcemap from typescript
only start-tag and close-tag are mapped, but not the content

App.svelte

src 1: <script lang="ts">
src 2: 
src 3: 
src 4: 
src 5: 
src 6: export let name: string;
src 7: 
src 8: 
src 9: 
src 10: console.log('this is line 10' as string);
src 11: 
src 12: 
src 13: </script>

result.code from typescript

src 1: <script lang="ts">export let name;
src 2: console.log('this is line 10');
src 3: </script>

result.map from typescript

1:0 <-- App.svelte 1:0 <script lang="ts">export let name: string;
1:1 <-- App.svelte 1:1 <script lang="ts">export let name: string;
1:7 <-- App.svelte 1:7 <script lang="ts">export let name: string;
1:8 <-- App.svelte 1:8 <script lang="ts">export let name: string;
1:12 <-- App.svelte 1:12 <script lang="ts">export let name: string;
1:13 <-- App.svelte 1:13 <script lang="ts">export let name: string;
1:14 <-- App.svelte 1:14 <script lang="ts">export let name: string;
1:16 <-- App.svelte 1:16 <script lang="ts">export let name: string;
1:17 <-- App.svelte 1:17 <script lang="ts">export let name: string;
3:0 <-- App.svelte 13:0 </script>
3:1 <-- App.svelte 13:1 </script>
3:2 <-- App.svelte 13:2 </script>
3:8 <-- App.svelte 13:8 </script>

= line 2 <-- 10 is not mapped, browser shows line 1 in console = uses previous mapping? 1:17 <-- 1:17

trace:

result.map == undefined and result.code contains no sourcemap

// svelte-preprocess/src/transformers/typescript.ts

  const {
    outputText: code,
    sourceMapText: map,
    diagnostics,
  } = ts.transpileModule(content, {
    fileName: filename,
    compilerOptions,
    reportDiagnostics: options.reportDiagnostics !== false,
    transformers: {
      before: [importTransformer],
    },
  });

here compilerOptions.sourceMap == undefined, default is false
when set to true everything works as expected --> patch #299

Maggi64 · 2021-01-18T15:58:52Z

@milahu Thank you very much, works perfectly. I also tested some more complex configs with autoprefixer & sass prependData.
Lines did map correctly.

Would like to see these PRs merged to fix the issue: @benmccann

Related PRs:

sass: remove sourceMappingURL from result.code #297 (Bug is fixed without using this PR. Don't exactly know what this is for.)

Example Repo is updated with the patched versions.

… for ts (#286) (#299) * fix typescript: make sourcemaps default on (#286) * move option * getTransformerOptions: accept propPath * typescript: fix propPath to compilerOptions.sourceMap * typescript: remove old patch * lint * fix test to use propPath * chore: 🤖 lint * refactor to function setProp * setProp: clarify fn signature Co-authored-by: Christian Kaisermann <christian@kaisermann.me>

Conduitry · 2021-01-25T23:00:20Z

Thanks to sveltejs/svelte#5854, svelte@3.32.0 will now also look for sourcemaps embedded in preprocessor output.

benmccann · 2021-01-25T23:38:22Z

A new version of rollup-plugin-svelte has been released as well. Please try updating both svelte and rollup-plugin-svelte and seeing if it fixes this for you. I'm not 100% sure it will, but it should at least be closer

egargan · 2021-01-26T11:13:17Z

Still facing the same issues as before, TS <script> blocks' source maps appear to map everything to the first line. Again, I'm just running the template repo, adding a couple of lines to App.svelte's script block and checking whether I can set breakpoints in devtools (I'm not sure how else to verify, don't know much about sourcemaps!).

This is with svelte@3.32.0, rollup-plugin-svelte@7.1.0, and svelte-preprocess@4.6.3 - all latest versions.

Maggi64 · 2021-01-26T12:07:54Z

@benmccann @Conduitry Thanks at least for me all issues are fixed

dummdidumm · 2021-01-26T13:05:04Z

Still facing the same issues as before, TS <script> blocks' source maps appear to map everything to the first line. Again, I'm just running the template repo, adding a couple of lines to App.svelte's script block and checking whether I can set breakpoints in devtools (I'm not sure how else to verify, don't know much about sourcemaps!).

This is with svelte@3.32.0, rollup-plugin-svelte@7.1.0, and svelte-preprocess@4.6.3 - all latest versions.

I think the problem is #300 - svelte-preprocess does not seem to support the extends-feature of the tsconfig.json, which means that source mapping is off. If you add "sourceMap": true to compilerOptions you will likely get better results.

milahu · 2021-01-26T13:56:29Z

I think the problem is #300 - svelte-preprocess does not seem to support the extends-feature of the tsconfig.json, which means that source mapping is off.

nope, when sveltePreprocess({ sourceMap: true }) is called (development mode)
then opts = { compilerOptions: { sourceMap: true } } should be set by getTransformerOptions
as last step, so other values for sourceMap are overwritten

svelte-preprocess/src/autoProcess.ts

Lines 129 to 133 in 4446571

    
           if (sourceMap && name in SOURCE_MAP_PROP_MAP) { 
        
             setProp(opts, ...SOURCE_MAP_PROP_MAP[name]); 
        
           } 
        
           return opts;

svelte-preprocess/src/modules/language.ts

Lines 39 to 41 in 4446571

    
           export const SOURCE_MAP_PROP_MAP: Record<string, [string[], any]> = { 
        
             babel: [['sourceMaps'], true], 
        
             typescript: [['compilerOptions', 'sourceMap'], true],

TS <script> blocks' source maps appear to map everything to the first line

"map everything to the first line" happens when svelte-preprocess returns no sourcemap

*should* be fixed by #299 = svelte-preprocess@4.6.2, but maybe there is another bug in that code

This is with ... svelte-preprocess@4.6.3

100% sure? please run

node -e 'console.log(require("svelte-preprocess/package.json").version)'

egargan · 2021-01-26T14:03:51Z

100% sure? please run...

Yep 100%.

svelte/template % node -e 'console.log(require("svelte-preprocess/package.json").version)'
4.6.3

Perhaps the config created by the scripts/setupTypeScript.js isn't correct? I've hacked around with some combinations of adding/removing 'sourceMap' in various config blocks, no dice. I realise I'm not being very helpful haha, it's just that I'm a bit out of my depth here and I don't have much time to spare to learn what's going on and look into this!

milahu · 2021-01-26T17:12:23Z

Perhaps the config created by the scripts/setupTypeScript.js isn't correct?

yes! this was missing:

when sveltePreprocess({ sourceMap: true }) is called (development mode)

fixed in sveltejs/template#216

kaisermann · 2021-02-03T19:29:16Z

I'm reading the whole thread and I'm a bit lost. Apart from prependData modifications not being considered by the sourcemap, are the other issues fixed?

(the ts sourcemap prop I know it is, thanks @milahu 🎉 )

milahu · 2021-02-03T20:23:41Z

should be all fixed, @Maggi64 said

I also tested some more complex configs with autoprefixer & sass prependData.
Lines did map correctly.

CherryDT · 2021-02-27T14:10:01Z

Hm, but, my issue still exists:

On 4.6.9

I also noticed the indentation isn't respected either, the error should be on column 3 and not 1. So it's wrong both in line and column.

dummdidumm · 2021-02-27T15:26:45Z

That's the VS Code extension putting out these warnings, that's unrelated. The VS Code extension uses a somewhat incomplete approach to this and I plan to refactor this some day.

milahu mentioned this issue Dec 8, 2020

stop adding newline before sourceMappingURL postcss/postcss#1486

Closed

dmitrage mentioned this issue Dec 8, 2020

Fix some issues with preprocess source maps sveltejs/svelte#5754

Merged

milahu mentioned this issue Jan 3, 2021

add: parse attached sourcemap from preprocessor sveltejs/svelte#5854

Merged

This comment has been minimized.

Sign in to view

milahu mentioned this issue Jan 6, 2021

sass: remove sourceMappingURL from result.code #297

Merged

milahu added a commit to milahu/svelte-preprocess that referenced this issue Jan 18, 2021

fix typescript: make sourcemaps default on (sveltejs#286)

cc633b5

milahu mentioned this issue Jan 26, 2021

pass option sourceMap to sveltePreprocess sveltejs/template#216

Merged

This was referenced Jan 27, 2021

pass option sourceMap to sveltePreprocess sveltejs/sapper-template#299

Merged

pass option sourceMap to sveltePreprocess sveltejs/template-webpack#37

Merged

kaisermann closed this as completed Feb 3, 2021

benmccann mentioned this issue Jun 24, 2021

Better preservation/fallback of/for source maps #377

Open

Sourcemaps offset #286

Sourcemaps offset #286

Comments

Maggi64 commented Dec 5, 2020 • edited Loading

benmccann commented Dec 7, 2020 • edited Loading

milahu commented Dec 7, 2020 • edited Loading

dummdidumm commented Dec 7, 2020

milahu commented Dec 7, 2020 • edited Loading

milahu commented Dec 8, 2020 • edited Loading

milahu commented Dec 8, 2020

dummdidumm commented Dec 8, 2020 • edited Loading

milahu commented Dec 8, 2020

dummdidumm commented Dec 8, 2020

milahu commented Dec 8, 2020

Maggi64 commented Dec 9, 2020 • edited Loading

milahu commented Dec 9, 2020

Maggi64 commented Jan 2, 2021

milahu commented Jan 3, 2021 • edited Loading

benmccann commented Jan 3, 2021

milahu commented Jan 3, 2021

dummdidumm commented Jan 3, 2021

milahu commented Jan 3, 2021 • edited Loading

milahu commented Jan 3, 2021 • edited Loading

dmitrage commented Jan 3, 2021

CherryDT commented Jan 4, 2021 • edited Loading

Maggi64 commented Jan 5, 2021 • edited Loading

egargan commented Jan 5, 2021 • edited Loading

milahu commented Jan 5, 2021 • edited Loading

Maggi64 commented Jan 5, 2021 • edited Loading

This comment has been minimized.

milahu commented Jan 15, 2021

milahu commented Jan 18, 2021 • edited Loading

egargan commented Jan 18, 2021 • edited Loading

milahu commented Jan 18, 2021 • edited Loading

Maggi64 commented Jan 18, 2021 • edited Loading

Conduitry commented Jan 25, 2021

benmccann commented Jan 25, 2021

egargan commented Jan 26, 2021

Maggi64 commented Jan 26, 2021

dummdidumm commented Jan 26, 2021

milahu commented Jan 26, 2021

egargan commented Jan 26, 2021

milahu commented Jan 26, 2021

kaisermann commented Feb 3, 2021

milahu commented Feb 3, 2021

CherryDT commented Feb 27, 2021 • edited Loading

dummdidumm commented Feb 27, 2021 • edited Loading

Maggi64 commented Dec 5, 2020 •

edited

Loading

benmccann commented Dec 7, 2020 •

edited

Loading

milahu commented Dec 7, 2020 •

edited

Loading

milahu commented Dec 7, 2020 •

edited

Loading

milahu commented Dec 8, 2020 •

edited

Loading

dummdidumm commented Dec 8, 2020 •

edited

Loading

Maggi64 commented Dec 9, 2020 •

edited

Loading

milahu commented Jan 3, 2021 •

edited

Loading

milahu commented Jan 3, 2021 •

edited

Loading

milahu commented Jan 3, 2021 •

edited

Loading

CherryDT commented Jan 4, 2021 •

edited

Loading

Maggi64 commented Jan 5, 2021 •

edited

Loading

egargan commented Jan 5, 2021 •

edited

Loading

milahu commented Jan 5, 2021 •

edited

Loading

Maggi64 commented Jan 5, 2021 •

edited

Loading

milahu commented Jan 18, 2021 •

edited

Loading

egargan commented Jan 18, 2021 •

edited

Loading

milahu commented Jan 18, 2021 •

edited

Loading

Maggi64 commented Jan 18, 2021 •

edited

Loading

CherryDT commented Feb 27, 2021 •

edited

Loading

dummdidumm commented Feb 27, 2021 •

edited

Loading