Skip to content

Commit

Permalink
fix(watcher): improve watching efficiency
Browse files Browse the repository at this point in the history
This fix significantly improves watching performance. It tells chokidar to
ignore all paths that do not match any of the watched patterns.

Before we were only ignoring paths that match some of the excludes patterns.
That means that even files that didn't match any of the watched patterns were
still watched by chokidar and ignored on the level of `fileList`.

Note, this fix requires chokidar 0.7.0

Closes #616
  • Loading branch information
vojtajina committed Oct 22, 2013
1 parent ee8303c commit 6a272aa
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 48 deletions.
55 changes: 36 additions & 19 deletions lib/watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,7 @@ var watchPatterns = function(patterns, watcher) {
var uniqueMap = {};
var path;

patterns.forEach(function(patternObject) {
var pattern = patternObject.pattern;

if (!patternObject.watched) {
return;
}

patterns.forEach(function(pattern) {
path = baseDirFromPattern(pattern);
if (!uniqueMap[path]) {
uniqueMap[path] = true;
Expand All @@ -42,27 +36,50 @@ var watchPatterns = function(patterns, watcher) {
});
};

// Function to test if an item is on the exclude list
// and therefore should not be watched by chokidar
// TODO(vojta): ignore non-matched files as well
var createIgnore = function(excludes) {
return function(item) {
var matchExclude = function(pattern) {
log.debug('Excluding %s', pattern);
return mm(item, pattern, {dot: true});
};
return excludes.some(matchExclude);
// Function to test if a path should be ignored by chokidar.
var createIgnore = function(patterns, excludes) {
return function(path, stat) {
if (!stat || stat.isDirectory()) {
return false;
}

// Check if the path matches any of the watched patterns.
if (!patterns.some(function(pattern) {
return mm(path, pattern, {dot: true});
})) {
return true;
}

// Check if the path matches any of the exclude patterns.
if (excludes.some(function(pattern) {
return mm(path, pattern, {dot: true});
})) {
return true;
}

return false;
};
};

var onlyWatchedTrue = function(pattern) {
return pattern.watched;
};

var getWatchedPatterns = function(patternObjects) {
return patternObjects.filter(onlyWatchedTrue).map(function(patternObject) {
return patternObject.pattern;
});
};

exports.watch = function(patterns, excludes, fileList) {
var watchedPatterns = getWatchedPatterns(patterns);
var options = {
ignorePermissionErrors: true,
ignored: createIgnore(excludes)
ignored: createIgnore(watchedPatterns, excludes)
};
var chokidarWatcher = new chokidar.FSWatcher(options);

watchPatterns(patterns, chokidarWatcher);
watchPatterns(watchedPatterns, chokidarWatcher);

var bind = function(fn) {
return function(path) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
"dependencies": {
"di": "~0.0.1",
"socket.io": "~0.9.13",
"chokidar": "~0.6",
"chokidar": "~0.7.0",
"glob": "~3.1.21",
"minimatch": "~0.2",
"http-proxy": "~0.10",
Expand Down
73 changes: 45 additions & 28 deletions test/unit/watcher.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ describe 'watcher', ->
config = require '../../lib/config'
m = null

# create an array of pattern objects from given strings
patterns = (strings...) ->
new config.createPatternObject(str) for str in strings

beforeEach ->
mocks_ = chokidar: mocks.chokidar
m = mocks.loadFile __dirname + '/../../lib/watcher.js', mocks_
Expand Down Expand Up @@ -43,55 +39,76 @@ describe 'watcher', ->
chokidarWatcher = new mocks.chokidar.FSWatcher

it 'should watch all the patterns', ->
m.watchPatterns patterns('/some/*.js', '/a/*'), chokidarWatcher
m.watchPatterns ['/some/*.js', '/a/*'], chokidarWatcher
expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some', '/a']


it 'should not watch urls', ->
m.watchPatterns patterns('http://some.com', '/a.*'), chokidarWatcher
expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/']


it 'should not watch the same path twice', ->
m.watchPatterns patterns('/some/a*.js', '/some/*.txt'), chokidarWatcher
m.watchPatterns ['/some/a*.js', '/some/*.txt'], chokidarWatcher
expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some']


it 'should not watch subpaths that are already watched', ->
m.watchPatterns patterns('/some/sub/*.js', '/some/a*.*'), chokidarWatcher
m.watchPatterns ['/some/sub/*.js', '/some/a*.*'], chokidarWatcher
expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some']


it 'should watch a file matching subpath directory', ->
# regression #521
m.watchPatterns patterns('/some/test-file.js', '/some/test/**'), chokidarWatcher
m.watchPatterns ['/some/test-file.js', '/some/test/**'], chokidarWatcher
expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some/test-file.js', '/some/test']


it 'should not watch if watched false', ->
m.watchPatterns [
new config.Pattern('/some/*.js', true, true, false)
new config.Pattern('/some/sub/*.js')
], chokidarWatcher
describe 'getWatchedPatterns', ->

expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some/sub']
it 'should return list of watched patterns (strings)', ->
watchedPatterns = m.getWatchedPatterns [
config.createPatternObject('/watched.js')
config.createPatternObject(pattern: 'non/*.js', watched: false)
]
expect(watchedPatterns).to.deep.equal ['/watched.js']


#============================================================================
# ignore() [PRIVATE]
#============================================================================
describe 'ignore', ->
FILE_STAT =
isDirectory: -> false
isFile: -> true
DIRECTORY_STAT =
isDirectory: -> true
isFile: -> false

it 'should ignore all files', ->
ignore = m.createIgnore ['**/*']
expect(ignore '/some/files/deep/nested.js').to.equal true
expect(ignore '/some/files').to.equal true
ignore = m.createIgnore ['**/*'], ['**/*']
expect(ignore '/some/files/deep/nested.js', FILE_STAT).to.equal true
expect(ignore '/some/files', FILE_STAT).to.equal true


it 'should ignore .# files', ->
ignore = m.createIgnore ['**/.#*']
expect(ignore '/some/files/deep/nested.js').to.equal false
expect(ignore '/some/files').to.equal false
expect(ignore '/some/files/deep/.npm').to.equal false
expect(ignore '.#files.js').to.equal true
expect(ignore '/some/files/deeper/nested/.#files.js').to.equal true
ignore = m.createIgnore ['**/*'], ['**/.#*']
expect(ignore '/some/files/deep/nested.js', FILE_STAT).to.equal false
expect(ignore '/some/files', FILE_STAT).to.equal false
expect(ignore '/some/files/deep/.npm', FILE_STAT).to.equal false
expect(ignore '.#files.js', FILE_STAT).to.equal true
expect(ignore '/some/files/deeper/nested/.#files.js', FILE_STAT).to.equal true


it 'should ignore files that do not match any pattern', ->
ignore = m.createIgnore ['/some/*.js'], []
expect(ignore '/a.js', FILE_STAT).to.equal true
expect(ignore '/some.js', FILE_STAT).to.equal true
expect(ignore '/some/a.js', FILE_STAT).to.equal false


it 'should not ignore directories', ->
ignore = m.createIgnore ['**/*'], ['**/*']
expect(ignore '/some/dir', DIRECTORY_STAT).to.equal false


it 'should not ignore items without stat', ->
# before we know whether it's a directory or file, we can't ignore
ignore = m.createIgnore ['**/*'], ['**/*']
expect(ignore '/some.js', undefined).to.equal false
expect(ignore '/what/ever', undefined).to.equal false

0 comments on commit 6a272aa

Please sign in to comment.