Merge pull request #345 from webpack/fix/report-removal-after-start-time

fix: report a file removed after startTime to a watcher attached later

Author: alexander-akait

.changeset/report-removal-after-start-time.md

@@ -0,0 +1,7 @@
+---
+"watchpack": patch
+---
+
+fix: report a file removed after `startTime` to a watcher attached later, as a change after `startTime` already is.
+
+Previously a watcher attached with a `startTime` learned of files changed since then ("outdated on attach") but not of files removed since then, so webpack missed a dependency deleted while a build ran.

lib/DirectoryWatcher.js

@@ -233,6 +233,10 @@ class DirectoryWatcher extends EventEmitter {
 		this.initialScanRemoved = new Set();
 		/** @type {undefined | number} */
 		this.initialScanFinished = undefined;
+		// when a known file was removed, so a watcher attaching later with an
+		// earlier start time learns of the removal, as it learns of a change
+		/** @type {Map<string, number>} */
+		this.removedTimes = new Map();
 		/** @type {Map<string, Set<Watcher<DirectoryWatcherEvents> | Watcher<FileWatcherEvents>>>} */
 		this.watchers = new Map();
 		/** @type {Watcher<FileWatcherEvents> | null} */
@@ -319,6 +323,7 @@ class DirectoryWatcher extends EventEmitter {
 		const oldFile = this.files.get(itemPath);
 		if (oldFile) {
 			this.files.delete(itemPath);
+			if (!initial) this.removedTimes.set(withoutCase(itemPath), Date.now());
 			const key = withoutCase(itemPath);
 			const count = /** @type {number} */ (this.filesWithoutCase.get(key)) - 1;
 			if (count <= 0) {
@@ -377,6 +382,7 @@ class DirectoryWatcher extends EventEmitter {
 		});
 
 		if (!old) {
+			this.removedTimes.delete(withoutCase(target));
 			const key = withoutCase(target);
 			const count = this.filesWithoutCase.get(key);
 			this.filesWithoutCase.set(key, (count || 0) + 1);
@@ -566,11 +572,17 @@ class DirectoryWatcher extends EventEmitter {
 		} else if (
 			target !== this.path &&
 			!this.directories.has(target) &&
-			watcher.checkStartTime(
+			(watcher.checkStartTime(
 				/** @type {number} */
 				(this.initialScanFinished),
 				false,
-			)
+			) ||
+				(this.removedTimes.has(withoutCase(target)) &&
+					watcher.checkStartTime(
+						/** @type {number} */
+						(this.removedTimes.get(withoutCase(target))),
+						false,
+					)))
 		) {
 			process.nextTick(() => {
 				if (this.closed) return;

test/Watchpack.test.js

@@ -1043,6 +1043,65 @@ describe("Watchpack", () => {
 		});
 	});
 
+	it("should detect a past removal of a file (watched)", (done) => {
+		testHelper.file("a");
+		// one options object shares one watcher pool, as webpack does
+		const options = {};
+		const w2 = new WatchpackTest(options);
+		const w = new WatchpackTest(options);
+		w.on("aggregated", (changes, removals) => {
+			expect([...changes]).toEqual([]);
+			expect([...removals]).toEqual([path.join(fixtures, "a")]);
+			w.close();
+			w2.close();
+			done();
+		});
+		testHelper.tick(400, () => {
+			w2.watch([path.join(fixtures, "a")], []);
+			testHelper.tick(() => {
+				const startTime = Date.now();
+				testHelper.tick(() => {
+					// attach once the shared pool has seen the removal, not a live one
+					w2.once("remove", () => {
+						testHelper.tick(() => {
+							w.watch([path.join(fixtures, "a")], [], startTime);
+						});
+					});
+					testHelper.remove("a");
+				});
+			});
+		});
+	});
+
+	it("should not detect a removal before the start time (watched)", (done) => {
+		testHelper.file("a");
+		// one options object shares one watcher pool, as webpack does
+		const options = {};
+		const w2 = new WatchpackTest(options);
+		const w = new WatchpackTest(options);
+		let aggregatedEvents = 0;
+		w.on("aggregated", () => {
+			aggregatedEvents++;
+		});
+		testHelper.tick(400, () => {
+			w2.watch([path.join(fixtures, "a")], []);
+			testHelper.tick(() => {
+				w2.once("remove", () => {
+					testHelper.tick(() => {
+						w.watch([path.join(fixtures, "a")], [], Date.now());
+						testHelper.tick(1000, () => {
+							expect(aggregatedEvents).toBe(0);
+							w.close();
+							w2.close();
+							done();
+						});
+					});
+				});
+				testHelper.remove("a");
+			});
+		});
+	});
+
 	it("should watch a single file removal", (done) => {
 		testHelper.file("a");
 		const w = new WatchpackTest({

types/DirectoryWatcher.d.ts

@@ -62,6 +62,8 @@ declare class DirectoryWatcher extends EventEmitter<{
 	initialScanRemoved: null | InitialScanRemoved;
 	/** @type {undefined | number} */
 	initialScanFinished: undefined | number;
+	/** @type {Map<string, number>} */
+	removedTimes: Map<string, number>;
 	/** @type {Map<string, Set<Watcher<DirectoryWatcherEvents> | Watcher<FileWatcherEvents>>>} */
 	watchers: Map<
 		string,