watch: detect files replaced via unlink and create

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);

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);