Merge pull request #11690 from webpack/bugfix/11673

fix unused modules in chunk when optimizing runtime-specific
webpack · Oct 15, 2020 · 4669600 · 4669600
2 parents 234373e + 50c3a83
commit 4669600
Show file tree

Hide file tree

Showing 12 changed files with 123 additions and 26 deletions.
diff --git a/lib/ExportsInfo.js b/lib/ExportsInfo.js
@@ -416,7 +416,7 @@ class ExportsInfo {
 
 	/**
 	 * @param {RuntimeSpec} runtime the runtime
-	 * @returns {boolean} true, when the module is used in any way
+	 * @returns {boolean} true, when the module exports are used in any way
 	 */
 	isUsed(runtime) {
 		if (this._redirectTo !== undefined) {
@@ -436,6 +436,17 @@ class ExportsInfo {
 		return false;
 	}
 
+	/**
+	 * @param {RuntimeSpec} runtime the runtime
+	 * @returns {boolean} true, when the module is used in any way
+	 */
+	isModuleUsed(runtime) {
+		if (this.isUsed(runtime)) return true;
+		if (this._sideEffectsOnlyInfo.getUsed(runtime) !== UsageState.Unused)
+			return true;
+		return false;
+	}
+
 	/**
 	 * @param {RuntimeSpec} runtime the runtime
 	 * @returns {SortableSet<string> | boolean | null} set of used exports, or true (when namespace object is used), or false (when unused), or null (when unknown)

diff --git a/lib/FlagDependencyUsagePlugin.js b/lib/FlagDependencyUsagePlugin.js
@@ -63,9 +63,15 @@ class FlagDependencyUsagePlugin {
 					 * @param {Module} module module to process
 					 * @param {(string[] | ReferencedExport)[]} usedExports list of used exports
 					 * @param {RuntimeSpec} runtime part of which runtime
+					 * @param {boolean} forceSideEffects always apply side effects
 					 * @returns {void}
 					 */
-					const processReferencedModule = (module, usedExports, runtime) => {
+					const processReferencedModule = (
+						module,
+						usedExports,
+						runtime,
+						forceSideEffects
+					) => {
 						const exportsInfo = moduleGraph.getExportsInfo(module);
 						if (usedExports.length > 0) {
 							if (!module.buildMeta || !module.buildMeta.exportsType) {
@@ -143,10 +149,12 @@ class FlagDependencyUsagePlugin {
 							// This module won't be evaluated in this case
 							// TODO webpack 6 remove this check
 							if (
+								!forceSideEffects &&
 								module.factoryMeta !== undefined &&
 								module.factoryMeta.sideEffectFree
-							)
+							) {
 								return;
+							}
 							if (exportsInfo.setUsedForSideEffectsOnly(runtime)) {
 								queue.enqueue(module, runtime);
 							}
@@ -242,12 +250,18 @@ class FlagDependencyUsagePlugin {
 
 						for (const [module, referencedExports] of map) {
 							if (Array.isArray(referencedExports)) {
-								processReferencedModule(module, referencedExports, runtime);
+								processReferencedModule(
+									module,
+									referencedExports,
+									runtime,
+									false
+								);
 							} else {
 								processReferencedModule(
 									module,
 									Array.from(referencedExports.values()),
-									runtime
+									runtime,
+									false
 								);
 							}
 						}
@@ -270,8 +284,12 @@ class FlagDependencyUsagePlugin {
 					const processEntryDependency = (dep, runtime) => {
 						const module = moduleGraph.getModule(dep);
 						if (module) {
-							processReferencedModule(module, NO_EXPORTS_REFERENCED, runtime);
-							queue.enqueue(module, runtime);
+							processReferencedModule(
+								module,
+								NO_EXPORTS_REFERENCED,
+								runtime,
+								true
+							);
 						}
 					};
 					/** @type {RuntimeSpec} */
@@ -307,28 +325,59 @@ class FlagDependencyUsagePlugin {
 			);
 			if (!this.global) {
 				compilation.hooks.afterChunks.tap("FlagDependencyUsagePlugin", () => {
-					/** @type {Map<Chunk, string>} */
-					const runtimeChunks = new Map();
-					for (const entrypoint of compilation.entrypoints.values()) {
-						runtimeChunks.set(
-							entrypoint.getRuntimeChunk(),
-							entrypoint.options.runtime
+					const processEntrypoint = entrypoint => {
+						const runtime = getEntryRuntime(
+							compilation,
+							entrypoint.name,
+							entrypoint.options
 						);
+						for (const chunk of entrypoint
+							.getEntrypointChunk()
+							.getAllReferencedChunks()) {
+							chunk.runtime = mergeRuntime(chunk.runtime, runtime);
+						}
+					};
+					for (const entrypoint of compilation.entrypoints.values()) {
+						processEntrypoint(entrypoint);
 					}
-					for (const entrypoint of compilation.asyncEntrypoints) {
-						runtimeChunks.set(
-							entrypoint.getRuntimeChunk(),
-							entrypoint.options.runtime
-						);
+					for (const asyncEntry of compilation.asyncEntrypoints) {
+						processEntrypoint(asyncEntry);
 					}
+				});
 
-					for (const [runtimeChunk, runtimeName] of runtimeChunks) {
-						const runtime = runtimeName || runtimeChunk.name;
-						for (const chunk of runtimeChunk.getAllReferencedChunks()) {
-							chunk.runtime = mergeRuntime(chunk.runtime, runtime);
+				compilation.hooks.optimizeChunkModules.tap(
+					"FlagDependencyUsagePlugin",
+					(chunks, modules) => {
+						for (const module of modules) {
+							const exportsInfo = compilation.moduleGraph.getExportsInfo(
+								module
+							);
+							let removeFromRuntimes = undefined;
+							for (const runtime of compilation.chunkGraph.getModuleRuntimes(
+								module
+							)) {
+								if (!exportsInfo.isModuleUsed(runtime)) {
+									if (removeFromRuntimes === undefined) {
+										removeFromRuntimes = new Set();
+									}
+									removeFromRuntimes.add(runtime);
+								}
+							}
+							if (removeFromRuntimes !== undefined) {
+								for (const chunk of compilation.chunkGraph.getModuleChunksIterable(
+									module
+								)) {
+									if (removeFromRuntimes.has(chunk.runtime)) {
+										compilation.chunkGraph.disconnectChunkAndModule(
+											chunk,
+											module
+										);
+									}
+								}
+							}
 						}
 					}
-				});
+				);
 			}
 		});
 	}

diff --git a/lib/optimize/ConcatenatedModule.js b/lib/optimize/ConcatenatedModule.js
@@ -1508,7 +1508,7 @@ ${defineGetters}`
 		const orderedConcatenationList = this._createConcatenationList(
 			this.rootModule,
 			this._modules,
-			undefined,
+			runtime,
 			moduleGraph
 		);
 		return orderedConcatenationList.map((info, index) => {

diff --git a/lib/util/runtime.js b/lib/util/runtime.js
@@ -44,7 +44,7 @@ exports.getEntryRuntime = (compilation, name, options) => {
 				result = mergeRuntimeOwned(result, runtime || name);
 			}
 		}
-		return result;
+		return result || name;
 	} else {
 		return runtime || name;
 	}

diff --git a/test/TestCases.template.js b/test/TestCases.template.js
@@ -287,7 +287,7 @@ const describeCases = config => {
 										if (module.substr(0, 2) === "./") {
 											const p = path.join(outputDirectory, module);
 											const fn = vm.runInThisContext(
-												"(function(require, module, exports, __dirname, it, expect) {" +
+												"(function(require, module, exports, __dirname, __filename, it, expect) {" +
 													"global.expect = expect;" +
 													'function nsObj(m) { Object.defineProperty(m, Symbol.toStringTag, { value: "Module" }); return m; }' +
 													fs.readFileSync(p, "utf-8") +
@@ -304,6 +304,7 @@ const describeCases = config => {
 												m,
 												m.exports,
 												outputDirectory,
+												p,
 												_it,
 												expect
 											);

diff --git a/test/cases/side-effects/issue-11673/index.js b/test/cases/side-effects/issue-11673/index.js
@@ -0,0 +1,12 @@
+import { Worker } from "worker_threads";
+import { X } from "./module";
+// test
+
+it("should compile", done => {
+	expect(X()).toBe("X");
+	const worker = new Worker(new URL("worker.js", import.meta.url));
+	worker.once("message", value => {
+		expect(value).toBe(42);
+		done();
+	});
+});
diff --git a/test/cases/side-effects/issue-11673/module.js b/test/cases/side-effects/issue-11673/module.js
@@ -0,0 +1,9 @@
+import value from "package";
+
+export function X() {
+	return "X";
+}
+
+export function Y() {
+	return value;
+}
diff --git a/test/cases/side-effects/issue-11673/node_modules/package/index.js b/test/cases/side-effects/issue-11673/node_modules/package/index.js
diff --git a/test/cases/side-effects/issue-11673/node_modules/package/package.json b/test/cases/side-effects/issue-11673/node_modules/package/package.json
diff --git a/test/cases/side-effects/issue-11673/test.filter.js b/test/cases/side-effects/issue-11673/test.filter.js
@@ -0,0 +1,5 @@
+var supportsWorker = require("../../../helpers/supportsWorker");
+
+module.exports = function (config) {
+	return supportsWorker();
+};
diff --git a/test/cases/side-effects/issue-11673/worker.js b/test/cases/side-effects/issue-11673/worker.js
@@ -0,0 +1,4 @@
+import { Y } from "./module";
+import { parentPort } from "worker_threads";
+
+parentPort.postMessage(Y());
diff --git a/types.d.ts b/types.d.ts
@@ -3076,6 +3076,7 @@ declare abstract class ExportsInfo {
 	setAllKnownExportsUsed(runtime: string | SortableSet<string>): boolean;
 	setUsedForSideEffectsOnly(runtime: string | SortableSet<string>): boolean;
 	isUsed(runtime: string | SortableSet<string>): boolean;
+	isModuleUsed(runtime: string | SortableSet<string>): boolean;
 	getUsedExports(
 		runtime: string | SortableSet<string>
 	): boolean | SortableSet<string>;