Commit 3641c36af1e for nodejs
commit 3641c36af1e99d47d0b90a9e86143153efdc54a8
Author: Marco <marco.piraccini@gmail.com>
Date: Sun Sep 27 11:36:07 2026 +0200
watch: detect files replaced via unlink and create
Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/63888
Fixes: https://github.com/nodejs/node/issues/51621
Refs: https://github.com/nodejs/node/issues/54774
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js
index c87f489b637..8b5faeaf0e9 100644
--- a/lib/internal/watch_mode/files_watcher.js
+++ b/lib/internal/watch_mode/files_watcher.js
@@ -25,6 +25,9 @@ const { setTimeout, clearTimeout } = require('timers');
const supportsRecursiveWatching = process.platform === 'win32' ||
process.platform === 'darwin';
+const supportsDirectoryWatching = supportsRecursiveWatching ||
+ process.platform === 'linux';
+
const isParentPath = (parentCandidate, childCandidate) => {
const parent = resolve(parentCandidate);
const child = resolve(childCandidate);
@@ -117,13 +120,15 @@ class FilesWatcher extends EventEmitter {
if (this.#isPathWatched(path)) {
return;
}
- const { allowMissing = false } = options;
+ // `watchEntries` tells that `path` is a directory whose entries are
+ // reported by name, as opposed to a single watched file.
+ const { allowMissing = false, watchEntries = recursive } = options;
const watcher = watch(path, { recursive, signal: this.#signal, throwIfNoEntry: !allowMissing });
watcher.on('change', (eventType, fileName) => {
// `fileName` can be `null` if it cannot be determined. See
// https://github.com/nodejs/node/pull/49891#issuecomment-1744673430.
- this.#onChange(recursive ? resolve(path, fileName ?? '') : path, eventType);
+ this.#onChange(watchEntries ? resolve(path, fileName ?? '') : path, eventType);
});
this.#watchers.set(path, { handle: watcher, recursive });
if (recursive) {
@@ -135,9 +140,15 @@ class FilesWatcher extends EventEmitter {
if (!file) return;
if (supportsRecursiveWatching) {
this.watchPath(dirname(file), true, options);
+ } else if (supportsDirectoryWatching) {
+ // Watch the parent directory non-recursively rather than the file
+ // itself. A watch bound to the file's inode stops receiving events once
+ // the file is replaced via unlink+create or rename (atomic saves, Docker
+ // Compose watch, ...), so only the first replacement would be detected.
+ // Watching the directory keeps working across replacements, and unrelated
+ // siblings are discarded by the `filter` mode check in `#onChange`.
+ this.watchPath(dirname(file), false, { __proto__: null, ...options, watchEntries: true });
} else {
- // Having multiple FSWatcher's seems to be slower
- // than a single recursive FSWatcher
this.watchPath(file, false, options);
}
this.#filteredFiles.add(file);
diff --git a/test/parallel/test-watch-mode-files_watcher.mjs b/test/parallel/test-watch-mode-files_watcher.mjs
index e1595350cd0..10e078286e0 100644
--- a/test/parallel/test-watch-mode-files_watcher.mjs
+++ b/test/parallel/test-watch-mode-files_watcher.mjs
@@ -6,7 +6,7 @@ import path from 'node:path';
import assert from 'node:assert';
import process from 'node:process';
import { describe, it, beforeEach, afterEach } from 'node:test';
-import { writeFileSync, mkdirSync, appendFileSync } from 'node:fs';
+import { writeFileSync, mkdirSync, appendFileSync, rmSync } from 'node:fs';
import { createInterface } from 'node:readline';
import { setTimeout } from 'node:timers/promises';
import { once } from 'node:events';
@@ -17,6 +17,9 @@ if (common.isIBMi)
common.skip('IBMi does not support `fs.watch()`');
const supportsRecursiveWatching = common.isMacOS || common.isWindows;
+// Elsewhere a directory watch does not report changes to its entries by name,
+// so the files are watched directly.
+const watchesParentDirectory = supportsRecursiveWatching || common.isLinux;
const { FilesWatcher } = watcher;
tmpdir.refresh();
@@ -44,6 +47,19 @@ describe('watch mode file watcher', () => {
});
}
+ function replaceAndWaitForChanges(watcher, file) {
+ return new Promise((resolve) => {
+ const interval = setInterval(() => {
+ rmSync(file, { force: true });
+ writeFileSync(file, `replace ${counter++}`);
+ }, 100);
+ watcher.once('changed', () => {
+ clearInterval(interval);
+ resolve();
+ });
+ });
+ }
+
it('should watch changed files', async () => {
const file = tmpdir.resolve('file1');
writeFileSync(file, 'written');
@@ -52,6 +68,19 @@ describe('watch mode file watcher', () => {
assert.strictEqual(changesCount, 1);
});
+ it('should keep detecting files replaced via unlink and create', { skip: !watchesParentDirectory }, async () => {
+ // Regression test for https://github.com/nodejs/node/issues/51621: a watch
+ // bound to the file inode stops firing after the first replacement, so the
+ // second `replaceAndWaitForChanges` call would hang on the buggy behavior.
+ const file = tmpdir.resolve('replaced.js');
+ writeFileSync(file, 'written');
+ watcher.filterFile(file);
+ await replaceAndWaitForChanges(watcher, file);
+ await replaceAndWaitForChanges(watcher, file);
+ await replaceAndWaitForChanges(watcher, file);
+ assert.ok(changesCount >= 3, `expected at least 3 changes, got ${changesCount}`);
+ });
+
it('should watch changed files with same prefix path string', async () => {
mkdirSync(tmpdir.resolve('subdir'));
mkdirSync(tmpdir.resolve('sub'));
@@ -205,7 +234,9 @@ describe('watch mode file watcher', () => {
watcher.watchChildProcessModules(child);
await once(child, 'exit');
let expected = [file, tmpdir.resolve('file')];
- if (supportsRecursiveWatching) {
+ if (watchesParentDirectory) {
+ // The parent directory is watched so that files replaced via
+ // unlink+create or rename are still detected.
expected = expected.map((file) => path.dirname(file));
}
assert.deepStrictEqual(watcher.watchedPaths, expected);