google / web-starter-kit

Web Starter Kit - a workflow for multi-device websites
http://developers.google.com/web/starter-kit
Apache License 2.0
18.42k stars 3.01k forks source link

(Re-add) Only compile Sass files that have been modified #472

Open addyosmani opened 10 years ago

addyosmani commented 10 years ago

In 0.5.x, we dropped support for only recompiling Sass files that have changed. This is pretty painful during gulp serve in the material-sprint branch :)

This feature was reverted because users were running into issues with it not correctly reloading on save.

Let's try adding back in support. We'll probably want to look at updating paths and any open issues around browser-sync in this area.

addyosmani commented 9 years ago

Just to summarize what the actual bug is here:

During editing of your Sass files, for LiveReload to be truly efficient, you only want the files you have modified to be recompiled. gulp-changed is typically used to achieve this. At the moment, if anyone has run gulp serve against the material-sprint branch you'll notice this happening..

Basically, we're recompiling all our source files which increases time between edit to refresh quite substantially. It would be super useful to only recompile files that were modified since the last compilation run.

Note that the initial Gulp changes for this may well be minimal. There could be larger questions this exploration exposes.

@samthor assigning this to you if you have time. Happy to answer any questions you might have on it :)

samthor commented 9 years ago

Oh yeah, this has been really annoying. I'll take a look.

addyosmani commented 9 years ago

Yay!

samthor commented 9 years ago

So this looks to be tricky, and I'm not really an expert here. At a high level, your normal watcher code won't work: we have plenty of includes that mean that we need a proper tree. gulp-sass-graph seems to be tied to the non-Ruby sass, and that has some problems of its own while running- it doesn't identify everything as unchanged to start with, plus sometimes complains when dependencies can't be found in the change folder.

addyosmani commented 9 years ago

Thanks for taking an initial look at this, Sam. @gauntface and I can take another look at how the current Sass setup can be improved to build on this.

gauntface commented 9 years ago

I'm happy for us to switch from the individual demo files and just have a larger styleguide and styleguide sass file which should drastically reduce the dependency tree. @addyosmani thoughts?

addyosmani commented 9 years ago

Just confirming, the suggestion is to ship:

as part of this repo. Is that right?

ghost commented 9 years ago

With respect to the issue of all source files being recompiled, I think gulp-changed has been incorrectly implemented in the styles task.

As far as I understand it gulp-changed compares the modified time of the source files against the files in a specified destination directory.

From looking at the gulp-changed docs, it seems the current implementation tries to compare the modified date of the source scss files against .scss files in a directory named 'styles':

.pipe($.changed('styles', {extension: '.scss'}))

This will always fail to find matches because the task outputs .css files to '.tmp/styles' and 'dist/styles' directories. So i believe a closer implementation would be:

.pipe($.changed('dist/styles', {extension: '.css'}))

This however will only support a 1:1 source:dest relationship instead of the many:1 needed to support scss files which import other scss files.

I'm only just starting with web starter kit but his jumped out at me as an error when looking through the gulp file so I may have missed the proper intention of this line. Also I'm not familiar enough with web starter kit to suggest any real solution though I understand that gulp-newer is similar to gulp-changed with additional capability of handling many:1 relationships, so it may be more appropriate here.

addyosmani commented 9 years ago

cc @sindresorhus who wrote gulp-changed and IIRC, added in our support for it for thoughts on the above comments. Thanks @DominicBarnard.

adyz commented 9 years ago

I have this issue with BrowserSync & Gulp alone following BrowserSync's guide for Gulp. http://www.browsersync.io/docs/gulp/

If I have two files with .scss extention and I change either one, it will recompile both:

[BS] 2 files changed (main.css, secondary.css)

The same thing happens with Web-Starter-Kit.

sindresorhus commented 9 years ago

Yes, gulp-changed only handles 1:1 relationships. Maybe try using gulp-cached + gulp-remember instead.

motionharvest commented 9 years ago

I found this thread after attempting to solve this same problem on my own. I'm no where near as experienced with the codebase as @sindresorhus.

I've found [https://github.com/xzyfer/sass-graph] which builds a dependency chart for @imports.This seemed like a good method to explore. before i start watching for file changes I save the graph into a variable

var graph = sassGraph.parseDir(config.paths.src + '/scss/pages');
gulp.watch('public_html/src/scss/**/*.scss').on('change', function(file){
    var imports = config.graph.index[file.path].importedBy;
    return sass(imports).pipe(gulp.dest('public_html/dist/css'));
});

Of course that doesn't work, but the idea might. That would at least cut down on the number of files sass is building on each change.

sindresorhus commented 9 years ago

@cmndo Yup, having the dependency graph is the only way to reliably cache the Sass compilation.

robwierzbowski commented 9 years ago

Because:

Would we gain enough speed to make adding a couple more plugins and complexity to the styles task worth it? My styles refresh pretty damn fast (on a MacBook Pro 2015 13").

Based on my experience I would be happy only filtering changed files for CSS.