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

Commit

Permalink
Decouple the graph and render logic from the fs watcher (#2020)
Browse files Browse the repository at this point in the history
This logic is all tightly coupled in the bin. This logic has been
notoriously impossible to test due to weird fs timing issues. By
extracting the actual logic we're now able to test it in isolation.

With this in place replacing Gaze (#636) becomes a viable option.
This PR not only massively increases our test coverage for the
watcher but also address a bunch of known edge cases i.e. orphaned
imports when a files is deleted.

Closes #1896
Fixes #1891
  • Loading branch information
xzyfer authored Jun 17, 2017
1 parent 61d7e1c commit aadf8d4
Show file tree
Hide file tree
Showing 11 changed files with 327 additions and 47 deletions.
70 changes: 24 additions & 46 deletions bin/node-sass
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
var Emitter = require('events').EventEmitter,
forEach = require('async-foreach').forEach,
Gaze = require('gaze'),
grapher = require('sass-graph'),
meow = require('meow'),
util = require('util'),
path = require('path'),
glob = require('glob'),
sass = require('../lib'),
render = require('../lib/render'),
watcher = require('../lib/watcher'),
stdout = require('stdout-stream'),
stdin = require('get-stdin'),
fs = require('fs');
Expand Down Expand Up @@ -229,63 +229,41 @@ function getOptions(args, options) {
*/

function watch(options, emitter) {
var buildGraph = function(options) {
var graph;
var graphOptions = {
loadPaths: options.includePath,
extensions: ['scss', 'sass', 'css']
};

if (options.directory) {
graph = grapher.parseDir(options.directory, graphOptions);
} else {
graph = grapher.parseFile(options.src, graphOptions);
}
var handler = function(files) {
files.added.forEach(function(file) {
var watch = gaze.watched();
if (watch.indexOf(file) === -1) {
gaze.add(file);
}
});

return graph;
files.changed.forEach(function(file) {
if (path.basename(file)[0] !== '_') {
renderFile(file, options, emitter);
}
});

files.removed.forEach(function(file) {
gaze.remove(file);
});
};

var watch = [];
var graph = buildGraph(options);

// Add all files to watch list
for (var i in graph.index) {
watch.push(i);
}
watcher.reset(options);

var gaze = new Gaze();
gaze.add(watch);
gaze.add(watcher.reset(options));
gaze.on('error', emitter.emit.bind(emitter, 'error'));

gaze.on('changed', function(file) {
var files = [file];

// descendents may be added, so we need a new graph
graph = buildGraph(options);
graph.visitAncestors(file, function(parent) {
files.push(parent);
});

// Add children to watcher
graph.visitDescendents(file, function(child) {
if (watch.indexOf(child) === -1) {
watch.push(child);
gaze.add(child);

This comment has been minimized.

Copy link
@evanfuture

evanfuture Nov 14, 2017

Contributor

@xzyfer this is seems to be missing from the refactored version ( gaze.add(child) in the on.changed graph.visitDescendents) Maybe it's what's causing the watchers to still fail?

}
});
files.forEach(function(file) {
if (path.basename(file)[0] !== '_') {
renderFile(file, options, emitter);
}
});
handler(watcher.changed(file));
});

gaze.on('added', function() {
graph = buildGraph(options);
gaze.on('added', function(file) {
handler(watcher.added(file));
});

gaze.on('deleted', function() {
graph = buildGraph(options);
gaze.on('deleted', function(file) {
handler(watcher.deleted(file));
});
}

Expand Down
83 changes: 83 additions & 0 deletions lib/watcher.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
var grapher = require('sass-graph'),
clonedeep = require('lodash.clonedeep'),
config = {},
watcher = {},
graph = null;

watcher.reset = function(opts) {
config = clonedeep(opts || config || {});
var options = {
loadPaths: config.includePath,
extensions: ['scss', 'sass', 'css']
};

if (config.directory) {
graph = grapher.parseDir(config.directory, options);
} else {
graph = grapher.parseFile(config.src, options);
}

return Object.keys(graph.index);
};

watcher.changed = function(absolutePath) {
var files = {
added: [],
changed: [],
removed: [],
};

this.reset();

graph.visitAncestors(absolutePath, function(parent) {
files.changed.push(parent);
});

graph.visitDescendents(absolutePath, function(child) {
files.added.push(child);
});

return files;
};

watcher.added = function(absolutePath) {
var files = {
added: [],
changed: [],
removed: [],
};

this.reset();

if (Object.keys(graph.index).indexOf(absolutePath) !== -1) {
files.added.push(absolutePath);
}

graph.visitDescendents(absolutePath, function(child) {
files.added.push(child);
});

return files;
};

watcher.removed = function(absolutePath) {
var files = {
added: [],
changed: [],
removed: [],
};

graph.visitAncestors(absolutePath, function(parent) {
files.changed.push(parent);
});

if (Object.keys(graph.index).indexOf(absolutePath) !== -1) {
files.removed.push(absolutePath);
}

this.reset();

return files;
};

module.exports = watcher;
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,14 @@
"devDependencies": {
"coveralls": "^2.11.8",
"eslint": "^3.4.0",
"fs-extra": "^0.30.0",
"istanbul": "^0.4.2",
"mocha": "^3.1.2",
"mocha-lcov-reporter": "^1.2.0",
"object-merge": "^2.5.1",
"read-yaml": "^1.0.0",
"rimraf": "^2.5.2",
"sass-spec": "3.5.0-1"
"sass-spec": "3.5.0-1",
"unique-temp-dir": "^1.0.0"
}
}
5 changes: 5 additions & 0 deletions test/fixtures/watcher/main/one.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@import "partials/one";

.one {
color: red;
}
3 changes: 3 additions & 0 deletions test/fixtures/watcher/main/partials/_one.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.one {
color: darkred;
}
3 changes: 3 additions & 0 deletions test/fixtures/watcher/main/partials/_three.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.three {
color: darkgreen;
}
3 changes: 3 additions & 0 deletions test/fixtures/watcher/main/partials/_two.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.two {
color: darkblue;
}
5 changes: 5 additions & 0 deletions test/fixtures/watcher/main/two.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@import "partials/two";

.two {
color: blue;
}
3 changes: 3 additions & 0 deletions test/fixtures/watcher/sibling/partials/_three.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.three {
color: darkgreen;
}
5 changes: 5 additions & 0 deletions test/fixtures/watcher/sibling/three.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@import "partials/three";

.three {
color: green;
}
Loading

0 comments on commit aadf8d4

Please sign in to comment.