Source-maps option about gulp-sass HOT 34 CLOSED

From what I can tell this feature doesn't yet exist in libsass. Supposedly, native source maps are going to be part of the 3.3 release. I don't know when that will make its way into the C implementation.

from gulp-sass.

I'm using source-maps with libsass with the equivalent grunt-task.
I'll lookup the libsass docs about this feature (is not on the master release yet) and let you know what I find.

Sent from my iPhone

On 10/01/2014, at 21:55, David Manning [email protected] wrote:

From what I can tell this feature doesn't yet exist in libsass. Supposedly, native source maps are going to be part of the 3.3 release. I don't know when that will make its way into the C implementation.

—
Reply to this email directly or view it on GitHub.

from gulp-sass.

Okay, please let me know. I'll look over quickly and see what grunt-sass is doing.

from gulp-sass.

Actually grunt-sass is not updated yet also, I'm using modified (quite simple) that wasn't merged yet. I'll post that info too when I'm on a computer.

Sent from my iPhone

On 10/01/2014, at 22:02, David Manning [email protected] wrote:

Okay, please let me know. I'll look over quickly and see what grunt-sass is doing.

—
Reply to this email directly or view it on GitHub.

from gulp-sass.

@dlmanning David, node-sass, which is just a wrapper to libsass, just updated to include the source maps feature.
Might be useful: https://github.com/andrew/node-sass/releases/tag/v0.8.0

from gulp-sass.

Thanks! I'll look into how to implement this.

from gulp-sass.

@dlmanning Thank you for looking into this.
One more detail, a comment from the guy who implemented source-maps in libsass, that also might be useful to you:
sass/node-sass#194 (comment)

from gulp-sass.

grunt-sass has updated with source map support.

from gulp-sass.

I looked at this exact thing earlier, and currently the sourcemaps in node-sass (achieved by the sourceComments:'map' option) are only supported with files - not the data option, which gulp-sass is using.

from gulp-sass.

yeah. This is what makes it pretty complicated to implement.

from gulp-sass.

Not sure what it means, is that something missing in libsass or in the other hand is something more related to the way gulp handles files?

Sent from my iPhone

On 14/01/2014, at 23:17, David Manning [email protected] wrote:

yeah. This is what makes it pretty complicated to implement.

—
Reply to this email directly or view it on GitHub.

from gulp-sass.

node-sass allows you to pass either a file reference (file), or a string of sass (data). gulp-sass uses the latter because gulp takes care of opening the actual files and reading them into a buffer. It's not immediately apparent to me how I could rework this to use node-sass's file option without reimplementing a large portion of gulp's native functionality.

from gulp-sass.

Ah,ok. Thanks for clarifying that. It helps me getting back to libsass people in order to have an idea on when this will become possible.

Sent from my iPhone

On 14/01/2014, at 23:23, David Manning [email protected] wrote:

node-sass allows you to pass either a file reference (file), or a string of sass (data). gulp-sass uses the latter because gulp takes care of opening the actual files and reading them into a buffer. It's not immediately apparent to me how I could rework this to use node-sass's file option without reimplementing a large portion of gulp's native functionality.

—
Reply to this email directly or view it on GitHub.

from gulp-sass.

If you need an immediate solution I'd probably do it the non-plugin/dirty way - something along the lines of (in your gulpfile):

var sass = require('node-sass');

gulp.task('sass', function() {
  sass.render({
    file: scss_filename
    [, options..]
  });
});

Haven't tested it though - obviously piping wouldn't work this way.

from gulp-sass.

Thank you for the suggestion. I'll wait for a native solution because without piping that is pretty much what the grunt task does, so no point fiddling with it at the moment.

from gulp-sass.

What is the reason you don't use the file based approach?

Something like

opts.file = file.path;

instead of

opts.data = file.contents.toString();

node-sass allows you to pass either a file reference (file), or a string of sass (data). gulp-sass uses the latter because gulp takes care of opening the actual files and reading them into a buffer.

I'm not familiar with gulp or node stream. Don't hesitate to let me know if I miss something.

It is very common that the source file imports several other files which will be read and parsed by the libsass parser anyway. I don't think it is desirable to control the reading of all these files in gulp-sass. How would you implement that without duplicating the resolving of the imports? Is there any particular reason why you would like to control the opening of just the source file?

from gulp-sass.

@svnieuw gulp guidelines prefer that the altered file.contents is taken from gulp.src, though if node-sass takes a single file entry point you could modify it with some of the file modifying code here:
https://github.com/stevelacy/gulp-browserify/blob/master/index.js

from gulp-sass.

@stevelacy I still do not see why we should prefer to provide the file contents. Could you provide some pseudo-code how you expect the node-sass (or libsass) interface to be in order to be usable with gulp-sass?

from gulp-sass.

