keep hmr update with output.clean=true based on timestamp

vankop · vankop · commit 1485eb51b978 · 2022-02-26T15:28:33.000+03:00
diff --git a/lib/CleanPlugin.js b/lib/CleanPlugin.js
@@ -19,6 +19,7 @@ const processAsyncTree = require("./util/processAsyncTree");
 /** @typedef {import("./util/fs").StatsCallback} StatsCallback */
 
 /** @typedef {(function(string):boolean)|RegExp} IgnoreItem */
+/** @typedef {Map<string, number>} Assets */
 /** @typedef {function(IgnoreItem): void} AddToIgnoreCallback */
 
 /**
@@ -40,18 +41,32 @@ const validate = createSchemaValidation(
 		baseDataPath: "options"
 	}
 );
+const _10sec = 10 * 1000;
+
+/**
+ * marge assets map 2 into map 1
+ * @param {Assets} as1 assets
+ * @param {Assets} as2 assets
+ * @returns {void}
+ */
+const mergeAssets = (as1, as2) => {
+	for (const [key, value1] of as2) {
+		const value2 = as1.get(key);
+		if (!value2 || value1 > value2) as1.set(key, value1);
+	}
+};
 
 /**
  * @param {OutputFileSystem} fs filesystem
  * @param {string} outputPath output path
- * @param {Set<string>} currentAssets filename of the current assets (must not start with .. or ., must only use / as path separator)
+ * @param {Map<string, number>} currentAssets filename of the current assets (must not start with .. or ., must only use / as path separator)
  * @param {function((Error | null)=, Set<string>=): void} callback returns the filenames of the assets that shouldn't be there
  * @returns {void}
  */
 const getDiffToFs = (fs, outputPath, currentAssets, callback) => {
 	const directories = new Set();
 	// get directories of assets
-	for (const asset of currentAssets) {
+	for (const [asset] of currentAssets) {
 		directories.add(asset.replace(/(^|\/)[^/]*$/, ""));
 	}
 	// and all parent directories
@@ -91,13 +106,15 @@ const getDiffToFs = (fs, outputPath, currentAssets, callback) => {
 };
 
 /**
- * @param {Set<string>} currentAssets assets list
- * @param {Set<string>} oldAssets old assets list
+ * @param {Assets} currentAssets assets list
+ * @param {Assets} oldAssets old assets list
  * @returns {Set<string>} diff
  */
 const getDiffToOldAssets = (currentAssets, oldAssets) => {
 	const diff = new Set();
-	for (const asset of oldAssets) {
+	const now = Date.now();
+	for (const [asset, ts] of oldAssets) {
+		if (ts >= now) continue;
 		if (!currentAssets.has(asset)) diff.add(asset);
 	}
 	return diff;
@@ -124,7 +141,7 @@ const doStat = (fs, filename, callback) => {
  * @param {Logger} logger logger
  * @param {Set<string>} diff filenames of the assets that shouldn't be there
  * @param {function(string): boolean} isKept check if the entry is ignored
- * @param {function(Error=): void} callback callback
+ * @param {function(Error=, Assets=): void} callback callback
  * @returns {void}
  */
 const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => {
@@ -137,11 +154,13 @@ const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => {
 	};
 	/** @typedef {{ type: "check" | "unlink" | "rmdir", filename: string, parent: { remaining: number, job: Job } | undefined }} Job */
 	/** @type {Job[]} */
-	const jobs = Array.from(diff, filename => ({
+	const jobs = Array.from(diff.keys(), filename => ({
 		type: "check",
 		filename,
 		parent: undefined
 	}));
+	/** @type {Assets} */
+	const keptAssets = new Map();
 	processAsyncTree(
 		jobs,
 		10,
@@ -161,6 +180,7 @@ const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => {
 			switch (type) {
 				case "check":
 					if (isKept(filename)) {
+						keptAssets.set(filename, Date.now());
 						// do not decrement parent entry as we don't want to delete the parent
 						log(`${filename} will be kept`);
 						return process.nextTick(callback);
@@ -247,7 +267,10 @@ const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => {
 					break;
 			}
 		},
-		callback
+		err => {
+			if (err) return callback(err);
+			callback(undefined, keptAssets);
+		}
 	);
 };
 
@@ -302,6 +325,7 @@ class CleanPlugin {
 		// We assume that no external modification happens while the compiler is active
 		// So we can store the old assets and only diff to them to avoid fs access on
 		// incremental builds
+		/** @type {undefined|Assets} */
 		let oldAssets;
 
 		compiler.hooks.emit.tapAsync(
@@ -322,7 +346,9 @@ class CleanPlugin {
 					);
 				}
 
-				const currentAssets = new Set();
+				/** @type {Assets} */
+				const currentAssets = new Map();
+				const now = Date.now();
 				for (const asset of Object.keys(compilation.assets)) {
 					if (/^[A-Za-z]:\\|^\/|^\\\\/.test(asset)) continue;
 					let normalizedAsset;
@@ -335,7 +361,12 @@ class CleanPlugin {
 						);
 					} while (newNormalizedAsset !== normalizedAsset);
 					if (normalizedAsset.startsWith("../")) continue;
-					currentAssets.add(normalizedAsset);
+					const assetInfo = compilation.assetsInfo.get(asset);
+					if (assetInfo && assetInfo.hotModuleReplacement) {
+						currentAssets.set(normalizedAsset, now + _10sec);
+					} else {
+						currentAssets.set(normalizedAsset, now);
+					}
 				}
 
 				const outputPath = compilation.getPath(compiler.outputPath, {});
@@ -346,19 +377,34 @@ class CleanPlugin {
 					return keepFn(path);
 				};
 
+				/**
+				 * @param {Error=} err err
+				 * @param {Set<string>=} diff diff
+				 */
 				const diffCallback = (err, diff) => {
 					if (err) {
 						oldAssets = undefined;
-						return callback(err);
+						callback(err);
+						return;
 					}
-					applyDiff(fs, outputPath, dry, logger, diff, isKept, err => {
-						if (err) {
-							oldAssets = undefined;
-						} else {
-							oldAssets = currentAssets;
+					applyDiff(
+						fs,
+						outputPath,
+						dry,
+						logger,
+						diff,
+						isKept,
+						(err, notDeletedAssets) => {
+							if (err) {
+								oldAssets = undefined;
+							} else {
+								if (oldAssets) mergeAssets(currentAssets, oldAssets);
+								oldAssets = currentAssets;
+								if (notDeletedAssets) mergeAssets(oldAssets, notDeletedAssets);
+							}
+							callback(err);
 						}
-						callback(err);
-					});
+					);
 				};
 
 				if (oldAssets) {
diff --git a/test/HotModuleReplacementPlugin.test.js b/test/HotModuleReplacementPlugin.test.js
@@ -99,6 +99,81 @@ describe("HotModuleReplacementPlugin", () => {
 		});
 	}, 120000);
 
+	it("output.clean=true should keep 1 last update", done => {
+		const outputPath = path.join(__dirname, "js", "HotModuleReplacementPlugin");
+		const entryFile = path.join(outputPath, "entry.js");
+		const recordsFile = path.join(outputPath, "records.json");
+		let step = 0;
+		let firstUpdate;
+		try {
+			fs.mkdirSync(outputPath, { recursive: true });
+		} catch (e) {
+			// empty
+		}
+		fs.writeFileSync(entryFile, `${++step}`, "utf-8");
+		const updates = new Set();
+		const hasFile = file => {
+			try {
+				fs.statSync(path.join(outputPath, file));
+				return true;
+			} catch (err) {
+				return false;
+			}
+		};
+		const compiler = webpack({
+			mode: "development",
+			cache: false,
+			entry: {
+				0: entryFile
+			},
+			recordsPath: recordsFile,
+			output: {
+				path: outputPath,
+				clean: true
+			},
+			plugins: [new webpack.HotModuleReplacementPlugin()]
+		});
+		const callback = (err, stats) => {
+			if (err) return done(err);
+			const jsonStats = stats.toJson();
+			const hash = jsonStats.hash;
+			const hmrUpdateMainFileName = `0.${hash}.hot-update.json`;
+
+			switch (step) {
+				case 1:
+					expect(updates.size).toBe(0);
+					firstUpdate = hmrUpdateMainFileName;
+					break;
+				case 2:
+					expect(updates.size).toBe(1);
+					expect(updates.has(firstUpdate)).toBe(true);
+					expect(hasFile(firstUpdate)).toBe(true);
+					break;
+				case 3:
+					expect(updates.size).toBe(2);
+					for (const file of updates) {
+						expect(hasFile(file)).toBe(true);
+					}
+					return setTimeout(() => {
+						fs.writeFileSync(entryFile, `${++step}`, "utf-8");
+						compiler.run(err => {
+							if (err) return done(err);
+							for (const file of updates) {
+								expect(hasFile(file)).toBe(false);
+							}
+							done();
+						});
+					}, 10100);
+			}
+
+			updates.add(hmrUpdateMainFileName);
+			fs.writeFileSync(entryFile, `${++step}`, "utf-8");
+			compiler.run(callback);
+		};
+
+		compiler.run(callback);
+	}, 20000);
+
 	it("should correct working when entry is Object and key is a number", done => {
 		const outputPath = path.join(__dirname, "js", "HotModuleReplacementPlugin");
 		const entryFile = path.join(outputPath, "entry.js");
