Skip to content

Commit

Permalink
watch: ensure watch mode detects deleted and re-added files
Browse files Browse the repository at this point in the history
  • Loading branch information
znewsham committed May 7, 2024
1 parent f202322 commit 95a9603
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 9 deletions.
56 changes: 51 additions & 5 deletions lib/internal/watch_mode/files_watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ const { TIMEOUT_MAX } = require('internal/timers');

const EventEmitter = require('events');
const { addAbortListener } = require('internal/events/abort_listener');
const { watch } = require('fs');
const { watch, existsSync } = require('fs');
const { fileURLToPath } = require('internal/url');
const { resolve, dirname } = require('path');
const { setTimeout } = require('timers');
const { setTimeout, clearTimeout, setInterval, clearInterval } = require('timers');

const supportsRecursiveWatching = process.platform === 'win32' ||
process.platform === 'darwin';
Expand All @@ -29,16 +29,26 @@ class FilesWatcher extends EventEmitter {
#depencencyOwners = new SafeMap();
#ownerDependencies = new SafeMap();
#debounce;
#renameInterval;
#renameTimeout;
#mode;
#signal;

constructor({ debounce = 200, mode = 'filter', signal } = kEmptyObject) {
constructor({
debounce = 200,
mode = 'filter',
renameInterval = 1000,
renameTimeout = 60_000,
signal,
} = kEmptyObject) {
super({ __proto__: null, captureRejections: true });

validateNumber(debounce, 'options.debounce', 0, TIMEOUT_MAX);
validateOneOf(mode, 'options.mode', ['filter', 'all']);
this.#debounce = debounce;
this.#mode = mode;
this.#renameInterval = renameInterval;
this.#renameTimeout = renameTimeout;
this.#signal = signal;

if (signal) {
Expand Down Expand Up @@ -74,7 +84,10 @@ class FilesWatcher extends EventEmitter {
watcher.handle.close();
}

#onChange(trigger) {
#onChange(eventType, trigger, recursive) {
if (eventType === 'rename' && !recursive) {
return this.#rewatch(trigger);
}
if (this.#debouncing.has(trigger)) {
return;
}
Expand All @@ -89,6 +102,39 @@ class FilesWatcher extends EventEmitter {
}, this.#debounce).unref();
}

// When a file is removed, wait for it to be re-added.
// Often this re-add is immediate - some editors (e.g., gedit) and some docker mount modes do this.
#rewatch(path) {
if (this.#isPathWatched(path)) {
this.#unwatch(this.#watchers.get(path));
this.#watchers.delete(path);
if (existsSync(path)) {
this.watchPath(path, false);
// This might be redundant. If the file was re-added due to a save event, we will probably see change -> rename.
// However, in certain situations it's entirely possible for the content to have changed after the rename
// In these situations we'd miss the change after the rename event
this.#onChange('change', path, false);
return;
}
let timeout;

// Wait for the file to exist - check every `renameInterval` ms
const interval = setInterval(async () => {
if (existsSync(path)) {
clearInterval(interval);
clearTimeout(timeout);
this.watchPath(path, false);
this.#onChange('change', path, false);
}
}, this.#renameInterval).unref();

// Don't wait forever - after `renameTimeout` ms, stop trying
timeout = setTimeout(() => {
clearInterval(interval);
}, this.#renameTimeout).unref();
}
}

get watchedPaths() {
return [...this.#watchers.keys()];
}
Expand All @@ -101,7 +147,7 @@ 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);
this.#onChange(eventType, recursive ? resolve(path, fileName) : path, recursive);
});
this.#watchers.set(path, { handle: watcher, recursive });
if (recursive) {
Expand Down
41 changes: 40 additions & 1 deletion test/parallel/test-watch-mode-files_watcher.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 } from 'node:fs';
import { writeFileSync, mkdirSync, rmSync } from 'node:fs';
import { setTimeout } from 'node:timers/promises';
import { once } from 'node:events';
import { spawn } from 'node:child_process';
Expand Down Expand Up @@ -159,4 +159,43 @@ describe('watch mode file watcher', () => {
}
assert.deepStrictEqual(watcher.watchedPaths, expected);
});

it('should capture changes of a renamed file when re-written within the timeout', async () => {
watcher = new FilesWatcher({ debounce: 1, renameInterval: 10, renameTimeout: 20, mode: 'all' });
watcher.on('changed', () => changesCount++);

const file = tmpdir.resolve('file5');
writeFileSync(file, 'changed');
watcher.watchPath(file, false);

let changed = once(watcher, 'changed');
rmSync(file);
await changed;
changed = once(watcher, 'changed');
writeFileSync(file, 'changed1');
await changed;
changed = once(watcher, 'changed');
writeFileSync(file, 'changed1');
await changed;
assert.strictEqual(changesCount, 3);
});

it('should NOT capture changes of a renamed file when re-written after the timeout', async () => {
watcher = new FilesWatcher({ debounce: 1, renameInterval: 20, renameTimeout: 10, mode: 'all' });
watcher.on('changed', () => changesCount++);

const file = tmpdir.resolve('file5');
writeFileSync(file, 'changed');
watcher.watchPath(file, false);

const changed = once(watcher, 'changed');

rmSync(file);
await changed;
writeFileSync(file, 'changed1');
await setTimeout(5);
writeFileSync(file, 'changed2');
await setTimeout(5);
assert.strictEqual(changesCount, 1);
});
});
41 changes: 38 additions & 3 deletions test/sequential/test-watch-mode.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import path from 'node:path';
import { execPath } from 'node:process';
import { describe, it } from 'node:test';
import { spawn } from 'node:child_process';
import { writeFileSync, readFileSync, mkdirSync } from 'node:fs';
import { writeFileSync, readFileSync, mkdirSync, rmSync } from 'node:fs';
import { inspect } from 'node:util';
import { pathToFileURL } from 'node:url';
import { createInterface } from 'node:readline';
Expand Down Expand Up @@ -37,7 +37,8 @@ async function runWriteSucceed({
completed = 'Completed running',
restarts = 2,
options = {},
shouldFail = false
shouldFail = false,
restartFn = restart
}) {
args.unshift('--no-warnings');
if (watchFlag !== null) args.unshift(watchFlag);
Expand All @@ -63,7 +64,7 @@ async function runWriteSucceed({
break;
}
if (completes === 1) {
cancelRestarts = restart(watchedFile);
cancelRestarts = restartFn(watchedFile);
}
}

Expand Down Expand Up @@ -574,4 +575,38 @@ console.log(values.random);
`Completed running ${inspect(file)}`,
]);
});

it('should watch changes to removed and readded files', async () => {
const file = createTmpFile();
let restartCount = 0;
const { stderr, stdout } = await runWriteSucceed({
file,
watchedFile: file,
watchFlag: '--watch=true',
options: {
timeout: 10000
},
restarts: 3,
restartFn(fileName) {
const content = readFileSync(fileName);
if (restartCount === 0) {
rmSync(fileName);
}
restartCount++;
return restart(fileName, content);
}
});

assert.strictEqual(stderr, '');
assert.deepStrictEqual(stdout, [
'running',
`Completed running ${inspect(file)}`,
`Restarting ${inspect(file)}`,
'running',
`Completed running ${inspect(file)}`,
`Restarting ${inspect(file)}`,
'running',
`Completed running ${inspect(file)}`,
]);
});
});

0 comments on commit 95a9603

Please sign in to comment.