Skip to content
Permalink
Browse files

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>
  • Loading branch information...
lholmquist authored and Trott committed Oct 9, 2019
1 parent 40ef537 commit 7eacb7438960da98eaf210e232ea93a98e247b72
Showing with 14 additions and 20 deletions.
  1. +4 −4 lib/fs.js
  2. +10 −9 lib/internal/fs/watchers.js
  3. +0 −7 test/parallel/test-fs-watch.js
@@ -1342,10 +1342,10 @@ function watch(filename, options, listener) {
if (!watchers)
watchers = require('internal/fs/watchers');
const watcher = new watchers.FSWatcher();
watcher.start(filename,
options.persistent,
options.recursive,
options.encoding);
watcher[watchers.kFSWatchStart](filename,
options.persistent,
options.recursive,
options.encoding);

if (listener) {
watcher.addListener('change', listener);
@@ -25,6 +25,8 @@ const assert = require('internal/assert');
const kOldStatus = Symbol('kOldStatus');
const kUseBigint = Symbol('kUseBigint');

const kFSWatchStart = Symbol('kFSWatchStart');

function emitStop(self) {
self.emit('stop');
}
@@ -135,18 +137,16 @@ function FSWatcher() {
Object.setPrototypeOf(FSWatcher.prototype, EventEmitter.prototype);
Object.setPrototypeOf(FSWatcher, EventEmitter);


// 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('kFSWatchStart') is called
// 2. Return silently if Symbol('kFSWatchStart') has already been called
// on a valid filename and the wrap has been initialized
// 3. Return silently if the watcher has already been closed
// This method is a noop if the watcher has already been started.
FSWatcher.prototype.start = function(filename,
persistent,
recursive,
encoding) {
FSWatcher.prototype[kFSWatchStart] = function(filename,
persistent,
recursive,
encoding) {
if (this._handle === null) { // closed
return;
}
@@ -202,5 +202,6 @@ Object.defineProperty(FSEvent.prototype, 'owner', {

module.exports = {
FSWatcher,
StatWatcher
StatWatcher,
kFSWatchStart
};
@@ -57,8 +57,6 @@ for (const testCase of cases) {
});
watcher.on('close', common.mustCall(() => {
watcher.close(); // Closing a closed watcher should be a noop
// Starting a closed watcher should be a noop
watcher.start();
}));
watcher.on('change', common.mustCall(function(eventType, argFilename) {
if (interval) {
@@ -71,16 +69,11 @@ for (const testCase of cases) {
assert.strictEqual(eventType, 'change');
assert.strictEqual(argFilename, testCase.fileName);

// Starting a started watcher should be a noop
watcher.start();
watcher.start(pathToWatch);

watcher.close();

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

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

0 comments on commit 7eacb74

Please sign in to comment.
You can’t perform that action at this time.