From 7f4c116422def3f52aad5197e305c92267367de5 Mon Sep 17 00:00:00 2001 From: Lucas Holmquist Date: Mon, 14 Oct 2019 11:57:24 -0400 Subject: [PATCH 1/4] 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 --- lib/fs.js | 3 ++- lib/internal/fs/watchers.js | 14 +++++++++----- test/parallel/test-fs-watchfile-bigint.js | 2 -- test/parallel/test-fs-watchfile.js | 2 -- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 1b3df1119f3aa5..e6bcf5ed07ba87 100644 --- 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 index c974f40aae104f..18ae8e4553103c 100644 --- 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; @@ -209,5 +212,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 index 500a0ef1f64b1b..569c7ad6f57fe4 100644 --- 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 index 0b28e0331d3f91..7b3ad4a26db6b5 100644 --- 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) { From 0aef19777c750322e589d7ccf72e3bb34614c00c Mon Sep 17 00:00:00 2001 From: Lucas Holmquist Date: Mon, 28 Oct 2019 12:17:01 -0400 Subject: [PATCH 2/4] squash: adding back in the start method, but as a stub --- lib/internal/fs/watchers.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js index 18ae8e4553103c..b340acd4095adf 100644 --- a/lib/internal/fs/watchers.js +++ b/lib/internal/fs/watchers.js @@ -91,6 +91,8 @@ StatWatcher.prototype[kFSStatWatcherStart] = function(filename, } }; +StatWatcher.prototype.start = () => {}; + // FIXME(joyeecheung): this method is not documented while there is // another documented fs.unwatchFile(). The counterpart in // FSWatcher is .close() From 1ec46c65bf472cb234cc69423ed18fa111fa0625 Mon Sep 17 00:00:00 2001 From: Lucas Holmquist Date: Tue, 29 Oct 2019 14:22:19 -0400 Subject: [PATCH 3/4] squash: add a comment for future maintainers --- lib/internal/fs/watchers.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js index b340acd4095adf..fda24fa78c9adb 100644 --- a/lib/internal/fs/watchers.js +++ b/lib/internal/fs/watchers.js @@ -91,6 +91,10 @@ StatWatcher.prototype[kFSStatWatcherStart] = function(filename, } }; +// To maximize backword-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 From 7e03961f0cfde9aeaed4ab0c5bfdcd0ffe61fb2c Mon Sep 17 00:00:00 2001 From: Lucas Holmquist Date: Tue, 29 Oct 2019 14:41:14 -0400 Subject: [PATCH 4/4] squash: slight spelling and grammer update --- lib/internal/fs/watchers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js index fda24fa78c9adb..0685bcef4dff58 100644 --- a/lib/internal/fs/watchers.js +++ b/lib/internal/fs/watchers.js @@ -91,10 +91,10 @@ StatWatcher.prototype[kFSStatWatcherStart] = function(filename, } }; -// To maximize backword-compatiblity for the end user +// 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 +// This should not be documented. StatWatcher.prototype.start = () => {}; // FIXME(joyeecheung): this method is not documented while there is