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

Source Maps: Add 'SourcesContent' support #212

Closed
bdkjones opened this issue Dec 19, 2013 · 11 comments
Closed

Source Maps: Add 'SourcesContent' support #212

bdkjones opened this issue Dec 19, 2013 · 11 comments

Comments

@bdkjones
Copy link

bdkjones commented Dec 19, 2013

I see that source maps have made it to libsass!

However, it would be far better if they used the 'sourcesContent' property outlined in the official spec: https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit?hl=en_US&pli=1&pli=1#heading=h.lmz475t4mvbx

In short, you take each Sass file and dump its contents directly into the *.map file, in that sourcesContent array. The advantage to this is that it ELIMINATES problems with resolving the path from the CSS output file to the original Sass file. It makes source maps "just work". No futzing around with Chrome's DevTools workspaces, etc.

Yes, the *.map file is a little larger as a result, but that's okay because the only time we use that is during development --- it has no effect on the published website's load speed.

This should not be too hard to implement, either.

Most importantly, Less 1.5 has this ability. I would love to see Sass keep up.

@mgol
Copy link

mgol commented Dec 19, 2013

Won't it slow everything down?

Also, you don't need to set up workspaces for the linking to work.

@bdkjones
Copy link
Author

All I can tell you is that Less does it and it is lightning fast. I really can't recommend it enough.

On Dec 19, 2013, at 4:12 AM, Michał Gołębiowski notifications@github.com wrote:

Won't it slow everything down?

Also, you don't need to set up workspaces for the linking to work.


Reply to this email directly or view it on GitHub.

@mgol
Copy link

mgol commented Dec 19, 2013

I think it can hamper productivity. From what I understand, this will make it impossible to edit SCSS files from within Chrome Dev Tools since the mappings will be to the copied inlined SCSS stylesheets and not to the source onces.

Current situation is that:

  1. you don't have to setup workspaces for the linking to work
  2. if you do, you may not only see the source lines but also live-edit them.

Am I missing something?

@emagnier
Copy link

Maybe this could be an option (as indicated in the spec).
So we could have the choice to use it or not.

@bdkjones
Copy link
Author

@etienne is right: it should be an option, just as it is for Less.

I think the vast majority of folks would rather edit stylesheets in a real editor than work in Chrome's Dev tools. For these people, having the original sass sources right in the map file seriously reduces headaches.

A good example is Wordpress: it can be incredibly tricky to get source map urls to resolve to the correct path when you have Wordpress running with sub-sites. Another example is a sass file that is not part of a website's project folder. I might keep a sass file defining some basic styles on my disk and use it from a shared location across many web projects, without ever copying it into my website folder. In this case, source mapping urls will always fail. The only way to make that work is to embed the source in the map file.

Sent from my iPhone

On Dec 19, 2013, at 10:50 AM, Etienne notifications@github.com wrote:

Maybe this could be an option (as indicated in the spec).
So we could have the choice to use it or not.


Reply to this email directly or view it on GitHub.

@HamptonMakes
Copy link
Member

I agree. Option. Though, I could be swayed that it should be the default in
sassc... since its easier to startup.

On Thu, Dec 19, 2013 at 12:31 PM, Bryan Jones notifications@github.comwrote:

@etienne is right: it should be an option, just as it is for Less.

I think the vast majority of folks would rather edit stylesheets in a real
editor than work in Chrome's Dev tools. For these people, having the
original sass sources right in the map file seriously reduces headaches.

A good example is Wordpress: it can be incredibly tricky to get source map
urls to resolve to the correct path when you have Wordpress running with
sub-sites. Another example is a sass file that is not part of a website's
project folder. I might keep a sass file defining some basic styles on my
disk and use it from a shared location across many web projects, without
ever copying it into my website folder. In this case, source mapping urls
will always fail. The only way to make that work is to embed the source in
the map file.

Sent from my iPhone

On Dec 19, 2013, at 10:50 AM, Etienne notifications@github.com wrote:

Maybe this could be an option (as indicated in the spec).
So we could have the choice to use it or not.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//issues/212#issuecomment-30963697
.

@bdkjones
Copy link
Author

Yep. @mzgol raises a good point with the live-editing in Chrome Dev Tools, but the reality is that most folks work in a good editor, save the file, and rely on any number of apps to refresh their browsers/devices. The reason this is very pertinent for me is that I'm about to release CodeKit 2, which will refresh browsers on everything from your Mac to your freaking kitchen refrigerator.

Source Maps are a pain. Embedding the source content into the map file makes them much less painful.

@mgol
Copy link

mgol commented Dec 22, 2013

I realize for many folks it might be better to have the source inlined. I don't care what's the default, I just need an option to use regular source mappings. With that, it's 👍 from me.

@dpashkevich
Copy link

Would be great to have this option, although I agree that it breaks live editing of the real files from CDT

@simonexmachina
Copy link
Contributor

I'd just like to add a big +1 to this issue 👍

@mgreter
Copy link
Contributor

mgreter commented Nov 7, 2014

This was addressed in #591

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

Successfully merging a pull request may close this issue.

7 participants