Skip to content

Commit 7eacb74

Browse files
lholmquistTrott
authored andcommitted
fs: make FSWatcher.start private
* This is a semver-major change to rename the FSWatcher.start function to FSWatcher._start to make it private The motivation here is that it serves no purpose to the end user. An instance of FSWatcher is returned when a user calls fs.watch, which will call the start method. A user can't create an instance of a FSWatcher directly. If the start method is called by a user it is a noop since the watcher has already started. Calling start after a watcher has closed is also a noop PR-URL: #29905 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 40ef537 commit 7eacb74

File tree

3 files changed

+14
-20
lines changed

3 files changed

+14
-20
lines changed

lib/fs.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -1342,10 +1342,10 @@ function watch(filename, options, listener) {
13421342
if (!watchers)
13431343
watchers = require('internal/fs/watchers');
13441344
const watcher = new watchers.FSWatcher();
1345-
watcher.start(filename,
1346-
options.persistent,
1347-
options.recursive,
1348-
options.encoding);
1345+
watcher[watchers.kFSWatchStart](filename,
1346+
options.persistent,
1347+
options.recursive,
1348+
options.encoding);
13491349

13501350
if (listener) {
13511351
watcher.addListener('change', listener);

lib/internal/fs/watchers.js

+10-9
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ const assert = require('internal/assert');
2525
const kOldStatus = Symbol('kOldStatus');
2626
const kUseBigint = Symbol('kUseBigint');
2727

28+
const kFSWatchStart = Symbol('kFSWatchStart');
29+
2830
function emitStop(self) {
2931
self.emit('stop');
3032
}
@@ -135,18 +137,16 @@ function FSWatcher() {
135137
Object.setPrototypeOf(FSWatcher.prototype, EventEmitter.prototype);
136138
Object.setPrototypeOf(FSWatcher, EventEmitter);
137139

138-
139-
// FIXME(joyeecheung): this method is not documented.
140140
// At the moment if filename is undefined, we
141-
// 1. Throw an Error if it's the first time .start() is called
142-
// 2. Return silently if .start() has already been called
141+
// 1. Throw an Error if it's the first time Symbol('kFSWatchStart') is called
142+
// 2. Return silently if Symbol('kFSWatchStart') has already been called
143143
// on a valid filename and the wrap has been initialized
144144
// 3. Return silently if the watcher has already been closed
145145
// This method is a noop if the watcher has already been started.
146-
FSWatcher.prototype.start = function(filename,
147-
persistent,
148-
recursive,
149-
encoding) {
146+
FSWatcher.prototype[kFSWatchStart] = function(filename,
147+
persistent,
148+
recursive,
149+
encoding) {
150150
if (this._handle === null) { // closed
151151
return;
152152
}
@@ -202,5 +202,6 @@ Object.defineProperty(FSEvent.prototype, 'owner', {
202202

203203
module.exports = {
204204
FSWatcher,
205-
StatWatcher
205+
StatWatcher,
206+
kFSWatchStart
206207
};

test/parallel/test-fs-watch.js

-7
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,6 @@ for (const testCase of cases) {
5757
});
5858
watcher.on('close', common.mustCall(() => {
5959
watcher.close(); // Closing a closed watcher should be a noop
60-
// Starting a closed watcher should be a noop
61-
watcher.start();
6260
}));
6361
watcher.on('change', common.mustCall(function(eventType, argFilename) {
6462
if (interval) {
@@ -71,16 +69,11 @@ for (const testCase of cases) {
7169
assert.strictEqual(eventType, 'change');
7270
assert.strictEqual(argFilename, testCase.fileName);
7371

74-
// Starting a started watcher should be a noop
75-
watcher.start();
76-
watcher.start(pathToWatch);
77-
7872
watcher.close();
7973

8074
// We document that watchers cannot be used anymore when it's closed,
8175
// here we turn the methods into noops instead of throwing
8276
watcher.close(); // Closing a closed watcher should be a noop
83-
watcher.start(); // Starting a closed watcher should be a noop
8477
}));
8578

8679
// Long content so it's actually flushed. toUpperCase so there's real change.

0 commit comments

Comments
 (0)