-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Decouple the graph and render logic from the fs watcher #2020
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'); | ||
|
@@ -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] !== '_') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could move this logic into |
||
renderFile(file, options, emitter); | ||
} | ||
}); | ||
|
||
files.removed.forEach(function(file) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be
but it's not likely the next watcher would have the same API |
||
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); | ||
} | ||
}); | ||
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)); | ||
}); | ||
} | ||
|
||
|
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; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,12 +75,14 @@ | |
"devDependencies": { | ||
"coveralls": "^2.11.8", | ||
"eslint": "^3.4.0", | ||
"fs-extra": "^0.30.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a super older version but it's the last version that supports Node 0.10. It's only for tests so I'm not too fussed. |
||
"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" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
@import "partials/one"; | ||
|
||
.one { | ||
color: red; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
.one { | ||
color: darkred; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
.three { | ||
color: darkgreen; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
.two { | ||
color: darkblue; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
@import "partials/two"; | ||
|
||
.two { | ||
color: blue; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
.three { | ||
color: darkgreen; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
@import "partials/three"; | ||
|
||
.three { | ||
color: green; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Abstract was probably the wrong word, but here is a crude example example
So the
watcher
module would export certain functions required in the binThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I'm still not following.
If I'm understanding correctly wouldn't that then couple us to the fs watcher implementation's api?
What happens if the next fs watcher doesn't expose a method like
watch()
for exposing all currently watched files?The
watch.indexOf(file) === -1
is an optimisation to prevent callinggaze.add
superfluously. Under the coversgaze.add
also does deduping but there's not guarantee the next fs watcher would.