Refactor underlying watcher close handling

in preparation for `unwatch` method

lib/fsevents-handler.js

@@ -30,7 +30,7 @@ function createFSEventsInstance(path, callback) {
 // * listener   - function, called when fsevents emits events
 // * rawEmitter - function, passes data to listeners of the 'raw' event
 
-// Returns object with a close function
+// Returns close function
 function setFSEventsListener(path, realPath, listener, rawEmitter) {
   var watchPath = sysPath.extname(path) ? sysPath.dirname(path) : path;
   var watchContainer;
@@ -76,17 +76,17 @@ function setFSEventsListener(path, realPath, listener, rawEmitter) {
     };
   }
   var listenerIndex = watchContainer.listeners.length - 1;
-  return {
-    // removes this instance's listeners and closes the underlying fsevents
-    // instance if there are no more listeners left
-    close: function() {
-      delete watchContainer.listeners[listenerIndex];
-      if (!Object.keys(watchContainer.listeners).length) {
-        watchContainer.watcher.stop();
-        delete FSEventsWatchers[watchPath];
-      }
+
+  // removes this instance's listeners and closes the underlying fsevents
+  // instance if there are no more listeners left
+  return function close() {
+    delete watchContainer.listeners[listenerIndex];
+    delete watchContainer.rawEmitters[listenerIndex];
+    if (!Object.keys(watchContainer.listeners).length) {
+      watchContainer.watcher.stop();
+      delete FSEventsWatchers[watchPath];
     }
-  };
+  }
 }
 
 // returns boolean indicating whether fsevents can be used
@@ -112,7 +112,7 @@ function FsEventsHandler() {}
 // * transform  - function, path transformer
 // * globFilter - function, path filter in case a glob pattern was provided
 
-// Returns nothing
+// Returns close function for the watcher instance
 FsEventsHandler.prototype._watchWithFsEvents =
 function(watchPath, realPath, transform, globFilter) {
   if (this._isIgnored(watchPath)) return;
@@ -196,15 +196,15 @@ function(watchPath, realPath, transform, globFilter) {
     }
   }.bind(this);
 
-  var watcher = setFSEventsListener(
+  var closer = setFSEventsListener(
     watchPath,
     realPath,
     watchCallback,
     this.emit.bind(this, 'raw')
   );
 
   this._emitReady();
-  return watcher;
+  return closer;
 };
 
 // Private method: Handle symlinks encountered during directory scan
@@ -315,16 +315,13 @@ function(path, transform, forceAdd, priorDepth) {
 
   if (this.options.persistent) {
     var initWatch = function(error, realPath) {
-      var watcher = this._watchWithFsEvents(
+      var closer = this._watchWithFsEvents(
         wh.watchPath,
         sysPath.resolve(realPath || wh.watchPath),
         processPath,
         wh.globFilter
       );
-      if (watcher) {
-        watcher.userPath = path;
-        this._watchers.push(watcher);
-      }
+      if (closer) this._closers[path] = closer;
     }.bind(this);
 
     if (typeof transform === 'function') {

lib/index.js

@@ -33,7 +33,7 @@ function FSWatcher(_opts) {
   // in case _opts that is passed in is a frozen object
   if (_opts) for (var opt in _opts) opts[opt] = _opts[opt];
   this._watched = Object.create(null);
-  this._watchers = [];
+  this._closers = Object.create(null);
   this._ignoredPaths = Object.create(null);
   this._globIgnored = [];
   this.closed = false;
@@ -366,9 +366,10 @@ FSWatcher.prototype.close = function() {
   if (this.closed) return this;
 
   this.closed = true;
-  this._watchers.forEach(function(watcher) {
-    watcher.close();
-  });
+  Object.keys(this._closers).forEach(function(watchPath) {
+    this._closers[watchPath]();
+    delete this._closers[watchPath];
+  }, this);
   this._watched = Object.create(null);
 
   this.removeAllListeners();

lib/nodefs-handler.js

@@ -50,7 +50,7 @@ function createFsWatchInstance(path, options, listener, errHandler, emitRaw) {
 // * type       - string, listener type
 // * val[1..3]  - arguments to be passed to listeners
 
-// Returns object with a close function
+// Returns nothing
 function fsWatchBroadcast(fullPath, type, val1, val2, val3) {
   if (!FsWatchInstances[fullPath]) return;
   FsWatchInstances[fullPath][type].forEach(function(listener) {
@@ -66,7 +66,7 @@ function fsWatchBroadcast(fullPath, type, val1, val2, val3) {
 // * options    - object, options to be passed to fs.watch
 // * handlers   - object, container for event listener functions
 
-// Returns object with a close function
+// Returns close function
 function setFsWatchListener(path, fullPath, options, handlers) {
   var listener = handlers.listener;
   var errHandler = handlers.errHandler;
@@ -109,14 +109,16 @@ function setFsWatchListener(path, fullPath, options, handlers) {
     container.rawEmitters.push(rawEmitter);
   }
   var listenerIndex = container.listeners.length - 1;
-  return {
-    close: function() {
-      delete container.listeners[listenerIndex];
-      delete container.errHandlers[listenerIndex];
-      if (!Object.keys(container.listeners).length) {
-        container.watcher.close();
-        delete FsWatchInstances[fullPath];
-      }
+
+  // removes this instance's listeners and closes the underlying fs.watch
+  // instance if there are no more listeners left
+  return function close() {
+    delete container.listeners[listenerIndex];
+    delete container.errHandlers[listenerIndex];
+    delete container.rawEmitters[listenerIndex];
+    if (!Object.keys(container.listeners).length) {
+      container.watcher.close();
+      delete FsWatchInstances[fullPath];
     }
   };
 }
@@ -135,7 +137,7 @@ var FsWatchFileInstances = Object.create(null);
 // * options    - object, options to be passed to fs.watchFile
 // * handlers   - object, container for event listener functions
 
-// Returns object with a close function
+// Returns close function
 function setFsWatchFileListener(path, fullPath, options, handlers) {
   var listener = handlers.listener;
   var rawEmitter = handlers.rawEmitter;
@@ -178,17 +180,20 @@ function setFsWatchFileListener(path, fullPath, options, handlers) {
     };
   } else {
     container.listeners.push(listener);
+    container.rawEmitters.push(rawEmitter);
   }
   var listenerIndex = container.listeners.length - 1;
-  return {
-    close: function() {
-      delete container.listeners[listenerIndex];
-      if (!Object.keys(container.listeners).length) {
-        fs.unwatchFile(fullPath);
-        delete FsWatchFileInstances[fullPath];
-      }
+
+  // removes this instance's listeners and closes the underlying fs.watchFile
+  // instance if there are no more listeners left
+  return function close() {
+    delete container.listeners[listenerIndex];
+    delete container.rawEmitters[listenerIndex];
+    if (!Object.keys(container.listeners).length) {
+      fs.unwatchFile(fullPath);
+      delete FsWatchFileInstances[fullPath];
     }
-  };
+  }
 }
 
 // fake constructor for attaching nodefs-specific prototype methods that
@@ -200,7 +205,7 @@ function NodeFsHandler() {}
 // * path     - string, path to file or directory.
 // * listener - function, to be executed on fs change.
 
-// Returns nothing
+// Returns close function for the watcher instance
 NodeFsHandler.prototype._watchWithNodeFs =
 function(path, listener) {
   var directory = sysPath.dirname(path);
@@ -212,22 +217,22 @@ function(path, listener) {
   var options = {persistent: this.options.persistent};
   if (!listener) listener = Function.prototype; // empty function
 
-  var watcher;
+  var closer;
   if (this.options.usePolling) {
     options.interval = this.enableBinaryInterval && isBinaryPath(basename) ?
       this.options.binaryInterval : this.options.interval;
-    watcher = setFsWatchFileListener(path, absolutePath, options, {
+    closer = setFsWatchFileListener(path, absolutePath, options, {
       listener: listener,
       rawEmitter: this.emit.bind(this, 'raw')
     });
   } else {
-    watcher = setFsWatchListener(path, absolutePath, options, {
+    closer = setFsWatchListener(path, absolutePath, options, {
       listener: listener,
       errHandler: this._handleError.bind(this),
       rawEmitter: this.emit.bind(this, 'raw')
     });
   }
-  if (watcher) this._watchers.push(watcher);
+  return closer;
 };
 
 // Private method: Watch a file and emit add event if warranted
@@ -237,7 +242,7 @@ function(path, listener) {
 // * initialAdd - boolean, was the file added at watch instantiation?
 // * callback   - function, called when done processing as a newly seen file
 
-// Returns nothing
+// Returns close function for the watcher instance
 NodeFsHandler.prototype._handleFile =
 function(file, stats, initialAdd, callback) {
   var dirname = sysPath.dirname(file);
@@ -248,7 +253,7 @@ function(file, stats, initialAdd, callback) {
   if (parent.has(basename)) return;
 
   // kick off the watcher
-  this._watchWithNodeFs(file, function(path, newStats) {
+  var closer = this._watchWithNodeFs(file, function(path, newStats) {
     if (!this._throttle('watch', file, 5)) return;
     if (!newStats || newStats && newStats.mtime.getTime() === 0) {
       fs.stat(file, function(error, newStats) {
@@ -272,6 +277,7 @@ function(file, stats, initialAdd, callback) {
   }
 
   if (callback) callback();
+  return closer;
 };
 
 // Private method: Handle symlinks encountered while reading a dir
@@ -322,7 +328,7 @@ function(entry, directory, path, item) {
 // * wh         - object, common watch helpers for this path
 // * callback   - function, called when dir scan is complete
 
-// Returns nothing
+// Returns close function for the watcher instance
 NodeFsHandler.prototype._handleDir =
 function(dir, stats, initialAdd, depth, target, wh, callback) {
   if (!(initialAdd && this.options.ignoreInitial) && !target && !wh.hasGlob) {
@@ -382,7 +388,7 @@ function(dir, stats, initialAdd, depth, target, wh, callback) {
 
   if (this.options.depth === undefined || depth <= this.options.depth) {
     if (!target) read(dir, initialAdd, callback);
-    this._watchWithNodeFs(dir, function(dirPath, stats) {
+    var closer = this._watchWithNodeFs(dir, function(dirPath, stats) {
       // if current directory is removed, do nothing
       if (stats && stats.mtime.getTime() === 0) return;
 
@@ -391,6 +397,7 @@ function(dir, stats, initialAdd, depth, target, wh, callback) {
   } else {
     callback();
   }
+  return closer;
 };
 
 // Private method: Handle added file, directory, or glob pattern.
@@ -406,9 +413,9 @@ function(dir, stats, initialAdd, depth, target, wh, callback) {
 NodeFsHandler.prototype._addToNodeFs =
 function(path, initialAdd, priorWh, depth, target, callback) {
   if (!callback) callback = Function.prototype;
-  var emitReady = this._emitReady;
+  var ready = this._emitReady;
   if (this._isIgnored(path) || this.closed) {
-    emitReady();
+    ready();
     return callback(null, false);
   }
 
@@ -425,30 +432,33 @@ function(path, initialAdd, priorWh, depth, target, callback) {
     var permError = this.options.ignorePermissionErrors &&
       !this._hasReadPermissions(stats);
     if (permError || this._isIgnored(wh.watchPath, stats)) {
-      emitReady();
+      ready();
       return callback(null, false);
     }
 
     var initDir = function(dir, target) {
-      this._handleDir(dir, stats, initialAdd, depth, target, wh, emitReady);
+      return this._handleDir(dir, stats, initialAdd, depth, target, wh, ready);
     }.bind(this);
 
+    var closer;
     if (stats.isDirectory()) {
-      initDir(wh.watchPath, target);
+      closer = initDir(wh.watchPath, target);
     } else if (stats.isSymbolicLink()) {
       var parent = sysPath.dirname(wh.watchPath);
       this._getWatchedDir(parent).add(wh.watchPath);
       this._emit('add', wh.watchPath, stats);
-      initDir(parent, path);
+      closer = initDir(parent, path);
 
       // preserve this symlink's target path
       fs.realpath(path, function(error, targetPath) {
         this._symlinkPaths[sysPath.resolve(path)] = targetPath;
-        emitReady();
+        ready();
       }.bind(this));
     } else {
-      this._handleFile(wh.watchPath, stats, initialAdd, this._emitReady);
+      closer = this._handleFile(wh.watchPath, stats, initialAdd, ready);
     }
+
+    if (closer) this._closers[path] = closer;
     callback(null, false);
   }.bind(this));
 };