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

nested styles refer to top-level parent in sourcemap line number #1206

Closed
opeologist opened this issue Oct 16, 2015 · 37 comments
Closed

nested styles refer to top-level parent in sourcemap line number #1206

opeologist opened this issue Oct 16, 2015 · 37 comments

Comments

@opeologist
Copy link

i'm having an issue where my nested styles' sourcemap line number in chrome is referencing the top-most parent selector line and not the line that the nested selector starts.

body {
  h1 {
    background: #f00;
  }
}

will show file.scss:1 instead of file.scss:2 for the h1

i've tried outputting the map file to a separate file and embedding it, all the same.

using 3.4.0-beta1

@xzyfer
Copy link
Contributor

xzyfer commented Oct 17, 2015

Hi @tehOPEologist, thanks for this issue. @mgreter any thoughts on this one?

@mgreter
Copy link
Contributor

mgreter commented Oct 17, 2015

Looks ok to me ...

@opeologist
Copy link
Author

hm, this isn't what's happening for me =/ here's what mine is doing: http://imgur.com/a/WiABA

@mgreter
Copy link
Contributor

mgreter commented Oct 17, 2015

Why is Body on line 5 in your screenshot??

@opeologist
Copy link
Author

i just added some sample code in an already existing file. the devtools image shows core.scss:5, the parent of the h1, instead of 6, where the h1 selector starts.

@opeologist
Copy link
Author

@mgreter am i not understanding how the sourcemapping is supposed to work? if it's working as intended, then ok, but i had someone online say their node-sass was showing the nested line number instead of the nested selector's parent line number =/

@opeologist
Copy link
Author

@xzyfer any update?

@xzyfer
Copy link
Contributor

xzyfer commented Oct 24, 2015

Not really looking into this at the moment. Have you tried te beta?
On 24 Oct 2015 5:30 am, "Aaron Giordano-Barry" notifications@github.com
wrote:

@xzfer any update?


Reply to this email directly or view it on GitHub
#1206 (comment).

@opeologist
Copy link
Author

didn't see there was a new beta. i'll try that and report back. thanks!

@mgreter
Copy link
Contributor

mgreter commented Oct 24, 2015

@tehOPEologist @xzyfer I dont see anything wrong with what libsass produces for the given input!

http://libsass.ocbnet.ch/srcmap/#Ym9keSB7CiAgaDEgewogICAgYmFja2dyb3VuZDogI2YwMDsKICB9Cn0=

@opeologist
Copy link
Author

looks like latest node-sass beta is doing the same thing as before. @mgreter i was under the impression that the sourcemapping would show the line of the actual selector, not its topmost parent. is showing the line number for the topmost parent of the tag the intended result? like i said when i first opened this issue, i've seen nested style with sourcemapping show the line of the actual selector, not its topmost parent..

@opeologist
Copy link
Author

just ran a test with the latest ruby sass and it's sourcemapping nested children on the child's line and not the top-most parent like it should be. this is definitely a bug and isn't working as intended.

Ruby Sass 3.4.19:

body {
  h1 {
    background: red;
  }
}

compiles to:

body h1 {
  background: red; }

/*# sourceMappingURL=core.css.map */

core.css.map:

{
"version": 3,
"mappings": "AACE,OAAG;EACD,UAAU,EAAE,GAAG",
"sources": ["core.scss"],
"names": [],
"file": "core.css"
}

node-sass 3.4.1:

body {
  h1 {
    background: red;
  }
}

compiles to:

body h1 {
  background: red; }

/*# sourceMappingURL=core.css.map */

core.css.map:

{
    "version": 3,
    "file": "core.css",
    "sources": [
        "core.scss"
    ],
    "mappings": "AAAA,IAAI,CACF,EAAE,CAAC;EACD,UAAU,EAAE,GAAI,GACjB",
    "names": []
}

@opeologist
Copy link
Author

@mgreter do you see what i mean now with those examples?

@opeologist
Copy link
Author

@xzyfer any thoughts? trying to figure out if this is a libsass issue and i should open an issue on their repo or if this is node-sass specific... definitely an issue that needs addressing!

@chriseppstein
Copy link
Contributor

Ruby Sass generates different source mappings for that input: "AACE,OAAG;EACD,UAAU,EAAE,IAAI"

@opeologist
Copy link
Author

@chriseppstein yes, and it seems like Ruby Sass is generating the correct mappings whereas libsass/node-sass is generating only the top-most parent of the selector

@opeologist
Copy link
Author

@mgreter any ideas as to why this is happening? i just want to know if this is intended or not, as the sourcemap is visibly different than ruby sass

