From b491b2c072e8e259e4e32b1385f45a20141f04df Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Mon, 21 Oct 2013 15:16:55 -0700 Subject: [PATCH] fix(watcher): improve watching efficiency 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 --- lib/watcher.js | 55 +++++++++++++++++++---------- package.json | 2 +- test/unit/watcher.spec.coffee | 66 ++++++++++++++++++++++------------- 3 files changed, 79 insertions(+), 44 deletions(-) diff --git a/lib/watcher.js b/lib/watcher.js index a6055bb70..232ffa81d 100644 --- a/lib/watcher.js +++ b/lib/watcher.js @@ -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; @@ -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) { diff --git a/package.json b/package.json index 48f127a95..0c2c5a10b 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/test/unit/watcher.spec.coffee b/test/unit/watcher.spec.coffee index 55ed85a54..88f4742a0 100644 --- a/test/unit/watcher.spec.coffee +++ b/test/unit/watcher.spec.coffee @@ -43,55 +43,73 @@ 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 patterns('/watched.js', {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