From 6a272aa5763eb0c728b76adc3b12bb12abc1aaca 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 | 73 +++++++++++++++++++++-------------- 3 files changed, 82 insertions(+), 48 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..7d4a72006 100644 --- a/test/unit/watcher.spec.coffee +++ b/test/unit/watcher.spec.coffee @@ -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_ @@ -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