From 6fb75a19d475936ee7c4f79f0da693b32834f8b7 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 13 Jun 2026 12:45:11 +0200 Subject: [PATCH] watch: detect files replaced via unlink and create Signed-off-by: marcopiraccini --- lib/internal/watch_mode/files_watcher.js | 14 +++++--- .../test-watch-mode-files_watcher.mjs | 35 ++++++++++++++++--- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index c94775b3e6e4bc..bcd88f8340fd44 100644 --- a/lib/internal/watch_mode/files_watcher.js +++ b/lib/internal/watch_mode/files_watcher.js @@ -120,7 +120,9 @@ class FilesWatcher extends EventEmitter { 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); + // `path` is the watched directory (see `filterFile`), so resolve the + // changed entry against it to get the absolute path of the trigger. + this.#onChange(resolve(path, fileName ?? ''), eventType); }); this.#watchers.set(path, { handle: watcher, recursive }); if (recursive) { @@ -133,9 +135,13 @@ class FilesWatcher extends EventEmitter { if (supportsRecursiveWatching) { this.watchPath(dirname(file), true, options); } else { - // Having multiple FSWatcher's seems to be slower - // than a single recursive FSWatcher - this.watchPath(file, false, options); + // 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, options); } this.#filteredFiles.add(file); if (owner) { diff --git a/test/parallel/test-watch-mode-files_watcher.mjs b/test/parallel/test-watch-mode-files_watcher.mjs index e1595350cd0f3e..e9995a7498b612 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'; @@ -44,6 +44,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 +65,19 @@ describe('watch mode file watcher', () => { assert.strictEqual(changesCount, 1); }); + it('should keep detecting files replaced via unlink and create', 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')); @@ -204,10 +230,9 @@ describe('watch mode file watcher', () => { const child = spawn(process.execPath, [file], { stdio: ['pipe', 'pipe', 'pipe', 'ipc'], encoding: 'utf8' }); watcher.watchChildProcessModules(child); await once(child, 'exit'); - let expected = [file, tmpdir.resolve('file')]; - if (supportsRecursiveWatching) { - expected = expected.map((file) => path.dirname(file)); - } + // The parent directory is watched on every platform so that files replaced + // via unlink+create or rename are still detected. + const expected = [file, tmpdir.resolve('file')].map((file) => path.dirname(file)); assert.deepStrictEqual(watcher.watchedPaths, expected); }); });