Skip to content

Commit 535e957

Browse files
lholmquistaddaleax
authored andcommitted
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]>
1 parent 4f43418 commit 535e957

File tree

4 files changed

+17
-10
lines changed

4 files changed

+17
-10
lines changed

lib/fs.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -1386,7 +1386,8 @@ function watchFile(filename, options, listener) {
13861386
if (!watchers)
13871387
watchers = require('internal/fs/watchers');
13881388
stat = new watchers.StatWatcher(options.bigint);
1389-
stat.start(filename, options.persistent, options.interval);
1389+
stat[watchers.kFSStatWatcherStart](filename,
1390+
options.persistent, options.interval);
13901391
statWatchers.set(filename, stat);
13911392
}
13921393

lib/internal/fs/watchers.js

+15-5
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const kOldStatus = Symbol('kOldStatus');
2626
const kUseBigint = Symbol('kUseBigint');
2727

2828
const kFSWatchStart = Symbol('kFSWatchStart');
29+
const kFSStatWatcherStart = Symbol('kFSStatWatcherStart');
2930

3031
function emitStop(self) {
3132
self.emit('stop');
@@ -54,13 +55,15 @@ function onchange(newStatus, stats) {
5455
getStatsFromBinding(stats, kFsStatsFieldsNumber));
5556
}
5657

57-
// FIXME(joyeecheung): this method is not documented.
5858
// At the moment if filename is undefined, we
59-
// 1. Throw an Error if it's the first time .start() is called
60-
// 2. Return silently if .start() has already been called
59+
// 1. Throw an Error if it's the first
60+
// time Symbol('kFSStatWatcherStart') is called
61+
// 2. Return silently if Symbol('kFSStatWatcherStart') has already been called
6162
// on a valid filename and the wrap has been initialized
6263
// This method is a noop if the watcher has already been started.
63-
StatWatcher.prototype.start = function(filename, persistent, interval) {
64+
StatWatcher.prototype[kFSStatWatcherStart] = function(filename,
65+
persistent,
66+
interval) {
6467
if (this._handle !== null)
6568
return;
6669

@@ -88,6 +91,12 @@ StatWatcher.prototype.start = function(filename, persistent, interval) {
8891
}
8992
};
9093

94+
// To maximize backward-compatiblity for the end user,
95+
// a no-op stub method has been added instead of
96+
// totally removing StatWatcher.prototpye.start.
97+
// This should not be documented.
98+
StatWatcher.prototype.start = () => {};
99+
91100
// FIXME(joyeecheung): this method is not documented while there is
92101
// another documented fs.unwatchFile(). The counterpart in
93102
// FSWatcher is .close()
@@ -209,5 +218,6 @@ Object.defineProperty(FSEvent.prototype, 'owner', {
209218
module.exports = {
210219
FSWatcher,
211220
StatWatcher,
212-
kFSWatchStart
221+
kFSWatchStart,
222+
kFSStatWatcherStart
213223
};

test/parallel/test-fs-watchfile-bigint.js

-2
Original file line numberDiff line numberDiff line change
@@ -67,5 +67,3 @@ const watcher =
6767
// 'stop' should only be emitted once - stopping a stopped watcher should
6868
// not trigger a 'stop' event.
6969
watcher.on('stop', common.mustCall(function onStop() {}));
70-
71-
watcher.start(); // Starting a started watcher should be a noop

test/parallel/test-fs-watchfile.js

-2
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ const watcher =
8282
// not trigger a 'stop' event.
8383
watcher.on('stop', common.mustCall(function onStop() {}));
8484

85-
watcher.start(); // Starting a started watcher should be a noop
86-
8785
// Watch events should callback with a filename on supported systems.
8886
// Omitting AIX. It works but not reliably.
8987
if (common.isLinux || common.isOSX || common.isWindows) {

0 commit comments

Comments
 (0)