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.

Sign up for GitHub

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

Jump to bottom

watch mode: use recursive fs.watch #45271

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+80 −135
Open

watch mode: use recursive fs.watch #45271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions doc/api/cli.md
View file
Open in desktop
Expand Up @@ -1624,10 +1624,6 @@ This flag cannot be combined with
$ node --watch-path=./src --watch-path=./tests index.js
```

This option is only supported on macOS and Windows.
An `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM` exception will be thrown
when the option is used on a platform that does not support it.

### `--zero-fill-buffers`

<!-- YAML
Expand Down
11 changes: 0 additions & 11 deletions doc/api/errors.md
View file
Open in desktop
Expand Up @@ -1280,17 +1280,6 @@ for the JS engine are not set up properly.
A `Promise` that was callbackified via `util.callbackify()` was rejected with a
falsy value.

<a id="ERR_FEATURE_UNAVAILABLE_ON_PLATFORM"></a>

### `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM`

<!-- YAML
added: v14.0.0
-->

Used when a feature that is not available
to the current platform which is running Node.js is used.

<a id="ERR_FS_CP_DIR_TO_NON_DIR"></a>

### `ERR_FS_CP_DIR_TO_NON_DIR`
Expand Down
4 changes: 0 additions & 4 deletions lib/internal/errors.js
View file
Open in desktop
Expand Up @@ -1025,10 +1025,6 @@ E('ERR_FALSY_VALUE_REJECTION', function(reason) {
this.reason = reason;
return 'Promise was rejected with falsy value';
}, Error);
E('ERR_FEATURE_UNAVAILABLE_ON_PLATFORM',
'The feature %s is unavailable on the current platform' +
', which is being used to run Node.js',
TypeError);
E('ERR_FS_CP_DIR_TO_NON_DIR',
'Cannot overwrite directory with non-directory', SystemError);
E('ERR_FS_CP_EEXIST', 'Target already exists', SystemError);
Expand Down
29 changes: 8 additions & 21 deletions lib/internal/watch_mode/files_watcher.js
View file
Open in desktop
Expand Up @@ -18,10 +18,6 @@ const { fileURLToPath } = require('url');
const { resolve, dirname } = require('path');
const { setTimeout } = require('timers');


const supportsRecursiveWatching = process.platform === 'win32' ||
process.platform === 'darwin';

class FilesWatcher extends EventEmitter {
#watchers = new SafeMap();
#filteredFiles = new SafeSet();
Expand All @@ -45,8 +41,8 @@ class FilesWatcher extends EventEmitter {
return true;
}

for (const { 0: watchedPath, 1: watcher } of this.#watchers.entries()) {
if (watcher.recursive && StringPrototypeStartsWith(path, watchedPath)) {
for (const watchedPath of this.#watchers.keys()) {
if (StringPrototypeStartsWith(path, watchedPath)) {
return true;
}
}
Expand Down Expand Up @@ -85,28 +81,19 @@ class FilesWatcher extends EventEmitter {
return [...this.#watchers.keys()];
}

watchPath(path, recursive = true) {
watchPath(path) {
if (this.#isPathWatched(path)) {
return;
}
const watcher = watch(path, { recursive });
watcher.on('change', (eventType, fileName) => this
.#onChange(recursive ? resolve(path, fileName) : path));
this.#watchers.set(path, { handle: watcher, recursive });
if (recursive) {
this.#removeWatchedChildren(path);
}
const watcher = watch(path, { recursive: true });
watcher.on('change', (eventType, fileName) => this.#onChange(resolve(path, fileName)));
this.#watchers.set(path, { handle: watcher });
this.#removeWatchedChildren(path);
}

filterFile(file, owner) {
if (!file) return;
if (supportsRecursiveWatching) {
this.watchPath(dirname(file));
} else {
// Having multiple FSWatcher's seems to be slower
// than a single recursive FSWatcher
this.watchPath(file, false);
}
this.watchPath(dirname(file));
this.#filteredFiles.add(file);
if (owner) {
const owners = this.#depencencyOwners.get(file) ?? new SafeSet();
Expand Down
118 changes: 54 additions & 64 deletions test/parallel/test-watch-mode-files_watcher.mjs
View file
Open in desktop
Expand Up @@ -15,8 +15,6 @@ import watcher from 'internal/watch_mode/files_watcher';
if (common.isIBMi)
common.skip('IBMi does not support `fs.watch()`');

const supportsRecursiveWatching = common.isOSX || common.isWindows;

const { FilesWatcher } = watcher;
tmpdir.refresh();

Expand Down Expand Up @@ -70,14 +68,13 @@ describe('watch mode file watcher', () => {
assert.ok(changesCount < 5);
});

it('should ignore files in watched directory if they are not filtered',
{ skip: !supportsRecursiveWatching }, async () => {
watcher.on('changed', common.mustNotCall());
watcher.watchPath(tmpdir.path);
writeFileSync(path.join(tmpdir.path, 'file3'), '1');
// Wait for this long to make sure changes are not triggered
await setTimeout(1000);
});
it('should ignore files in watched directory if they are not filtered', async () => {
watcher.on('changed', common.mustNotCall());
watcher.watchPath(tmpdir.path);
writeFileSync(path.join(tmpdir.path, 'file3'), '1');
// Wait for this long to make sure changes are not triggered
await setTimeout(1000);
MoLow marked this conversation as resolved.
Show resolved Hide resolved
});

it('should allow clearing filters', async () => {
const file = path.join(tmpdir.path, 'file4');
Expand All @@ -95,67 +92,60 @@ describe('watch mode file watcher', () => {
assert.strictEqual(changesCount, 1);
});

it('should watch all files in watched path when in "all" mode',
{ skip: !supportsRecursiveWatching }, async () => {
watcher = new FilesWatcher({ throttle: 100, mode: 'all' });
watcher.on('changed', () => changesCount++);

const file = path.join(tmpdir.path, 'file5');
watcher.watchPath(tmpdir.path);

const changed = once(watcher, 'changed');
writeFileSync(file, 'changed');
await changed;
assert.strictEqual(changesCount, 1);
});

it('should ruse existing watcher if it exists',
{ skip: !supportsRecursiveWatching }, () => {
assert.deepStrictEqual(watcher.watchedPaths, []);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
});

it('should ruse existing watcher of a parent directory',
{ skip: !supportsRecursiveWatching }, () => {
assert.deepStrictEqual(watcher.watchedPaths, []);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
watcher.watchPath(path.join(tmpdir.path, 'subdirectory'));
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
});

it('should remove existing watcher if adding a parent directory watcher',
{ skip: !supportsRecursiveWatching }, () => {
assert.deepStrictEqual(watcher.watchedPaths, []);
const subdirectory = path.join(tmpdir.path, 'subdirectory');
mkdirSync(subdirectory);
watcher.watchPath(subdirectory);
assert.deepStrictEqual(watcher.watchedPaths, [subdirectory]);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
});

it('should clear all watchers when calling clear',
{ skip: !supportsRecursiveWatching }, () => {
assert.deepStrictEqual(watcher.watchedPaths, []);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
watcher.clear();
assert.deepStrictEqual(watcher.watchedPaths, []);
});
it('should watch all files in watched path when in "all" mode', async () => {
watcher = new FilesWatcher({ throttle: 100, mode: 'all' });
watcher.on('changed', () => changesCount++);

const file = path.join(tmpdir.path, 'file5');
watcher.watchPath(tmpdir.path);

const changed = once(watcher, 'changed');
writeFileSync(file, 'changed');
MoLow marked this conversation as resolved.
Show resolved Hide resolved
await changed;
assert.strictEqual(changesCount, 1);
});

it('should ruse existing watcher if it exists', () => {
assert.deepStrictEqual(watcher.watchedPaths, []);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
});

it('should ruse existing watcher of a parent directory', () => {
assert.deepStrictEqual(watcher.watchedPaths, []);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
watcher.watchPath(path.join(tmpdir.path, 'subdirectory'));
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
});

it('should remove existing watcher if adding a parent directory watcher', () => {
assert.deepStrictEqual(watcher.watchedPaths, []);
const subdirectory = path.join(tmpdir.path, 'subdirectory');
mkdirSync(subdirectory);
watcher.watchPath(subdirectory);
assert.deepStrictEqual(watcher.watchedPaths, [subdirectory]);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
});

it('should clear all watchers when calling clear', () => {
assert.deepStrictEqual(watcher.watchedPaths, []);
watcher.watchPath(tmpdir.path);
assert.deepStrictEqual(watcher.watchedPaths, [tmpdir.path]);
watcher.clear();
assert.deepStrictEqual(watcher.watchedPaths, []);
});

it('should watch files from subprocess IPC events', async () => {
const file = fixtures.path('watch-mode/ipc.js');
const child = spawn(process.execPath, [file], { stdio: ['pipe', 'pipe', 'pipe', 'ipc'], encoding: 'utf8' });
watcher.watchChildProcessModules(child);
await once(child, 'exit');
let expected = [file, path.join(tmpdir.path, 'file')];
if (supportsRecursiveWatching) {
expected = expected.map((file) => path.dirname(file));
}
expected = expected.map((file) => path.dirname(file));
assert.deepStrictEqual(watcher.watchedPaths, expected);
});
});
49 changes: 18 additions & 31 deletions test/sequential/test-watch-mode.mjs
View file
Open in desktop
Expand Up @@ -14,8 +14,6 @@ import { createInterface } from 'node:readline/promises';
if (common.isIBMi)
common.skip('IBMi does not support `fs.watch()`');

const supportsRecursive = common.isOSX || common.isWindows;

function restart(file) {
// To avoid flakiness, we save the file repeatedly until test is done
writeFileSync(file, readFileSync(file));
Expand Down Expand Up @@ -125,9 +123,7 @@ describe('watch mode', { concurrency: true, timeout: 60_000 }, () => {
});
});

it('should watch when running an non-existing file - when specified under --watch-path', {
skip: !supportsRecursive
}, async () => {
it('should watch when running an non-existing file - when specified under --watch-path', async () => {
const file = fixtures.path('watch-mode/subdir/non-existing.js');
const watchedFile = fixtures.path('watch-mode/subdir/file.js');
const { stderr, stdout } = await spawnWithRestarts({
Expand Down Expand Up @@ -234,38 +230,29 @@ describe('watch mode', { concurrency: true, timeout: 60_000 }, () => {
});
});

// TODO: Remove skip after https://github.com/nodejs/node/pull/45271 lands
it('should not watch when running an missing file', {
skip: !supportsRecursive
}, async () => {
it('should not watch when running an missing file', async () => {
const nonExistingfile = path.join(tmpdir.path, `${tmpFiles++}.js`);
await failWriteSucceed({ file: nonExistingfile, watchedFile: nonExistingfile });
});

it('should not watch when running an missing mjs file', {
skip: !supportsRecursive
}, async () => {
const nonExistingfile = path.join(tmpdir.path, `${tmpFiles++}.mjs`);
await failWriteSucceed({ file: nonExistingfile, watchedFile: nonExistingfile });
});
// it('should not watch when running an missing mjs file', async () => {
// const nonExistingfile = path.join(tmpdir.path, `${tmpFiles++}.mjs`);
// await failWriteSucceed({ file: nonExistingfile, watchedFile: nonExistingfile });
// });

it('should watch changes to previously missing dependency', {
skip: !supportsRecursive
}, async () => {
const dependency = path.join(tmpdir.path, `${tmpFiles++}.js`);
const relativeDependencyPath = `./${path.basename(dependency)}`;
const dependant = createTmpFile(`console.log(require('${relativeDependencyPath}'))`);
// it('should watch changes to previously missing dependency', async () => {
// const dependency = path.join(tmpdir.path, `${tmpFiles++}.js`);
// const relativeDependencyPath = `./${path.basename(dependency)}`;
// const dependant = createTmpFile(`console.log(require('${relativeDependencyPath}'))`);

await failWriteSucceed({ file: dependant, watchedFile: dependency });
});
// await failWriteSucceed({ file: dependant, watchedFile: dependency });
// });

it('should watch changes to previously missing ESM dependency', {
skip: !supportsRecursive
}, async () => {
const dependency = path.join(tmpdir.path, `${tmpFiles++}.mjs`);
const relativeDependencyPath = `./${path.basename(dependency)}`;
const dependant = createTmpFile(`import '${relativeDependencyPath}'`, '.mjs');
// it('should watch changes to previously missing ESM dependency', async () => {
// const dependency = path.join(tmpdir.path, `${tmpFiles++}.mjs`);
// const relativeDependencyPath = `./${path.basename(dependency)}`;
// const dependant = createTmpFile(`import '${relativeDependencyPath}'`, '.mjs');

await failWriteSucceed({ file: dependant, watchedFile: dependency });
});
// await failWriteSucceed({ file: dependant, watchedFile: dependency });
// });
});