gulp-plugins takes file contents in, changes them, and pipes it out. Files are taken in with gulp.src, and outputed from the plugin as a buffer.
When I created gulp-stylus I had stylus take the files as contents and compile, then @'import the mixins, etc from the file's content's using the files path to afix the mixin location.

If you are creating a file path "entry point", you will create a mess which gulp-browserify had, one of them being gulp hanging due to the compiler (sass or browserify) hitting some issues or very large files and not piping them back correctly.

CC @phated @contra

from gulp-sass.

I don't necessarily agree with gulp.src being an entry point for modules that trace dependency graphs, but I am not sure how to deal with them in the concept of vinyl-fs streams.

from gulp-sass.

node-sass is a wrapper around libsass (which is a C/C++ library). If you change line 20 of the current gulp-sass implementation from

opts.data = file.contents.toString();

to

opts.file = file.path;

libsass will use the file context instead of the data context. The only difference between these is that the file context is capable to output file based information such as source maps if you configure it to do so, because it can use the path of the input file. This is an implementation detail of libsass. Client code like gulp-sass is not affected by this.

@stevelacy I guess you are confused by the fact that gulp supports both buffer based and stream based plugins. gulp-sass is a buffer based plugin. It isn't possible to support streams because libsass only supports buffers. The way you provide the buffer (as data string or file path) doesn't matter. If I look at other similiar compiler plugins like gulp-coffee or gulp-traceur, gulp-sass is implemented in a similar way.

from gulp-sass.

Gulp already has a way to solve this. https://github.com/gulpjs/gulp/blob/master/docs/API.md#optionsread - if contents is null, it should probably use the file option, otherwise it should use the data option.

from gulp-sass.

@svnieuw
I do understand what gulp is. gulp-coffee uses the file.contents as defined here: https://github.com/wearefractal/gulp-coffee/blob/master/index.js#L25
And traceur: https://github.com/sindresorhus/gulp-traceur/blob/master/index.js#L25
Plugins by Fractal do not use file.path as an 'entry point'
Currently gulp-sass does not use file.path, but the file.contents.

from gulp-sass.

I still fail to see how not using file.contents, but file.path instead which act as readonly input parameters for libsass could create problems. The same buffer will be assigned to file.contents and the extension of file.path will still be changed to .css in the success callback as defined here: https://github.com/dlmanning/gulp-sass/blob/master/index.js#L24. In both scenarios file.contents and file.path be updated in an identical way.

Can you provide me a concrete example how it will go wrong with file.path? Or can you provide me some pseudo-code how you want to see the node-sass or libsass interface in order to support source maps? If it's clear to me what needs to change, I will be glad to change libsass as needed.

from gulp-sass.

So, gulp-sass still has no source-maps support? Is there a workaround I could use?

from gulp-sass.

So, if I understand this correctly, gulp prefers to read the file contents once and pipe it forward. This makes possible to change the content in memory without saving changes to the filesystem. This way one could for example run automatic prefixing or cleaning or whatever on the .scss files before continuing compiling them to css.

Then there is node-sass or libsass that supports this approach on the basic level, but for source maps needs to get the path correctly so that it can reference it. And if it gets the path, it might as well read the file contents from the system. Did I get this right?

I have a couple of questionsn then:

• If the file has already been read to memory, is'nt it a bit faster to read from the memory than from the filesystem?
• Could all of this work if gulp-sass would pass both the file contents and the file path to node-sass/libsass and it would then use that content with that path without reading the file from the system?

Of course if someone changes the .scss file while gulp piping it and then passes it for compiling with sourcemaps, the pointers to original file will point to wrong places. Maybe this edge case however should be left for the user to handle.

from gulp-sass.

I'm confused now. What is everyone arguing about? Giving the file.path as well as the file.contents to a compiler is not abnormal. This is done in gulp-less, gulp-coffee, gulp-uglify, etc.

gulp-less and gulp-uglify do embedded sourcemaps which I think is what should be done here as well. If the library you're wrapping doesn't support embedded maps, you should hack it out somehow. It's just a base64 of the actual sourcemap file contents instead of the path. There are docs on this if you google for them. Emitting two types of files (a .map and a .css) will cause complexities with plugins downstream and make the user have to do a bunch of tedious filtering.

from gulp-sass.

Okay, I see a way to do this. It's a WIP

from gulp-sass.

Okay, folks. Enjoy.

1e7da62

from gulp-sass.

Not to sure about the 'fs'.
@contra ?

from gulp-sass.

What do you mean you aren't sure about it?

from gulp-sass.

@stevelacy If sass doesn't support inline maps there is no other way. Just something you have to deal with. You should 100% open an issue though so it can be fixed in the future.

from gulp-sass.

@dlmanning thanks!

from gulp-sass.

Wouldn't https://www.npmjs.org/package/vinyl-sourcemaps/ be helpful to have the sourcemaps generated as a separated file?

from gulp-sass.