diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index c87f489b6377..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,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 e1595350cd0f..10e078286e07 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);