Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

source map has multiple maps for a certain generated location #2092

Closed
tromey opened this issue Sep 7, 2017 · 4 comments
Closed

source map has multiple maps for a certain generated location #2092

tromey opened this issue Sep 7, 2017 · 4 comments

Comments

@tromey
Copy link

tromey commented Sep 7, 2017

When reporting an bug, you must provide this information:

  • NPM version (npm -v): 5.3.0

  • Node version (node -v): v8.4.0

  • Node Process (node -p process.versions):{ http_parser: '2.7.0',
    node: '8.4.0',
    v8: '6.0.286.52',
    uv: '1.13.1',
    zlib: '1.2.11',
    ares: '1.10.1-DEV',
    modules: '57',
    nghttp2: '1.22.0',
    openssl: '1.0.2l',
    icu: '59.1',
    unicode: '9.0',
    cldr: '31.0.1',
    tz: '2017b' }

  • Node Platform (node -p process.platform): linux

  • Node architecture (node -p process.arch): x64

  • node-sass version (node -p "require('node-sass').info"):
    node-sass 4.5.3 (Wrapper) [JavaScript]
    libsass 3.5.0.beta.2 (Sass Compiler) [C/C++]

  • npm node-sass versions (npm ls node-sass):
    └── [email protected]

This comes from https://bugzilla.mozilla.org/show_bug.cgi?id=1364232. There's a repo here: https:/ggedde/tomtest

Using 0-based lines and columns, in that repository, the source map has two entries for the generated file's line 2, column 2. The source map says this maps to both line 0 column 0, and to line 1 column 2.

This seems like an error; I think source maps should only have one entry for any given generated location.

@mgreter
Copy link
Contributor

mgreter commented Oct 5, 2017

If I got it right, this is strictly related to sass/libsass#1747 (comment). Would need to check how firefox handles this case today. Browser tend to expect only one mapping for a complex selector, although with sass each part of a complex selector can come from different sources. Browsers seem to report the mapping where the selector starts, which turns out to be pretty unusable with sass, since it will always point to the "root" block. It should preferably report where the declaration scope was opened (or position of the last selector). To overcome this issue we have added the "crutch" mentioned in the linked issue.

I think source maps should only have one entry for any given generated location

I would disagree, although I see the point. But it is not explicitly forbidden by the SourceMap V3 Draft (at least last time I checked) and it would makes sense for i.e. the following example (source maps tools unfortunately aren't there yet and the specs leave a lot of room open for interpretation in these cases):

@function inner($val) { @return $val; }
@function outer($val) { @return inner($val); }
foo { value: outer('value'); }

@ggedde
Copy link

ggedde commented Oct 17, 2017

@mgreter I just commented on this and suggested that since a hard change is not ideal that we have a compile option for this.
sass/libsass#1747 (comment)

Would that work?

@saper
Copy link
Member

saper commented Oct 18, 2019

node-sass 4.12.0 produces identical source map as the one in the test case.

As of Firefox 69.0.2 I get reference now to ta.scss line 2

Problem solved on Firefox side?

@saper saper closed this as completed Oct 18, 2019
@tromey
Copy link
Author

tromey commented Oct 22, 2019

As of Firefox 69.0.2 I get reference now to ta.scss line 2
Problem solved on Firefox side?

I don't work on Firefox any more, and I've forgotten some of the details here, but IIUC the issue is that there is no right answer. If the source map has multiple mappings for a given generated location, and the source map consumer (i.e., Firefox devtools) wants an original location, then it has to pick one. Which one you get seems arbitrary, like maybe FF switched from "first match" to "last match" ... but since it's not really in the spec, it could well switch back any time.

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

No branches or pull requests

5 participants