Skip to content

Commit 38c0947

Browse files
committed
watch: ensure watch mode detects deleted and re-added files
1 parent 70995bd commit 38c0947

File tree

3 files changed

+133
-9
lines changed

3 files changed

+133
-9
lines changed

lib/internal/watch_mode/files_watcher.js

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ const { TIMEOUT_MAX } = require('internal/timers');
1616

1717
const EventEmitter = require('events');
1818
const { addAbortListener } = require('internal/events/abort_listener');
19-
const { watch } = require('fs');
19+
const { watch, existsSync } = require('fs');
2020
const { fileURLToPath } = require('internal/url');
2121
const { resolve, dirname } = require('path');
22-
const { setTimeout } = require('timers');
22+
const { setTimeout, clearTimeout, setInterval, clearInterval } = require('timers');
2323

2424
const supportsRecursiveWatching = process.platform === 'win32' ||
2525
process.platform === 'darwin';
@@ -31,18 +31,28 @@ class FilesWatcher extends EventEmitter {
3131
#depencencyOwners = new SafeMap();
3232
#ownerDependencies = new SafeMap();
3333
#debounce;
34+
#renameInterval;
35+
#renameTimeout;
3436
#mode;
3537
#signal;
3638
#passthroughIPC = false;
3739
#ipcHandlers = new SafeWeakMap();
3840

39-
constructor({ debounce = 200, mode = 'filter', signal } = kEmptyObject) {
41+
constructor({
42+
debounce = 200,
43+
mode = 'filter',
44+
renameInterval = 1000,
45+
renameTimeout = 60_000,
46+
signal,
47+
} = kEmptyObject) {
4048
super({ __proto__: null, captureRejections: true });
4149

4250
validateNumber(debounce, 'options.debounce', 0, TIMEOUT_MAX);
4351
validateOneOf(mode, 'options.mode', ['filter', 'all']);
4452
this.#debounce = debounce;
4553
this.#mode = mode;
54+
this.#renameInterval = renameInterval;
55+
this.#renameTimeout = renameTimeout;
4656
this.#signal = signal;
4757
this.#passthroughIPC = Boolean(process.send);
4858

@@ -79,7 +89,10 @@ class FilesWatcher extends EventEmitter {
7989
watcher.handle.close();
8090
}
8191

82-
#onChange(trigger) {
92+
#onChange(eventType, trigger, recursive) {
93+
if (eventType === 'rename' && !recursive) {
94+
return this.#rewatch(trigger);
95+
}
8396
if (this.#debouncing.has(trigger)) {
8497
return;
8598
}
@@ -94,6 +107,39 @@ class FilesWatcher extends EventEmitter {
94107
}, this.#debounce).unref();
95108
}
96109

110+
// When a file is removed, wait for it to be re-added.
111+
// Often this re-add is immediate - some editors (e.g., gedit) and some docker mount modes do this.
112+
#rewatch(path) {
113+
if (this.#isPathWatched(path)) {
114+
this.#unwatch(this.#watchers.get(path));
115+
this.#watchers.delete(path);
116+
if (existsSync(path)) {
117+
this.watchPath(path, false);
118+
// This might be redundant. If the file was re-added due to a save event, we will probably see change -> rename.
119+
// However, in certain situations it's entirely possible for the content to have changed after the rename
120+
// In these situations we'd miss the change after the rename event
121+
this.#onChange('change', path, false);
122+
return;
123+
}
124+
let timeout;
125+
126+
// Wait for the file to exist - check every `renameInterval` ms
127+
const interval = setInterval(async () => {
128+
if (existsSync(path)) {
129+
clearInterval(interval);
130+
clearTimeout(timeout);
131+
this.watchPath(path, false);
132+
this.#onChange('change', path, false);
133+
}
134+
}, this.#renameInterval).unref();
135+
136+
// Don't wait forever - after `renameTimeout` ms, stop trying
137+
timeout = setTimeout(() => {
138+
clearInterval(interval);
139+
}, this.#renameTimeout).unref();
140+
}
141+
}
142+
97143
get watchedPaths() {
98144
return [...this.#watchers.keys()];
99145
}
@@ -106,7 +152,7 @@ class FilesWatcher extends EventEmitter {
106152
watcher.on('change', (eventType, fileName) => {
107153
// `fileName` can be `null` if it cannot be determined. See
108154
// https://github.com/nodejs/node/pull/49891#issuecomment-1744673430.
109-
this.#onChange(recursive ? resolve(path, fileName ?? '') : path);
155+
this.#onChange(eventType, recursive ? resolve(path, fileName) : path, recursive);
110156
});
111157
this.#watchers.set(path, { handle: watcher, recursive });
112158
if (recursive) {

test/parallel/test-watch-mode-files_watcher.mjs

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import path from 'node:path';
66
import assert from 'node:assert';
77
import process from 'node:process';
88
import { describe, it, beforeEach, afterEach } from 'node:test';
9-
import { writeFileSync, mkdirSync } from 'node:fs';
9+
import { writeFileSync, mkdirSync, rmSync } from 'node:fs';
1010
import { setTimeout } from 'node:timers/promises';
1111
import { once } from 'node:events';
1212
import { spawn } from 'node:child_process';
@@ -159,4 +159,47 @@ describe('watch mode file watcher', () => {
159159
}
160160
assert.deepStrictEqual(watcher.watchedPaths, expected);
161161
});
162+
163+
it('should capture changes of a renamed file when re-written within the timeout', async () => {
164+
watcher = new FilesWatcher({ debounce: 100, renameInterval: 100, renameTimeout: 400, mode: 'all' });
165+
watcher.on('changed', () => changesCount++);
166+
167+
const file = tmpdir.resolve('file5');
168+
writeFileSync(file, 'changed');
169+
watcher.watchPath(file, false);
170+
171+
let changed = once(watcher, 'changed');
172+
rmSync(file);
173+
await setTimeout(200); // debounce * 2
174+
await changed;
175+
changed = once(watcher, 'changed');
176+
writeFileSync(file, 'changed1');
177+
await setTimeout(200); // debounce * 2
178+
await changed;
179+
changed = once(watcher, 'changed');
180+
writeFileSync(file, 'changed1');
181+
await setTimeout(200); // debounce * 2
182+
await changed;
183+
assert.strictEqual(changesCount, 3);
184+
});
185+
186+
it('should NOT capture changes of a renamed file when re-written after the timeout', async () => {
187+
watcher = new FilesWatcher({ debounce: 100, renameInterval: 200, renameTimeout: 100, mode: 'all' });
188+
watcher.on('changed', () => changesCount++);
189+
190+
const file = tmpdir.resolve('file5');
191+
writeFileSync(file, 'changed');
192+
watcher.watchPath(file, false);
193+
194+
const changed = once(watcher, 'changed');
195+
196+
rmSync(file);
197+
await setTimeout(200); // debounce * 2
198+
await changed;
199+
writeFileSync(file, 'changed1');
200+
await setTimeout(5);
201+
writeFileSync(file, 'changed2');
202+
await setTimeout(5);
203+
assert.strictEqual(changesCount, 1);
204+
});
162205
});

test/sequential/test-watch-mode.mjs

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import path from 'node:path';
55
import { execPath } from 'node:process';
66
import { describe, it } from 'node:test';
77
import { spawn } from 'node:child_process';
8-
import { writeFileSync, readFileSync, mkdirSync } from 'node:fs';
8+
import { writeFileSync, readFileSync, mkdirSync, rmSync } from 'node:fs';
99
import { inspect } from 'node:util';
1010
import { pathToFileURL } from 'node:url';
1111
import { once } from 'node:events';
@@ -38,7 +38,8 @@ async function runWriteSucceed({
3838
completed = 'Completed running',
3939
restarts = 2,
4040
options = {},
41-
shouldFail = false
41+
shouldFail = false,
42+
restartFn = restart
4243
}) {
4344
args.unshift('--no-warnings');
4445
if (watchFlag !== null) args.unshift(watchFlag);
@@ -64,7 +65,7 @@ async function runWriteSucceed({
6465
break;
6566
}
6667
if (completes === 1) {
67-
cancelRestarts = restart(watchedFile);
68+
cancelRestarts = restartFn(watchedFile);
6869
}
6970
}
7071

@@ -655,4 +656,38 @@ process.on('message', (message) => {
655656
`Completed running ${inspect(file)}`,
656657
]);
657658
});
659+
660+
it('should watch changes to removed and readded files', async () => {
661+
const file = createTmpFile();
662+
let restartCount = 0;
663+
const { stderr, stdout } = await runWriteSucceed({
664+
file,
665+
watchedFile: file,
666+
watchFlag: '--watch=true',
667+
options: {
668+
timeout: 10000
669+
},
670+
restarts: 3,
671+
restartFn(fileName) {
672+
const content = readFileSync(fileName);
673+
if (restartCount === 0) {
674+
rmSync(fileName);
675+
}
676+
restartCount++;
677+
return restart(fileName, content);
678+
}
679+
});
680+
681+
assert.strictEqual(stderr, '');
682+
assert.deepStrictEqual(stdout, [
683+
'running',
684+
`Completed running ${inspect(file)}`,
685+
`Restarting ${inspect(file)}`,
686+
'running',
687+
`Completed running ${inspect(file)}`,
688+
`Restarting ${inspect(file)}`,
689+
'running',
690+
`Completed running ${inspect(file)}`,
691+
]);
692+
});
658693
});

0 commit comments

Comments
 (0)