@jenwachter
Copy link

Just wanted to add that I am also having the exact same issue.

@opeologist
Copy link
Author

@jenwachter ok cool, i'm glad i'm not the only one experiencing this. hopefully we can get some eyes on this and get it resolved asap!

@HugoHeneault
Copy link

I'm facing this issue too. It's quite hard to work with lots of nested selectors... :'(

@opeologist
Copy link
Author

gonna close this for now, since it seems to be an issue with how the browsers are interpreting the srcmaps. please reference this for a temporary way to get the lines of the styles: sass/libsass#1747

@xzyfer xzyfer reopened this Nov 28, 2015
@xzyfer
Copy link
Contributor

xzyfer commented Nov 28, 2015

Although I agree with @mgreter that we're likely technically correct, the fact that we differ from Ruby Sass in this is a enough to provide a fix.

@mgreter
Copy link
Contributor

mgreter commented Nov 28, 2015

@xzyfer ruby sass sourcemaps are lacking a lot according to previous discussions with @am11. I decided to do better than that and therefore libsass has much more detailed source-maps than ruby sass does (not sure if this is still true, since those discussion are over a year old). The question is really if we want to merge the crutch in #1759 into master or not. That change is techically not correct, but will make chrome and firefox report a more usefull mapping. But it could confuse more advanced tools, since that mapping is technically not correct.

@xzyfer
Copy link
Contributor

xzyfer commented Nov 28, 2015

The question is really if we want to merge the crutch in #1759 into master or not

Yep I agree. On it's merits alone merging that fix is kinda sucky, but I think we should (see below).

That change is techically not correct, but will make chrome and firefox report a more usefull mapping

This is my concern. LibSass feels broken to users which IMO is the important concern

it could confuse more advanced tools, since that mapping is technically not correct

As you previously mentioned these tools don't yet exist. Until someone reports an issue in regard to this I think it's more important to match user expectations.

@mgreter
Copy link
Contributor

mgreter commented Nov 28, 2015

OK, so this bascially means you're in favor of merging sass/libsass#1759 into master. Although it's technically not correct, it will satisfy the main consumers (aka browsers) of our sourcemaps. If so please feel free to hit the merge button on that PR!

@xzyfer
Copy link
Contributor

xzyfer commented Nov 28, 2015

This has been fixed in LibSass. I'll close this issue when we do a new release with the update LibSass version.

@opeologist
Copy link
Author

thank you so much all! i'll be patiently waiting for the new node-sass release! :D

@HugoHeneault
Copy link

Awesome! Can't wait for it!!

@ericwooley
Copy link

Any updates on this?

1 similar comment
@tomlagier
Copy link

Any updates on this?

@opeologist
Copy link
Author

just tested on node-sass v3.5.0-beta.1 and confirm sourcemap line numbers are referring to correct selector! thank you SO MUCH @mgreter !!!!! i'll leave this to be closed by you once the version goes out of beta.

@mgreter
Copy link
Contributor

mgreter commented Jan 30, 2016

Even though it is technically wrong what I did to achieve this (since browsers interpret it not carefully enough IMO), you're welcome 😄

@mgreter mgreter closed this as completed Jan 30, 2016
@opeologist
Copy link
Author

xD thanks again man!

@am11
Copy link
Contributor

am11 commented Jan 30, 2016

Was this problem occurring only on Chrome or every browser tools with source-map support? Last time I checked (few months ago, while giving a presentation), my sample worked on Chrome, Edge (MSDN) and Firefox (Mozilla hacks). If there is a disparity between what libsass was generating previously and how Chrome is interpreting it, with this fix, it might have broken other browsers. :/
LibSass emits detailed mappings, which many trans-compilers (of JS and CSS) lack including RubySass. @mgreter has nailed this feature! 👍

@mgreter, note that what I used in WebEssentials2013 for VS-IDE preview window was based on https://github.com/mozilla/source-map (which is probably used in Firefox tools implementation as well), so it might interpret the line numbers differently. Last I remember, line and col number were off by one (line was 0-based and col was 1-based or vice versa), viz the typical:

there are two hard things in computer science: cache invalidation, naming things, and off-by-one errors.

Thank you so very much nonetheless. 😎

@mgreter
Copy link
Contributor

mgreter commented Jan 30, 2016

@ggedde
Copy link

ggedde commented Sep 10, 2017

I am still having this issue on FIrefox.
There is another open issue for this that might clarify why this is happening: #2092

@SweetsXob
Copy link

This problem does not seem to be solved now.
It seems less doesn't have this problem

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