Skip to content

Commit

Permalink
watch: use recursive fs.watch
Browse files Browse the repository at this point in the history
  • Loading branch information
@MoLow
MoLow committed Nov 27, 2022
1 parent 147d810 commit 03e801d
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 135 deletions.
4 changes: 0 additions & 4 deletions doc/api/cli.md
View file
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
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
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
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
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);
});

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');
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
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 });
// });
});

0 comments on commit 03e801d

Please sign in to comment.