Skip to content

Commit fbf6b2d

Browse files
watch: only watch the parent directory where entries are reported
Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>
1 parent 6fb75a1 commit fbf6b2d

2 files changed

Lines changed: 21 additions & 10 deletions

File tree

‎lib/internal/watch_mode/files_watcher.js‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ const { setTimeout, clearTimeout } = require('timers');
2525
const supportsRecursiveWatching = process.platform === 'win32' ||
2626
process.platform === 'darwin';
2727

28+
const supportsDirectoryWatching = supportsRecursiveWatching ||
29+
process.platform === 'linux';
30+
2831
const isParentPath = (parentCandidate, childCandidate) => {
2932
const parent = resolve(parentCandidate);
3033
const child = resolve(childCandidate);
@@ -114,15 +117,15 @@ class FilesWatcher extends EventEmitter {
114117
if (this.#isPathWatched(path)) {
115118
return;
116119
}
117-
const { allowMissing = false } = options;
120+
// `watchEntries` tells that `path` is a directory whose entries are
121+
// reported by name, as opposed to a single watched file.
122+
const { allowMissing = false, watchEntries = recursive } = options;
118123

119124
const watcher = watch(path, { recursive, signal: this.#signal, throwIfNoEntry: !allowMissing });
120125
watcher.on('change', (eventType, fileName) => {
121126
// `fileName` can be `null` if it cannot be determined. See
122127
// https://github.com/nodejs/node/pull/49891#issuecomment-1744673430.
123-
// `path` is the watched directory (see `filterFile`), so resolve the
124-
// changed entry against it to get the absolute path of the trigger.
125-
this.#onChange(resolve(path, fileName ?? ''), eventType);
128+
this.#onChange(watchEntries ? resolve(path, fileName ?? '') : path, eventType);
126129
});
127130
this.#watchers.set(path, { handle: watcher, recursive });
128131
if (recursive) {
@@ -134,14 +137,16 @@ class FilesWatcher extends EventEmitter {
134137
if (!file) return;
135138
if (supportsRecursiveWatching) {
136139
this.watchPath(dirname(file), true, options);
137-
} else {
140+
} else if (supportsDirectoryWatching) {
138141
// Watch the parent directory non-recursively rather than the file
139142
// itself. A watch bound to the file's inode stops receiving events once
140143
// the file is replaced via unlink+create or rename (atomic saves, Docker
141144
// Compose watch, ...), so only the first replacement would be detected.
142145
// Watching the directory keeps working across replacements, and unrelated
143146
// siblings are discarded by the `filter` mode check in `#onChange`.
144-
this.watchPath(dirname(file), false, options);
147+
this.watchPath(dirname(file), false, { __proto__: null, ...options, watchEntries: true });
148+
} else {
149+
this.watchPath(file, false, options);
145150
}
146151
this.#filteredFiles.add(file);
147152
if (owner) {

‎test/parallel/test-watch-mode-files_watcher.mjs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ if (common.isIBMi)
1717
common.skip('IBMi does not support `fs.watch()`');
1818

1919
const supportsRecursiveWatching = common.isMacOS || common.isWindows;
20+
// Elsewhere a directory watch does not report changes to its entries by name,
21+
// so the files are watched directly.
22+
const watchesParentDirectory = supportsRecursiveWatching || common.isLinux;
2023

2124
const { FilesWatcher } = watcher;
2225
tmpdir.refresh();
@@ -65,7 +68,7 @@ describe('watch mode file watcher', () => {
6568
assert.strictEqual(changesCount, 1);
6669
});
6770

68-
it('should keep detecting files replaced via unlink and create', async () => {
71+
it('should keep detecting files replaced via unlink and create', { skip: !watchesParentDirectory }, async () => {
6972
// Regression test for https://github.com/nodejs/node/issues/51621: a watch
7073
// bound to the file inode stops firing after the first replacement, so the
7174
// second `replaceAndWaitForChanges` call would hang on the buggy behavior.
@@ -230,9 +233,12 @@ describe('watch mode file watcher', () => {
230233
const child = spawn(process.execPath, [file], { stdio: ['pipe', 'pipe', 'pipe', 'ipc'], encoding: 'utf8' });
231234
watcher.watchChildProcessModules(child);
232235
await once(child, 'exit');
233-
// The parent directory is watched on every platform so that files replaced
234-
// via unlink+create or rename are still detected.
235-
const expected = [file, tmpdir.resolve('file')].map((file) => path.dirname(file));
236+
let expected = [file, tmpdir.resolve('file')];
237+
if (watchesParentDirectory) {
238+
// The parent directory is watched so that files replaced via
239+
// unlink+create or rename are still detected.
240+
expected = expected.map((file) => path.dirname(file));
241+
}
236242
assert.deepStrictEqual(watcher.watchedPaths, expected);
237243
});
238244
});

0 commit comments

Comments
 (0)