fix duplication due to unsafe cache

lib/Compilation.js

@@ -416,10 +416,10 @@ const byLocation = compareSelect(err => err.loc, compareLocations);
 
 const compareErrors = concatComparators(byModule, byLocation, byMessage);
 
-/** @type {WeakMap<Dependency, Module & { restoreFromUnsafeCache: Function }>} */
+/** @type {WeakMap<Dependency, Module & { restoreFromUnsafeCache: Function } | null>} */
 const unsafeCacheDependencies = new WeakMap();
 
-/** @type {WeakMap<Module, object>} */
+/** @type {WeakMap<Module & { restoreFromUnsafeCache: Function }, object>} */
 const unsafeCacheData = new WeakMap();
 
 class Compilation {
@@ -1023,8 +1023,10 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
 		this.usedModuleIds = null;
 		/** @type {boolean} */
 		this.needAdditionalPass = false;
-		/** @type {Set<Module>} */
-		this._restoredUnsafeCacheEntries = new Set();
+		/** @type {Set<Module & { restoreFromUnsafeCache: Function }>} */
+		this._restoredUnsafeCacheModuleEntries = new Set();
+		/** @type {Map<string, Module & { restoreFromUnsafeCache: Function }>} */
+		this._restoredUnsafeCacheEntries = new Map();
 		/** @type {WeakSet<Module>} */
 		this.builtModules = new WeakSet();
 		/** @type {WeakSet<Module>} */
@@ -1424,9 +1426,7 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
 	 * @returns {void}
 	 */
 	_processModuleDependencies(module, callback) {
-		/**
-		 * @type {Array<{factory: ModuleFactory, dependencies: Dependency[], originModule: Module|null}>}
-		 */
+		/** @type {Array<{factory: ModuleFactory, dependencies: Dependency[], originModule: Module|null}>} */
 		const sortedDependencies = [];
 
 		/** @type {DependenciesBlock} */
@@ -1447,7 +1447,46 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
 		/** @type {Dependency[]} */
 		let listCacheValue;
 
-		const unsafeRestoredModules = new Set();
+		let inProgressSorting = 1;
+		let inProgressTransitive = 1;
+
+		const onDependenciesSorted = err => {
+			if (err) return callback(err);
+
+			// early exit without changing parallelism back and forth
+			if (sortedDependencies.length === 0 && inProgressTransitive === 1) {
+				return callback();
+			}
+
+			// This is nested so we need to allow one additional task
+			this.processDependenciesQueue.increaseParallelism();
+
+			for (const item of sortedDependencies) {
+				inProgressTransitive++;
+				this.handleModuleCreation(item, err => {
+					// In V8, the Error objects keep a reference to the functions on the stack. These warnings &
+					// errors are created inside closures that keep a reference to the Compilation, so errors are
+					// leaking the Compilation object.
+					if (err && this.bail) {
+						if (inProgressTransitive <= 0) return;
+						inProgressTransitive = -1;
+						// eslint-disable-next-line no-self-assign
+						err.stack = err.stack;
+						onTransitiveTasksFinished(err);
+						return;
+					}
+					if (--inProgressTransitive === 0) onTransitiveTasksFinished();
+				});
+			}
+			if (--inProgressTransitive === 0) onTransitiveTasksFinished();
+		};
+
+		const onTransitiveTasksFinished = err => {
+			if (err) return callback(err);
+			this.processDependenciesQueue.decreaseParallelism();
+
+			return callback();
+		};
 
 		/**
 		 * @param {Dependency} dep dependency
@@ -1458,34 +1497,111 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
 			this.moduleGraph.setParents(dep, currentBlock, module, index);
 			if (this._unsafeCache) {
 				try {
-					const cachedModule = unsafeCacheDependencies.get(dep);
-					if (cachedModule === null) return;
-					if (cachedModule !== undefined) {
-						if (!this._restoredUnsafeCacheEntries.has(cachedModule)) {
-							const data = unsafeCacheData.get(cachedModule);
-							cachedModule.restoreFromUnsafeCache(
-								data,
-								this.params.normalModuleFactory,
-								this.params
+					const unsafeCachedModule = unsafeCacheDependencies.get(dep);
+					if (unsafeCachedModule === null) return;
+					if (unsafeCachedModule !== undefined) {
+						if (
+							this._restoredUnsafeCacheModuleEntries.has(unsafeCachedModule)
+						) {
+							this._handleExistingModuleFromUnsafeCache(
+								module,
+								dep,
+								unsafeCachedModule
 							);
-							this._restoredUnsafeCacheEntries.add(cachedModule);
-							if (!this.modules.has(cachedModule)) {
-								this._handleNewModuleFromUnsafeCache(module, dep, cachedModule);
-								unsafeRestoredModules.add(cachedModule);
+							return;
+						}
+						const identifier = unsafeCachedModule.identifier();
+						const cachedModule =
+							this._restoredUnsafeCacheEntries.get(identifier);
+						if (cachedModule !== undefined) {
+							// update unsafe cache to new module
+							unsafeCacheDependencies.set(dep, cachedModule);
+							this._handleExistingModuleFromUnsafeCache(
+								module,
+								dep,
+								cachedModule
+							);
+							return;
+						}
+						inProgressSorting++;
+						this._modulesCache.get(identifier, null, (err, cachedModule) => {
+							if (err) {
+								if (inProgressSorting <= 0) return;
+								inProgressSorting = -1;
+								onDependenciesSorted(err);
 								return;
 							}
-						}
-						this._handleExistingModuleFromUnsafeCache(
-							module,
-							dep,
-							cachedModule
-						);
+							try {
+								if (!this._restoredUnsafeCacheEntries.has(identifier)) {
+									const data = unsafeCacheData.get(cachedModule);
+									if (data === undefined) {
+										processDependencyForResolving(dep);
+										if (--inProgressSorting === 0) onDependenciesSorted();
+										return;
+									}
+									if (cachedModule !== unsafeCachedModule) {
+										unsafeCacheDependencies.set(dep, cachedModule);
+									}
+									cachedModule.restoreFromUnsafeCache(
+										data,
+										this.params.normalModuleFactory,
+										this.params
+									);
+									this._restoredUnsafeCacheEntries.set(
+										identifier,
+										cachedModule
+									);
+									this._restoredUnsafeCacheModuleEntries.add(cachedModule);
+									if (!this.modules.has(cachedModule)) {
+										inProgressTransitive++;
+										this._handleNewModuleFromUnsafeCache(
+											module,
+											dep,
+											cachedModule,
+											err => {
+												if (err) {
+													if (inProgressTransitive <= 0) return;
+													inProgressTransitive = -1;
+													onTransitiveTasksFinished(err);
+												}
+												if (--inProgressTransitive === 0)
+													return onTransitiveTasksFinished();
+											}
+										);
+										if (--inProgressSorting === 0) onDependenciesSorted();
+										return;
+									}
+								}
+								if (unsafeCachedModule !== cachedModule) {
+									unsafeCacheDependencies.set(dep, cachedModule);
+								}
+								this._handleExistingModuleFromUnsafeCache(
+									module,
+									dep,
+									cachedModule
+								); // a3
+							} catch (err) {
+								if (inProgressSorting <= 0) return;
+								inProgressSorting = -1;
+								onDependenciesSorted(err);
+								return;
+							}
+							if (--inProgressSorting === 0) onDependenciesSorted();
+						});
 						return;
 					}
 				} catch (e) {
 					console.error(e);
 				}
 			}
+			processDependencyForResolving(dep);
+		};
+
+		/**
+		 * @param {Dependency} dep dependency
+		 * @returns {void}
+		 */
+		const processDependencyForResolving = dep => {
 			const resourceIdent = dep.getResourceIdentifier();
 			if (resourceIdent !== undefined && resourceIdent !== null) {
 				const category = dep.category;
@@ -1570,68 +1686,10 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
 			return callback(e);
 		}
 
-		if (sortedDependencies.length === 0 && unsafeRestoredModules.size === 0) {
-			callback();
-			return;
-		}
-
-		// This is nested so we need to allow one additional task
-		this.processDependenciesQueue.increaseParallelism();
-
-		const processSortedDependency = (item, callback) => {
-			this.handleModuleCreation(item, err => {
-				// In V8, the Error objects keep a reference to the functions on the stack. These warnings &
-				// errors are created inside closures that keep a reference to the Compilation, so errors are
-				// leaking the Compilation object.
-				if (err && this.bail) {
-					// eslint-disable-next-line no-self-assign
-					err.stack = err.stack;
-					return callback(err);
-				}
-				callback();
-			});
-		};
-
-		const processUnsafeRestoredModule = (item, callback) => {
-			this._handleModuleBuildAndDependencies(module, item, true, callback);
-		};
-
-		const finalCallback = err => {
-			this.processDependenciesQueue.decreaseParallelism();
-
-			return callback(err);
-		};
-
-		if (sortedDependencies.length === 0) {
-			asyncLib.forEach(
-				unsafeRestoredModules,
-				processUnsafeRestoredModule,
-				finalCallback
-			);
-		} else if (unsafeRestoredModules.size === 0) {
-			asyncLib.forEach(
-				sortedDependencies,
-				processSortedDependency,
-				finalCallback
-			);
-		} else {
-			asyncLib.parallel(
-				[
-					cb =>
-						asyncLib.forEach(
-							unsafeRestoredModules,
-							processUnsafeRestoredModule,
-							cb
-						),
-					cb =>
-						asyncLib.forEach(sortedDependencies, processSortedDependency, cb)
-				],
-				finalCallback
-			);
-		}
+		if (--inProgressSorting === 0) onDependenciesSorted();
 	}
 
-	_handleNewModuleFromUnsafeCache(originModule, dependency, module) {
+	_handleNewModuleFromUnsafeCache(originModule, dependency, module, callback) {
 		const moduleGraph = this.moduleGraph;
 
 		moduleGraph.setResolvedModule(originModule, dependency, module);
@@ -1644,6 +1702,13 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
 		this._modules.set(module.identifier(), module);
 		this.modules.add(module);
 		ModuleGraph.setModuleGraphForModule(module, this.moduleGraph);
+
+		this._handleModuleBuildAndDependencies(
+			originModule,
+			module,
+			true,
+			callback
+		);
 	}
 
 	_handleExistingModuleFromUnsafeCache(originModule, dependency, module) {
@@ -1747,20 +1812,24 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
 						/** @type {any} */ (module).restoreFromUnsafeCache &&
 						this._unsafeCachePredicate(module)
 					) {
+						const unsafeCacheableModule =
+							/** @type {Module & { restoreFromUnsafeCache: Function }} */ (
+								module
+							);
 						for (let i = 0; i < dependencies.length; i++) {
 							const dependency = dependencies[i];
 							moduleGraph.setResolvedModule(
 								connectOrigin ? originModule : null,
 								dependency,
-								module
-							);
-							unsafeCacheDependencies.set(
-								dependency,
-								/** @type {any} */ (module)
+								unsafeCacheableModule
 							);
+							unsafeCacheDependencies.set(dependency, unsafeCacheableModule);
 						}
-						if (!unsafeCacheData.has(module)) {
-							unsafeCacheData.set(module, module.getUnsafeCacheData());
+						if (!unsafeCacheData.has(unsafeCacheableModule)) {
+							unsafeCacheData.set(
+								unsafeCacheableModule,
+								unsafeCacheableModule.getUnsafeCacheData()
+							);
 						}
 					} else {
 						applyFactoryResultDependencies();

test/WatchTestCases.template.js

@@ -136,7 +136,7 @@ const describeCases = config => {
 										srcPath: tempDirectory
 									});
 								}
-								const applyConfig = options => {
+								const applyConfig = (options, idx) => {
 									if (!options.mode) options.mode = "development";
 									if (!options.context) options.context = tempDirectory;
 									if (!options.entry) options.entry = "./index.js";
@@ -148,6 +148,11 @@ const describeCases = config => {
 										options.output.pathinfo = true;
 									if (!options.output.filename)
 										options.output.filename = "bundle.js";
+									if (options.cache && options.cache.type === "filesystem") {
+										const cacheDirectory = path.join(tempDirectory, ".cache");
+										options.cache.cacheDirectory = cacheDirectory;
+										options.cache.name = `config-${idx}`;
+									}
 									if (config.experiments) {
 										if (!options.experiments) options.experiments = {};
 										for (const key of Object.keys(config.experiments)) {
@@ -166,7 +171,7 @@ const describeCases = config => {
 								if (Array.isArray(options)) {
 									options.forEach(applyConfig);
 								} else {
-									applyConfig(options);
+									applyConfig(options, 0);
 								}
 
 								const state = {};
@@ -194,7 +199,7 @@ const describeCases = config => {
 											triggeringFilename = filename;
 										}
 									);
-									const watching = compiler.watch(
+									compiler.watch(
 										{
 											aggregateTimeout: 1000
 										},
@@ -391,10 +396,10 @@ const describeCases = config => {
 																done
 															)
 														) {
-															watching.close();
+															compiler.close();
 															return;
 														}
-														watching.close(done);
+														compiler.close(done);
 													}
 												},
 												45000

test/watchCases/cache/unsafe-cache-duplicates/0/after.js

@@ -0,0 +1 @@
+export default 0;

test/watchCases/cache/unsafe-cache-duplicates/0/index.js

@@ -0,0 +1,3 @@
+import "./unsafe-cache-root";
+
+it("should compile fine", () => {});

test/watchCases/cache/unsafe-cache-duplicates/0/module.js

@@ -0,0 +1 @@
+export { default } from "./after";

test/watchCases/cache/unsafe-cache-duplicates/0/unsafe-cache-root.js

@@ -0,0 +1,2 @@
+export default require.resolve("./module");
+export { default as module } from "./module";

test/watchCases/cache/unsafe-cache-duplicates/1/alternative-path.js

@@ -0,0 +1,2 @@
+export default require.resolve("./module");
+export { default as module } from "./module";