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

Empty source map causes sources to be unset #116

Open
dummdidumm opened this issue Mar 14, 2021 · 14 comments
Open

Empty source map causes sources to be unset #116

dummdidumm opened this issue Mar 14, 2021 · 14 comments

Comments

@dummdidumm
Copy link

When combining source maps, and one of the maps is empty, the sources array is empty afterwards.

[
  {
    version: 3,
    names: [],
    sources: [ '..path/to/$layout.svelte' ],
    sourcesContent: [ '<p>asd</p>\r\n' ],
    mappings: ';;;;;;;'
  },
  {
    version: 3,
    mappings: 'AAAA,CAAC,CAAC,CAAC,GAAG,CAAC,CAAC,CAAC,CAAC;',
    names: [],
    sources: [ '$layout.svelte' ]
  }
]

It produces { version: 3, mappings: ';;;;;;;', names: [], sources: [] } (note the empty sources array. Mappings is also empty, not sure if that is intended).

Digging into the source code, I think that in this function the combination is done, and the line where sources is updated is never reached in case of empty mappings.

@jridgewell
Copy link
Collaborator

Hi @dummdidumm, I'm a bit confused on what output you expect here. The first map says there are 8 lines, put there are no segments on those lines that point into the original file. The second map says there's 2 lines, with only the first having any content.

What generated these sourcemaps? It seems like the output is incorrect, but maybe I'll understand better if you can give a simple reproduction.

@jridgewell
Copy link
Collaborator

This might be related to #91? In particular, if a mappings line is empty, we consider it to have no content.

@dummdidumm
Copy link
Author

dummdidumm commented Mar 15, 2021

Sorry for the confusion, here is a minimum reproducible with two empty source maps:

const result = remapping([
  {
    version: 3,
    names: [],
    sources: [ 'foo.js' ],
    sourcesContent: [ '' ],
    mappings: ''
  },
  {
    version: 3,
    mappings: '',
    names: [],
    sources: [ 'foo.js' ]
  }
],
() => null);
// result is { version: 3, mappings: [], sources: [], names: [] }
// expected { version: 3, mappings: [], sources: ['foo.js'], names: [] }

@jridgewell
Copy link
Collaborator

Ok, so we're expecting sources to be included even if there are 0 found segments? Also, we'd need to pull the sourcesContent from the original somehow, but it's not clear which content is associated because the first map needs to point into the second for the second to figure out which of its sources to look up.

And, is there a case where there are 2 sources in a file without any segments? That would be ambiguous.

@dummdidumm
Copy link
Author

I think 2 sources without any segments is a case that should never occur, simply because it makes no sense - how did 2 sources end up in the map if there is nothing to map? So I think this "no mappings" case may be a special one which needs special treatment like

if (mapIsEmpty && noSourcesSet) {
   sources.push(firstSourceOfEmptyMap)
}

I can't quite follow you on the sourcesContent bit, I'm not that well-versed with source maps unfortunately 😄 Maybe the same special-case-logic as for the sources array applies.

@jridgewell
Copy link
Collaborator

We might be able to use the first source (if it's the only source) and traceSegment({ line: CURRENT_LINE, column: 0, name: '' }).

Are you using magic-string and not setting hires: true?

@dummdidumm
Copy link
Author

I'm using magic-string and hires is true. Not sure though if this even matters in the case of empty source maps, they would be empty either way, right?

@jridgewell
Copy link
Collaborator

I'm using magic-string and hires is true. Not sure though if this even matters in the case of empty source maps, they would be empty either way, right?

I was trying to figure out what was generating the segment-less sourcemap. Appears it's coming from Svelte, in particular it's use of code-red to print out the AST tree: https://github.com/Rich-Harris/code-red/blob/3b32d2ef5bd954cb85a0d005f3a328bae57c6a97/src/print/index.ts#L60-L86

Normally, the AST being printed would contain loc objects that point into the original code. But when Svelte is processing a file without any dynamic code, it doesn't attach any locs (though it does add start and end). I think this is a bad behavior, Svelte should be associating source location for static code as well. Eg, something like the following:

<h1>decoded-sourcemap</h1>
<div>replace me</div>

The h1 should be associated with { start: { line: 1, column: 2 } }, the decoded-sourcemap with { start: { line: 1, column: 5 } }, etc. With the AST given the correct loc associations, code-red's printer will make a sourcemap with valid segments, and the bug will disappear.


So now we have to decide what to do for an empty map on our end. The complication is, where should we end the trace at? Just because the current map only has one source, the next map in our chain could have multiple sources. There's no way for us to associate a empty map into the correct original source in this scenario. I think that means that this case is invalid, and shouldn't be directly supported.

jridgewell added a commit that referenced this issue Mar 17, 2021
Re: #116, which has an sourcemap without any segments. There's no point in keeping trailing lines
without any segments, and truncating better matches the output from `source-map`.
@jridgewell
Copy link
Collaborator

Closing as intended behavior with the explanation in #116 (comment).

@jridgewell
Copy link
Collaborator

You know what, I'm changing my mind. Supporting an edits only type of sourcemap would reduce memory usage (no longer need to generate giant hires maps), and drastically simply the editing processes.

The rules for an "editmap" are:

  • Opt in (TBD how, maybe an extra field on the map, or maybe an optional callback function that says "yup, this is an edit map")
  • Single source
  • Lines without segments are assumed unchanged (we'll inherit all segments of the source's respective line)
  • Lines with only an initial segment (outputColumn = 0, souceLine = line and sourceColumn = 0) are also assumed unchanged
    • This is the default lowres output from magic-string
  • The remapped file will only contain the number of lines in the editmap.
    • If the editmap contains only 10 lines, and the source contains 20 lines, there's only going to be 10 lines in the remap
  • Lines with multiple segments (or a non-initial segment) are considered changed, and follow the same tracing logic as normal maps

@milahu
Copy link
Contributor

milahu commented Mar 17, 2021

note, probably this will only work with the array interface of remapping, where the previous sourcemap is unique

"editmap"

aka overlay map, transparent map, fall through map, partial map ....

@jridgewell
Copy link
Collaborator

note, probably this will only work with the array interface of remapping, where the previous sourcemap is unique

Right. The array map is guaranteed to work, and the function-callback style will only work when there's a single source file. If there's more, we'll throw during building, similar to the array interface's error.

@benmccann
Copy link

Thanks for the excellent investigation @jridgewell! I've filed an issue in the Svelte repo with your findings sveltejs/svelte#6092

@jridgewell
Copy link
Collaborator

I opened #120 which will partially solve this. I hit a problem with segmentless lines (the ;;; in your mappings means there's no segment on those 4 lines). Because they contain segments, they don't point at a child sourcemap (what's usually segment[1]) and they don't point at a line in that sourcemap (segment[2]). No matter what I tried, even if I limited to a single child sourcemap, I couldn't manage to track the appropriate child line number.

This is because lines any number of lines could be removed, or added, or mixed. Without a segment to tell me the respective line in the sourcemap, the remapped segments pointed at meaningless lines in the original source. So I had to require a line marker segment, like what magic-string generates by default. And once we require a line marker, it becomes easy to expand this to any number of child sources. So any maps can take advantage of this, and I've enabled it by default.

So some work will still be required for Svelte to generate correct sourcemaps. Eg, sveltejs/svelte#6092.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants