search

joliss / broccoli-sass

Sass compiler for Broccoli, using libsass
MIT License
44 stars 97 forks source link

Fix sourcemaps #21

Closed dschmidt closed 10 years ago

dschmidt commented 10 years ago

Hey,

I tried to fix sourcemaps with the current libsass/node-sass implementation and without having to include all the .scss files in the sourcemap.

This is basically a proof of concept and my first attempt to create something useful with broccoli and node.js in general, so I desperately need feedback if

a) this approach seems any good at all b) the node functions used were the correct ones c) the assumptions i've made about the paths make any sense or are way to fragile

Of course the code still needs cleaning up, a way to configure to (de)activate nice paths if the feature is desired at all, no debug output with console.log etc. but this should make it easier to see what's going on and for me to debug if it does not work for you.

I'm open for discussion, here or on IRC.

Best regards, Domme

dschmidt commented 10 years ago

At @rwjblue's requet I've created a small demo project (using Ember CLI) showing it's working.

https://github.com/dschmidt/ember-cli-sass-demo

You just need to manually copy over my broccoli-sass/index.js though.

dschmidt commented 10 years ago

I just confirmed that it does not break the workspace feature in Chrome. It still allows you to map files to your local files, edit them and have broccoli rebuild it for you.

simonexmachina commented 10 years ago

Hi Dominic. Thanks for your work on this. I'm away for the weekend but I'll have a look when I get back on Monday.

simonexmachina commented 10 years ago

FWIW this isn't necessary for me. The source map goes all the way back, through the top/ files, to the source files. And using workspaces I can edit the source files in the browser. So I suspect that we may have something else going on here :)

dschmidt commented 10 years ago

Hey Simon,

I know this is not neccessary to make workspaces work, just wanted to point out that it doesn't break workspace usage :)

Best regards, Domme

simonexmachina commented 10 years ago

Hi Domme, I've had a look at your PR and I'm afraid we think this approach leads to a dead end. I actually tried a similar approach and the problem you hit eventually is knowing which paths in the input tree corresponds to the relative paths provided in the source map.

@joliss and I had a lengthy chat about this (which you can read in the link above), and basically this should be handled by libsass, and that's the approach we're pursuing. But thanks for your effort on this one. Don't be discouraged!

dschmidt commented 10 years ago

Alright. I did not really fancy this solution myself, but I played around with it and didn't want to let my code go bitrot without any other opinions.

I've seen your PR to libsass for inline sourcemaps. I've read (kind of asking here myself, I did not try so I can't confirm myself) that they tend to be much more memory intensive in the browser and slower than external files.. Wouldnt it be easier to just have an option to have a different base dir than cwd? That seems to be the main issue right now.. otoh it probably helps with files outside of the source dir :-\

simonexmachina commented 10 years ago

Yes, I agree that it would be good to have an option for broccoli-sass to includeSources, and this is exactly what we investigated. The problem is mapping the file paths in the source map to the files in the input trees - there's no way to know which input file was used for a given source file in the source map.

Here's my implementation, feel free to have a look if you like. This is attempting to populate sourcesContent, but this approach could equally well be used to populate output.css.sources/ in the output tree.