Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: make FSWatcher.start private #29905

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 10 additions & 9 deletions lib/internal/fs/watchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -202,5 +202,6 @@ Object.defineProperty(FSEvent.prototype, 'owner', {

module.exports = {
FSWatcher,
StatWatcher
StatWatcher,
kFSWatchStart
};
7 changes: 0 additions & 7 deletions test/parallel/test-fs-watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand Down