Skip to content

Commit 31d7d42

Browse files
committed
Decouple the graph and render logic from the fs watcher
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 (sass#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 sass#1896 Fixes sass#1891
1 parent ae4f935 commit 31d7d42

File tree

11 files changed

+328
-46
lines changed

11 files changed

+328
-46
lines changed

bin/node-sass

Lines changed: 24 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ var Emitter = require('events').EventEmitter,
1010
glob = require('glob'),
1111
sass = require('../lib'),
1212
render = require('../lib/render'),
13+
watcher = require('../lib/watcher'),
1314
stdout = require('stdout-stream'),
1415
stdin = require('get-stdin'),
1516
fs = require('fs');
@@ -229,63 +230,41 @@ function getOptions(args, options) {
229230
*/
230231

231232
function watch(options, emitter) {
232-
var buildGraph = function(options) {
233-
var graph;
234-
var graphOptions = {
235-
loadPaths: options.includePath,
236-
extensions: ['scss', 'sass', 'css']
237-
};
238-
239-
if (options.directory) {
240-
graph = grapher.parseDir(options.directory, graphOptions);
241-
} else {
242-
graph = grapher.parseFile(options.src, graphOptions);
243-
}
233+
var handler = function(files) {
234+
files.added.forEach(function(file) {
235+
var watch = gaze.watched();
236+
if (watch.indexOf(file) === -1) {
237+
gaze.add(file);
238+
}
239+
});
244240

245-
return graph;
241+
files.changed.forEach(function(file) {
242+
if (path.basename(file)[0] !== '_') {
243+
renderFile(file, options, emitter);
244+
}
245+
});
246+
247+
files.removed.forEach(function(file) {
248+
gaze.remove(file);
249+
});
246250
};
247251

248-
var watch = [];
249-
var graph = buildGraph(options);
250-
251-
// Add all files to watch list
252-
for (var i in graph.index) {
253-
watch.push(i);
254-
}
252+
watcher.reset(options);
255253

256254
var gaze = new Gaze();
257-
gaze.add(watch);
255+
gaze.add(watcher.reset(options));
258256
gaze.on('error', emitter.emit.bind(emitter, 'error'));
259257

260258
gaze.on('changed', function(file) {
261-
var files = [file];
262-
263-
// descendents may be added, so we need a new graph
264-
graph = buildGraph(options);
265-
graph.visitAncestors(file, function(parent) {
266-
files.push(parent);
267-
});
268-
269-
// Add children to watcher
270-
graph.visitDescendents(file, function(child) {
271-
if (watch.indexOf(child) === -1) {
272-
watch.push(child);
273-
gaze.add(child);
274-
}
275-
});
276-
files.forEach(function(file) {
277-
if (path.basename(file)[0] !== '_') {
278-
renderFile(file, options, emitter);
279-
}
280-
});
259+
handler(watcher.changed(file));
281260
});
282261

283-
gaze.on('added', function() {
284-
graph = buildGraph(options);
262+
gaze.on('added', function(file) {
263+
handler(watcher.added(file));
285264
});
286265

287-
gaze.on('deleted', function() {
288-
graph = buildGraph(options);
266+
gaze.on('deleted', function(file) {
267+
handler(watcher.deleted(file));
289268
});
290269
}
291270

lib/watcher.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
var grapher = require('sass-graph'),
2+
clonedeep = require('lodash.clonedeep'),
3+
watcher = {
4+
opts: {}
5+
},
6+
graph = null;
7+
8+
watcher.reset = function(opts) {
9+
this.opts = clonedeep(opts || this.opts || {});
10+
var options = {
11+
loadPaths: this.opts.includePath,
12+
extensions: ['scss', 'sass', 'css']
13+
};
14+
15+
if (this.opts.directory) {
16+
graph = grapher.parseDir(this.opts.directory, options);
17+
} else {
18+
graph = grapher.parseFile(this.opts.src, options);
19+
}
20+
21+
return Object.keys(graph.index);
22+
};
23+
24+
watcher.changed = function(absolutePath) {
25+
var files = {
26+
added: [],
27+
changed: [],
28+
removed: [],
29+
};
30+
31+
this.reset();
32+
33+
graph.visitAncestors(absolutePath, function(parent) {
34+
files.changed.push(parent);
35+
});
36+
37+
graph.visitDescendents(absolutePath, function(child) {
38+
files.added.push(child);
39+
});
40+
41+
return files;
42+
};
43+
44+
watcher.added = function(absolutePath) {
45+
var files = {
46+
added: [],
47+
changed: [],
48+
removed: [],
49+
};
50+
51+
this.reset();
52+
53+
if (Object.keys(graph.index).indexOf(absolutePath) !== -1) {
54+
files.added.push(absolutePath);
55+
}
56+
57+
graph.visitDescendents(absolutePath, function(child) {
58+
files.added.push(child);
59+
});
60+
61+
return files;
62+
};
63+
64+
watcher.removed = function(absolutePath) {
65+
var files = {
66+
added: [],
67+
changed: [],
68+
removed: [],
69+
};
70+
71+
graph.visitAncestors(absolutePath, function(parent) {
72+
files.changed.push(parent);
73+
});
74+
75+
if (Object.keys(graph.index).indexOf(absolutePath) !== -1) {
76+
files.removed.push(absolutePath);
77+
}
78+
79+
this.reset();
80+
81+
return files;
82+
};
83+
84+
module.exports = watcher;

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,14 @@
7575
"devDependencies": {
7676
"coveralls": "^2.11.8",
7777
"eslint": "^3.4.0",
78+
"fs-extra": "^0.30.0",
7879
"istanbul": "^0.4.2",
7980
"mocha": "^3.1.2",
8081
"mocha-lcov-reporter": "^1.2.0",
8182
"object-merge": "^2.5.1",
8283
"read-yaml": "^1.0.0",
8384
"rimraf": "^2.5.2",
84-
"sass-spec": "3.5.0-1"
85+
"sass-spec": "3.5.0-1",
86+
"tempy": "^0.1.0"
8587
}
8688
}

test/fixtures/watcher/main/one.scss

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
@import "partials/one";
2+
3+
.one {
4+
color: red;
5+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.one {
2+
color: darkred;
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.three {
2+
color: darkgreen;
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.two {
2+
color: darkblue;
3+
}

test/fixtures/watcher/main/two.scss

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
@import "partials/two";
2+
3+
.two {
4+
color: blue;
5+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
.three {
2+
color: darkgreen;
3+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
@import "partials/three";
2+
3+
.three {
4+
color: green;
5+
}

0 commit comments

Comments
 (0)