fix: report a directory that appeared while no watcher listened to the watcher attaching next

.changeset/directory-outdated-on-attach.md

@@ -0,0 +1,5 @@
+---
+"watchpack": patch
+---
+
+Report a watched missing directory that appeared while the previous watcher was paused (e.g. during a webpack build) to the watcher that replaces it, like a file; before, a `node_modules` directory created during a build could be missed until the next unrelated change, most often with polling.

lib/DirectoryWatcher.js

@@ -245,6 +245,9 @@ class DirectoryWatcher extends EventEmitter {
 		this.caseInsensitive = undefined;
 		/** @type {Map<string, Watcher<DirectoryWatcherEvents> | boolean>} */
 		this.directories = new Map();
+		// when each directory was noticed, for watchers that attach later
+		/** @type {Map<string, number>} */
+		this.directorySafeTimes = new Map();
 		/** @type {Map<string, Watcher<FileWatcherEvents>>} */
 		this._symlinkTargetWatchers = new Map();
 		this.lastWatchEvent = 0;
@@ -350,6 +353,7 @@ class DirectoryWatcher extends EventEmitter {
 				(oldDirectory).close();
 			}
 			this.directories.delete(itemPath);
+			this.directorySafeTimes.delete(itemPath);
 			this.forEachWatcher(itemPath, (w) => w.emit("remove", type));
 			if (!initial) {
 				this.forEachWatcher(this.path, (w) =>
@@ -474,6 +478,9 @@ class DirectoryWatcher extends EventEmitter {
 				}
 
 				const safeTime = initial ? Math.min(now, birthtime) + FS_ACCURACY : now;
+				// when it was noticed, not created: a poll can notice it after the
+				// next build started, which then did not see it either
+				this.directorySafeTimes.set(directoryPath, safeTime);
 
 				this.forEachWatcher(directoryPath, (w) => {
 					if (!initial || w.checkStartTime(safeTime, false)) {
@@ -570,7 +577,13 @@ class DirectoryWatcher extends EventEmitter {
 				fixupEntryAccuracy(entry);
 				safeTime = entry.safeTime;
 			} else {
-				safeTime = 0;
+				// a directory that appeared while no watcher listened, e.g. during
+				// a build whose paused watcher is replaced by this one; internal
+				// watchers attach with start time 1 to wait for a directory to
+				// appear, which one already known is not
+				safeTime =
+					(startTime && startTime > 1 && this.directorySafeTimes.get(target)) ||
+					0;
 			}
 		}
 		if (safeTime) {

test/DirectoryOutdatedOnAttach.test.js

@@ -0,0 +1,143 @@
+"use strict";
+
+const path = require("path");
+const Watchpack = require("../lib");
+const DirectoryWatcher = require("../lib/DirectoryWatcher");
+const TestHelper = require("./helpers/TestHelper");
+
+const fixtures = path.join(__dirname, "fixtures");
+const testHelper = new TestHelper(fixtures);
+
+// eslint-disable-next-line jest/no-confusing-set-timeout
+jest.setTimeout(20000);
+
+describe("Directory created before a watcher attaches", () => {
+	beforeEach((done) => {
+		testHelper.before(done);
+	});
+
+	/** @type {InstanceType<typeof Watchpack>[]} */
+	let watchers = [];
+
+	afterEach((done) => {
+		// closed here, not in the test, so a failure cannot leave one running
+		for (const w of watchers) w.close();
+		watchers = [];
+		jest.restoreAllMocks();
+		testHelper.after(done);
+	});
+
+	/**
+	 * @param {false | number} poll poll option
+	 * @param {jest.DoneCallback} done done callback
+	 */
+	const expectReported = (poll, done) => {
+		// how webpack hands over between builds: the paused watcher of the
+		// last build sees the change, the next one attaches with the start
+		// time of that build and the paused one is closed
+		const dir = path.join(fixtures, "dir");
+		// webpack passes the same options, so both share one directory watcher
+		const options = { aggregateTimeout: 100, poll };
+		testHelper.tick(1000, () => {
+			const startTime = Date.now();
+			const paused = new Watchpack(options);
+			watchers.push(paused);
+			paused.watch({ missing: [dir], startTime });
+			testHelper.tick(500, () => {
+				paused.pause();
+				testHelper.dir("dir");
+				const waitForPaused = () => {
+					if (!paused.aggregatedChanges.has(dir)) {
+						setTimeout(waitForPaused, 50);
+						return;
+					}
+					const next = new Watchpack(options);
+					watchers.push(next);
+					next.on("aggregated", (changes) => {
+						expect([...changes]).toEqual([dir]);
+						done();
+					});
+					next.watch({ missing: [dir], startTime });
+					paused.close();
+				};
+				waitForPaused();
+			});
+		});
+	};
+
+	it("should report a missing directory created after its start time", (done) => {
+		expectReported(false, done);
+	});
+
+	it("should report a missing directory created after its start time (polling)", (done) => {
+		expectReported(100, done);
+	});
+
+	it("should report a directory noticed after the start time though created before it", (done) => {
+		// the next build starts right after the directory is created, before any
+		// watcher notices it; the build read the cached state, so it did not see
+		// the directory either
+		const dir = path.join(fixtures, "dir");
+		const options = { aggregateTimeout: 100, poll: 100 };
+		testHelper.tick(1000, () => {
+			const paused = new Watchpack(options);
+			watchers.push(paused);
+			paused.watch({ missing: [dir], startTime: Date.now() });
+			testHelper.tick(500, () => {
+				paused.pause();
+				testHelper.dir("dir");
+				const startTime = Date.now();
+				const waitForPaused = () => {
+					if (!paused.aggregatedChanges.has(dir)) {
+						setTimeout(waitForPaused, 50);
+						return;
+					}
+					const next = new Watchpack(options);
+					watchers.push(next);
+					next.on("aggregated", (changes) => {
+						expect([...changes]).toEqual([dir]);
+						done();
+					});
+					next.watch({ missing: [dir], startTime });
+					paused.close();
+				};
+				waitForPaused();
+			});
+		});
+	});
+
+	it("should not keep retrying a removed directory its parent still lists", (done) => {
+		// until its next scan the parent still lists the directory, which must
+		// not count as the directory appearing again
+		const dir = path.join(fixtures, "dir");
+		testHelper.dir("dir");
+		testHelper.file(path.join("dir", "a"));
+		const { watchInParentDirectory } = DirectoryWatcher.prototype;
+		let retries = 0;
+		jest
+			.spyOn(DirectoryWatcher.prototype, "watchInParentDirectory")
+			.mockImplementation(
+				/** @this {InstanceType<typeof DirectoryWatcher>} */
+				function retry() {
+					// a loop never yields to the timers, so it is cut off here
+					if (++retries > 50) return;
+					watchInParentDirectory.call(this);
+				},
+			);
+		testHelper.tick(1000, () => {
+			const w = new Watchpack({ aggregateTimeout: 100, poll: 1000 });
+			watchers.push(w);
+			w.watch({
+				files: [path.join(dir, "a"), path.join(fixtures, "b")],
+				startTime: Date.now(),
+			});
+			testHelper.tick(500, () => {
+				testHelper.remove("dir");
+				testHelper.tick(1500, () => {
+					expect(retries).toBeLessThan(10);
+					done();
+				});
+			});
+		});
+	});
+});

types/DirectoryWatcher.d.ts

@@ -50,6 +50,8 @@ declare class DirectoryWatcher extends EventEmitter<{
 	caseInsensitive: boolean | undefined;
 	/** @type {Map<string, Watcher<DirectoryWatcherEvents> | boolean>} */
 	directories: Map<string, Watcher<DirectoryWatcherEvents> | boolean>;
+	/** @type {Map<string, number>} */
+	directorySafeTimes: Map<string, number>;
 	/** @type {Map<string, Watcher<FileWatcherEvents>>} */
 	_symlinkTargetWatchers: Map<string, Watcher<FileWatcherEvents>>;
 	lastWatchEvent: number;