From 727a8d3f959426fda076050c989c0823d1e2d06b Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 13 Jun 2026 12:45:11 +0200 Subject: [PATCH 1/2] 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 c87f489b6377..d2399f958e91 100644 --- a/lib/internal/watch_mode/files_watcher.js +++ b/lib/internal/watch_mode/files_watcher.js @@ -123,7 +123,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) { @@ -136,9 +138,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 e1595350cd0f..e9995a7498b6 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); }); }); From f8e34b10f1cf29f0133b92083c0e321811f290f9 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 19 Sep 2026 12:08:42 +0200 Subject: [PATCH 2/2] watch: only watch the parent directory where entries are reported Signed-off-by: marcopiraccini --- lib/internal/watch_mode/files_watcher.js | 17 +++++++++++------ test/parallel/test-watch-mode-files_watcher.mjs | 14 ++++++++++---- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index d2399f958e91..8b5faeaf0e9d 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,15 +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. - // `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.#onChange(watchEntries ? resolve(path, fileName ?? '') : path, eventType); }); this.#watchers.set(path, { handle: watcher, recursive }); if (recursive) { @@ -137,14 +140,16 @@ class FilesWatcher extends EventEmitter { if (!file) return; if (supportsRecursiveWatching) { this.watchPath(dirname(file), true, options); - } else { + } 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, options); + this.watchPath(dirname(file), false, { __proto__: null, ...options, watchEntries: true }); + } else { + this.watchPath(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 e9995a7498b6..10e078286e07 100644 --- a/test/parallel/test-watch-mode-files_watcher.mjs +++ b/test/parallel/test-watch-mode-files_watcher.mjs @@ -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(); @@ -65,7 +68,7 @@ describe('watch mode file watcher', () => { assert.strictEqual(changesCount, 1); }); - it('should keep detecting files replaced via unlink and create', async () => { + 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. @@ -230,9 +233,12 @@ 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'); - // 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)); + let expected = [file, tmpdir.resolve('file')]; + 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); }); });