fs: make FSStatWatcher.start private

An instance of FSStatWatcher is returned when a user calls fs.watchFile, which will call the start method. A user can't create an instance of a FSStatWatcher directly. If the start method is called by a user it is a noop since the watcher has already started. This "Class" is currently undocumented. PR-URL: #29971 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
nodejs · Nov 19, 2019 · 535e957 · 535e957
1 parent 4f43418
commit 535e957
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 10 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -1386,7 +1386,8 @@ function watchFile(filename, options, listener) {
     if (!watchers)
       watchers = require('internal/fs/watchers');
     stat = new watchers.StatWatcher(options.bigint);
-    stat.start(filename, options.persistent, options.interval);
+    stat[watchers.kFSStatWatcherStart](filename,
+                                       options.persistent, options.interval);
     statWatchers.set(filename, stat);
   }
 

diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js
@@ -26,6 +26,7 @@ const kOldStatus = Symbol('kOldStatus');
 const kUseBigint = Symbol('kUseBigint');
 
 const kFSWatchStart = Symbol('kFSWatchStart');
+const kFSStatWatcherStart = Symbol('kFSStatWatcherStart');
 
 function emitStop(self) {
   self.emit('stop');
@@ -54,13 +55,15 @@ function onchange(newStatus, stats) {
             getStatsFromBinding(stats, kFsStatsFieldsNumber));
 }
 
-// FIXME(joyeecheung): this method is not documented.
 // At the moment if filename is undefined, we
-// 1. Throw an Error if it's the first time .start() is called
-// 2. Return silently if .start() has already been called
+// 1. Throw an Error if it's the first
+//    time Symbol('kFSStatWatcherStart') is called
+// 2. Return silently if Symbol('kFSStatWatcherStart') has already been called
 //    on a valid filename and the wrap has been initialized
 // This method is a noop if the watcher has already been started.
-StatWatcher.prototype.start = function(filename, persistent, interval) {
+StatWatcher.prototype[kFSStatWatcherStart] = function(filename,
+                                                      persistent,
+                                                      interval) {
   if (this._handle !== null)
     return;
 
@@ -88,6 +91,12 @@ StatWatcher.prototype.start = function(filename, persistent, interval) {
   }
 };
 
+// To maximize backward-compatiblity for the end user,
+// a no-op stub method has been added instead of
+// totally removing StatWatcher.prototpye.start.
+// This should not be documented.
+StatWatcher.prototype.start = () => {};
+
 // FIXME(joyeecheung): this method is not documented while there is
 // another documented fs.unwatchFile(). The counterpart in
 // FSWatcher is .close()
@@ -209,5 +218,6 @@ Object.defineProperty(FSEvent.prototype, 'owner', {
 module.exports = {
   FSWatcher,
   StatWatcher,
-  kFSWatchStart
+  kFSWatchStart,
+  kFSStatWatcherStart
 };
diff --git a/test/parallel/test-fs-watchfile-bigint.js b/test/parallel/test-fs-watchfile-bigint.js
@@ -67,5 +67,3 @@ const watcher =
 // 'stop' should only be emitted once - stopping a stopped watcher should
 // not trigger a 'stop' event.
 watcher.on('stop', common.mustCall(function onStop() {}));
-
-watcher.start();  // Starting a started watcher should be a noop
diff --git a/test/parallel/test-fs-watchfile.js b/test/parallel/test-fs-watchfile.js
@@ -82,8 +82,6 @@ const watcher =
 // not trigger a 'stop' event.
 watcher.on('stop', common.mustCall(function onStop() {}));
 
-watcher.start();  // Starting a started watcher should be a noop
-
 // Watch events should callback with a filename on supported systems.
 // Omitting AIX. It works but not reliably.
 if (common.isLinux || common.isOSX || common.isWindows) {