Repository: webpack/webpack (JavaScript)

Issue:
HMR not working on webpack 5.66.0
<!-- Please don't delete this template because we'll close your issue -->
<!-- Before creating an issue please make sure you are using the latest version of webpack. -->

# Bug report

<!-- Please ask questions on StackOverflow or the webpack Gitter. -->
<!-- https://stackoverflow.com/questions/ask?tags=webpack -->
<!-- https://gitter.im/webpack/webpack -->
<!-- Issues which contain questions or support requests will be closed. -->

**What is the current behavior?**
Hot module reloading is not working after updating webpack to v5.66.0.

**If the current behavior is a bug, please provide the steps to reproduce.**
- Clone https://github.com/saltire/react-node-boilerplate.
- Run `npm i webpack@latest` to update to **5.66.0**.
- Run `npm run watch`.
- Modify `src/App.jsx`. The browser console reports `bundle rebuilding` twice, the request to `main.xxxxx.hot-update.json` fails with status 404, and the app does not update.

Notice the issue does not occur if webpack **5.65.0** is installed.

<!-- A great way to do this is to provide your configuration via a GitHub repository -->
<!-- The most helpful is a minimal reproduction with instructions on how to reproduce -->
<!-- Repositories with too many files or large `webpack.config.js` files are not suitable -->
<!-- Please only add small code snippets directly into this issue -->
<!-- https://gist.github.com is a good place for longer code snippets -->
<!-- If your issue is caused by a plugin or loader, please create an issue on the loader/plugin repository instead -->

**What is the expected behavior?**
When making a change to a jsx file, the app should reload automatically.

<!-- "It should work" is not a helpful explanation -->
<!-- Explain exactly how it should behave -->

**Other relevant information:**
webpack version:  5.66.0
Node.js version: 16
Operating System: MacOS
Additional tools

This repo uses `webpack-dev-middleware`, `webpack-hot-middleware`, `react-refresh`, and `@pmmmwh/react-refresh-webpack-plugin`, among other packages. The issue may stem from a conflict with one of these, but I don't have enough internal knowledge of webpack or these packages to say which. It could also be a misconfiguration on my part. I'm filing the issue with `webpack` because I was able to isolate the problem to an update of this package.

Let me know if you need any more info!


Hints:
Can you try to increase `watchOptions.aggregateTimeout` due https://github.com/webpack/webpack/releases/tag/v5.66.0
add `aggregateTimeout:200`，and then change `aggregateTimeout` to `600`，all failed.

`hooks.done` callback doesn't run sometimes。
@saltire Wierd, I can't reproduce, maybe you have network problem? Try to reinstall deps
I confirm the same issue. I have upgraded to Webpack 5.66, and my perfectly working configuration on Webpack 5.55, now triggers a full page reload on every change, even of simple CSS files.

webpack: 5.66.0
Node.js: 17
MacOS: 12.1

`main.*.hot-update.json` fails with status **404**, and the page does a full reload.
We are seeing the same issue, a single change triggers a double compilation, and that causes the first compilation assets to be deleted (including the `*.hot-update.js(on)` files) - increasing the `aggregateTimeout` appears to help a little, but it's still a bit flaky.

I'll try to reduce to a minimal config - though may take me a few days to get to it.

node: v16.13.0
webpack: 5.66.0
macos: 11.6.2
Very strange, because we don't change many things around watching, only reduce `aggregateTimeout` for faster builds
this issue still exists in `v5.67.0`
We need more information to understand here is bug, I can't reproduce...
I created a project to reproduce the issue:
1. Download [reproduction.zip](https://github.com/webpack/webpack/files/7925727/reproduction.zip)
2. `npm run init`
3. `npm run dev`

Change any .tsx or .css file and see the 404 error in the network panel (enable "Preserve log" in Chrome) and the webpack doing a full page reload.
Weird... I can't reproduce too, maybe somebody can record video/screenshots
@daniele-orlando do yo have M1 ?

I was able to run @daniele-orlando's example, and it's flaky - most of the times the hot-update is a 404, though sometimes it works fine. Here's the recording @alexander-akait 

I'll try to also create a smaller repro...


https://user-images.githubusercontent.com/879060/150806400-3aafdb89-0bde-404b-bc00-17f057be1dd7.mov

Not an M1 machine:

![Screenshot 2022-01-24 at 4 00 33 PM](https://user-images.githubusercontent.com/879060/150807401-fb8ed2b3-ce5e-42c9-afa0-262ab8d3dc1b.png)


hm, I think it is due double bundling (you can see two `successfully` output), something was changed and webpack bundle it again, and hash may was changed
@alexander-akait yeah, that's what I think as well, we've been seeing double bundling on our side - though often times, the hash doesn't change, but the files are deleted from the output directory. I'm trying to bisect when this started happening, I'll post my results here shortly

**Update**: it's starting from `5.66.0` - any prior version I tried (`5.65.0`, `5.64.4`, `5.62.0`, `5.64.0`, `5.62.0`) works as expected.
Added the update to my previous comment - and just a note, in @daniele-orlando's example, setting the `aggregateTimeout` to 400, or 600 fixes it - I wasn't able to make it 404 in ~20-30 HMRs.
It seems we got an event too fast...
I created this repro that sort-of shows it with a minimal config: https://github.com/rigor789/webpack-hmr-dobule-emit

Added a screenshot to the readme - it logs which file triggered the change. The result is that the dist folder doesn't have the hot-update files when it does a double trigger like this.

Interestingly, if I just `touch src/app.js` it doesn't double-compile.

Also interesting, triggering the change with another editor like `nano` also only triggers one compilation - makes me think that there's a different saving mechanism in VSCode that triggers the file to be saved twice...

So while I think increasing the `aggregateTimeout` is a workaround - maybe webpack shouldn't delete the previous hot-update files in case the compilation didn't emit any new files/nothing changed? When I get a double compilation, both hashes are the same.

> maybe webpack shouldn't delete the previous hot-update files in case the compilation didn't emit any new files/nothing changed?

It can be memory problem, ideally we should find a way to prevent double compilation
try set webpack.config.js 
```
  devServer: {
    ...
    hot: false,
  },
```
This seem to be related to `output.clean: true` which removes the HMR update file on the second compilation. Maybe we should keep HMR files around for a while before removing them?

The second cause of the problem seem to be the double compilation, which could have various reasons...
I have a similar problem with `webpack@5.67.0` and `webpack-dev-server@4.7.3` 

```
1st emit
98% emitting after emit IsDonePluginCompilation afterEmit 2022-02-04T10:24:47.355Z
99% done plugins IsDonePluginCompilation is done 2022-02-04T10:24:47.357Z
...
webpack 5.67.0 compiled successfully in 41919 ms
2nd emit
98% emitting after emit IsDonePluginCompilation afterEmit 2022-02-04T10:25:38.063Z
99% done plugins IsDonePluginCompilation is done 2022-02-04T10:25:38.065Z
...
webpack 5.67.0 compiled successfully in 19602 ms
```

I'm only running a `webpack serve --mode development` command in terminal, browser is not open. 

> This seem to be related to `output.clean: true` which removes the HMR update file on the second compilation. Maybe we should keep HMR files around for a while before removing them?
> 
> The second cause of the problem seem to be the double compilation, which could have various reasons...

Can confirm that I only see the issue when I have `clean: true` set in my output configuration. I still see the "bundle rebuilding" messages twice, but at least it doesn't fail when trying to load the hot-update files after the second compilation.
Does anyone knows the simples way how to detect the reason why a `rebuild` happens?

In my case I was able to limit the cause to adding/removing `import "./someStyles.css" (with the import the initial process rebuilds 2times)
@vire will be great if you reduce this to reproducible repo
@vankop https://github.com/vire/webpack-rebuild-reproduce 

Let me please know if this works
I think my case is related to https://github.com/webpack/webpack/issues/14946
Apologies, I commented on the `aggregateTimeout` PR as I didn't know this issue was already open. Below is a copy of my comment, original here: https://github.com/webpack/webpack/pull/15041#issuecomment-1033459364

> The docs still state this value is `200`: https://webpack.js.org/configuration/watch/#watchoptionsaggregatetimeout
> 
> FWIW, this causes double-rebuilds when using Visual Studio Code (all extensions disabled). For brevity I've created a repro https://github.com/levymetal/webpack5-double-compile-repro but this occurs on all my projects upgraded to webpack>=5.66.0 on multiple machines.
> 
> This might be because VSCode has previously been known to touch files twice when saving: https://github.com/microsoft/vscode/issues/9419. This wasn't fixed at the time and I don't know if it's still the case as I haven't yet attempted to log filesystem events, but it seems likely.
> 
> I'm not sure if it's Webpack's job to work around the limitations of VSCode, but as it currently stands this is a tricky issue to debug that will affect a significant number of people. IMHO this seems big enough to warrant calling out in the docs, that some editors require an `aggregateTimeout` of at least 100ms to avoid compiling twice on every save.
> 
> I wonder if this may even be the historical reason for 200ms.
I've found that the timeout should be as high as 600ms to ensure HMR does not get bailout and fallback to live reloading the page. I believe it would be best if it worked with a more predictable behavior.

Patch:
diff --git a/lib/CleanPlugin.js b/lib/CleanPlugin.js
--- 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, 0);
 						// 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, 0);
+					}
 				}
 
 				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, keptAssets) => {
+							if (err) {
+								oldAssets = undefined;
+							} else {
+								if (oldAssets) mergeAssets(currentAssets, oldAssets);
+								oldAssets = currentAssets;
+								if (keptAssets) mergeAssets(oldAssets, keptAssets);
+							}
+							callback(err);
 						}
-						callback(err);
-					});
+					);
 				};
 
 				if (oldAssets) {


Test patch:
diff --git a/test/HotModuleReplacementPlugin.test.js b/test/HotModuleReplacementPlugin.test.js
--- 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");