prioritize static reexport over runtime reexport for target determina…

…tion this allows to optimize cases where empty modules are reexported e. g. caused by typescript type-only exports
webpack · Apr 6, 2021 · 83c8180 · 83c8180
1 parent 48b7cce
commit 83c8180
Show file tree

Hide file tree

Showing 13 changed files with 140 additions and 35 deletions.
diff --git a/lib/Dependency.js b/lib/Dependency.js
@@ -55,6 +55,7 @@ const memoize = require("./util/memoize");
  * @property {(string | ExportSpec)[]=} exports nested exports
  * @property {ModuleGraphConnection=} from when reexported: from which module
  * @property {string[] | null=} export when reexported: from which export
+ * @property {number=} priority when reexported: with which priority
  * @property {boolean=} hidden export is not visible, because another export blends over it
  */
 
@@ -64,6 +65,7 @@ const memoize = require("./util/memoize");
  * @property {Set<string>=} excludeExports when exports = true, list of unaffected exports
  * @property {Set<string>=} hideExports list of maybe prior exposed, but now hidden exports
  * @property {ModuleGraphConnection=} from when reexported: from which module
+ * @property {number=} priority when reexported: with which priority
  * @property {boolean=} canMangle can the export be renamed (defaults to true)
  * @property {boolean=} terminalBinding are the exports terminal bindings that should be checked for export star conflicts
  * @property {Module[]=} dependencies module on which the result depends on

diff --git a/lib/ExportsInfo.js b/lib/ExportsInfo.js
@@ -269,13 +269,15 @@ class ExportsInfo {
 	 * @param {Set<string>=} excludeExports list of unaffected exports
 	 * @param {any=} targetKey use this as key for the target
 	 * @param {ModuleGraphConnection=} targetModule set this module as target
+	 * @param {number=} priority priority
 	 * @returns {boolean} true, if this call changed something
 	 */
 	setUnknownExportsProvided(
 		canMangle,
 		excludeExports,
 		targetKey,
-		targetModule
+		targetModule,
+		priority
 	) {
 		let changed = false;
 		if (excludeExports) {
@@ -295,7 +297,7 @@ class ExportsInfo {
 				changed = true;
 			}
 			if (targetKey) {
-				exportInfo.setTarget(targetKey, targetModule, [exportInfo.name]);
+				exportInfo.setTarget(targetKey, targetModule, [exportInfo.name], -1);
 			}
 		}
 		if (this._redirectTo !== undefined) {
@@ -304,7 +306,8 @@ class ExportsInfo {
 					canMangle,
 					excludeExports,
 					targetKey,
-					targetModule
+					targetModule,
+					priority
 				)
 			) {
 				changed = true;
@@ -322,7 +325,12 @@ class ExportsInfo {
 				changed = true;
 			}
 			if (targetKey) {
-				this._otherExportsInfo.setTarget(targetKey, targetModule, undefined);
+				this._otherExportsInfo.setTarget(
+					targetKey,
+					targetModule,
+					undefined,
+					priority
+				);
 			}
 		}
 		return changed;
@@ -819,17 +827,20 @@ class ExportInfo {
 		this.exportsInfoOwned = false;
 		/** @type {ExportsInfo=} */
 		this.exportsInfo = undefined;
-		/** @type {Map<any, { connection: ModuleGraphConnection, export: string[] } | null>=} */
+		/** @type {Map<any, { connection: ModuleGraphConnection | null, export: string[], priority: number }>=} */
 		this._target = undefined;
 		if (initFrom && initFrom._target) {
 			this._target = new Map();
 			for (const [key, value] of initFrom._target) {
-				this._target.set(
-					key,
-					value ? { connection: value.connection, export: [name] } : null
-				);
+				this._target.set(key, {
+					connection: value.connection,
+					export: value.export || [name],
+					priority: value.priority
+				});
 			}
 		}
+		/** @type {Map<any, { connection: ModuleGraphConnection | null, export: string[], priority: number }>=} */
+		this._maxTarget = undefined;
 	}
 
 	// TODO webpack 5 remove
@@ -1023,46 +1034,45 @@ class ExportInfo {
 	 */
 	unsetTarget(key) {
 		if (!this._target) return false;
-		return this._target.delete(key);
+		if (this._target.delete(key)) {
+			this._maxTarget = undefined;
+			return true;
+		}
+		return false;
 	}
 
 	/**
 	 * @param {any} key the key
-	 * @param {ModuleGraphConnection=} connection the target module if a single one
+	 * @param {ModuleGraphConnection} connection the target module if a single one
 	 * @param {string[]=} exportName the exported name
+	 * @param {number=} priority priority
 	 * @returns {boolean} true, if something has changed
 	 */
-	setTarget(key, connection, exportName) {
+	setTarget(key, connection, exportName, priority = 0) {
 		if (exportName) exportName = [...exportName];
 		if (!this._target) {
 			this._target = new Map();
-			this._target.set(
-				key,
-				connection ? { connection, export: exportName } : null
-			);
+			this._target.set(key, { connection, export: exportName, priority });
 			return true;
 		}
 		const oldTarget = this._target.get(key);
 		if (!oldTarget) {
 			if (oldTarget === null && !connection) return false;
-			this._target.set(
-				key,
-				connection ? { connection, export: exportName } : null
-			);
-			return true;
-		}
-		if (!connection) {
-			this._target.set(key, null);
+			this._target.set(key, { connection, export: exportName, priority });
+			this._maxTarget = undefined;
 			return true;
 		}
 		if (
 			oldTarget.connection !== connection ||
+			oldTarget.priority !== priority ||
 			(exportName
 				? !oldTarget.export || !equals(oldTarget.export, exportName)
 				: oldTarget.export)
 		) {
 			oldTarget.connection = connection;
 			oldTarget.export = exportName;
+			oldTarget.priority = priority;
+			this._maxTarget = undefined;
 			return true;
 		}
 		return false;
@@ -1171,6 +1181,29 @@ class ExportInfo {
 		return !this.terminalBinding && this._target && this._target.size > 0;
 	}
 
+	_getMaxTarget() {
+		if (this._maxTarget !== undefined) return this._maxTarget;
+		if (this._target.size <= 1) return (this._maxTarget = this._target);
+		let maxPriority = -Infinity;
+		let minPriority = Infinity;
+		for (const { priority } of this._target.values()) {
+			if (maxPriority < priority) maxPriority = priority;
+			if (minPriority > priority) minPriority = priority;
+		}
+		// This should be very common
+		if (maxPriority === minPriority) return (this._maxTarget = this._target);
+
+		// This is an edge case
+		const map = new Map();
+		for (const [key, value] of this._target) {
+			if (maxPriority === value.priority) {
+				map.set(key, value);
+			}
+		}
+		this._maxTarget = map;
+		return map;
+	}
+
 	/**
 	 * @param {ModuleGraph} moduleGraph the module graph
 	 * @param {function(Module): boolean} validTargetModuleFilter a valid target module
@@ -1188,7 +1221,7 @@ class ExportInfo {
 	 */
 	_findTarget(moduleGraph, validTargetModuleFilter, alreadyVisited) {
 		if (!this._target || this._target.size === 0) return undefined;
-		let rawTarget = this._target.values().next().value;
+		let rawTarget = this._getMaxTarget().values().next().value;
 		if (!rawTarget) return undefined;
 		/** @type {{ module: Module, export: string[] | undefined }} */
 		let target = {
@@ -1296,7 +1329,7 @@ class ExportInfo {
 		if (alreadyVisited && alreadyVisited.has(this)) return CIRCULAR;
 		const newAlreadyVisited = new Set(alreadyVisited);
 		newAlreadyVisited.add(this);
-		const values = this._target.values();
+		const values = this._getMaxTarget().values();
 		const target = resolveTarget(values.next().value, newAlreadyVisited);
 		if (target === CIRCULAR) return CIRCULAR;
 		if (target === null) return undefined;
@@ -1324,7 +1357,7 @@ class ExportInfo {
 		const target = this._getTarget(moduleGraph, resolveTargetFilter, undefined);
 		if (target === CIRCULAR) return undefined;
 		if (!target) return undefined;
-		const originalTarget = this._target.values().next().value;
+		const originalTarget = this._getMaxTarget().values().next().value;
 		if (
 			originalTarget.connection === target.connection &&
 			originalTarget.export === target.export
@@ -1336,7 +1369,8 @@ class ExportInfo {
 			connection: updateOriginalConnection
 				? updateOriginalConnection(target)
 				: target.connection,
-			export: target.export
+			export: target.export,
+			priority: 0
 		});
 		return target;
 	}

diff --git a/lib/FlagDependencyExportsPlugin.js b/lib/FlagDependencyExportsPlugin.js
@@ -119,6 +119,7 @@ class FlagDependencyExportsPlugin {
 									const exports = exportDesc.exports;
 									const globalCanMangle = exportDesc.canMangle;
 									const globalFrom = exportDesc.from;
+									const globalPriority = exportDesc.priority;
 									const globalTerminalBinding =
 										exportDesc.terminalBinding || false;
 									const exportDeps = exportDesc.dependencies;
@@ -135,7 +136,8 @@ class FlagDependencyExportsPlugin {
 												globalCanMangle,
 												exportDesc.excludeExports,
 												globalFrom && dep,
-												globalFrom
+												globalFrom,
+												globalPriority
 											)
 										) {
 											changed = true;
@@ -154,6 +156,7 @@ class FlagDependencyExportsPlugin {
 												let exports = undefined;
 												let from = globalFrom;
 												let fromExport = undefined;
+												let priority = globalPriority;
 												let hidden = false;
 												if (typeof exportNameOrSpec === "string") {
 													name = exportNameOrSpec;
@@ -167,14 +170,19 @@ class FlagDependencyExportsPlugin {
 														exports = exportNameOrSpec.exports;
 													if (exportNameOrSpec.from !== undefined)
 														from = exportNameOrSpec.from;
+													if (exportNameOrSpec.priority !== undefined)
+														priority = exportNameOrSpec.priority;
 													if (exportNameOrSpec.terminalBinding !== undefined)
 														terminalBinding = exportNameOrSpec.terminalBinding;
 													if (exportNameOrSpec.hidden !== undefined)
 														hidden = exportNameOrSpec.hidden;
 												}
 												const exportInfo = exportsInfo.getExportInfo(name);
 
-												if (exportInfo.provided === false) {
+												if (
+													exportInfo.provided === false ||
+													exportInfo.provided === null
+												) {
 													exportInfo.provided = true;
 													changed = true;
 												}
@@ -204,7 +212,8 @@ class FlagDependencyExportsPlugin {
 														: exportInfo.setTarget(
 																dep,
 																from,
-																fromExport === undefined ? [name] : fromExport
+																fromExport === undefined ? [name] : fromExport,
+																priority
 														  ))
 												) {
 													changed = true;

diff --git a/lib/dependencies/HarmonyExportExpressionDependency.js b/lib/dependencies/HarmonyExportExpressionDependency.js
@@ -39,6 +39,7 @@ class HarmonyExportExpressionDependency extends NullDependency {
 	getExports(moduleGraph) {
 		return {
 			exports: ["default"],
+			priority: 1,
 			terminalBinding: true,
 			dependencies: undefined
 		};

diff --git a/lib/dependencies/HarmonyExportImportedSpecifierDependency.js b/lib/dependencies/HarmonyExportImportedSpecifierDependency.js
@@ -543,6 +543,7 @@ class HarmonyExportImportedSpecifierDependency extends HarmonyImportDependency {
 						export: item.ids,
 						hidden: item.hidden
 					})),
+					priority: 1,
 					dependencies: [from.module]
 				};
 			}
@@ -557,6 +558,7 @@ class HarmonyExportImportedSpecifierDependency extends HarmonyImportDependency {
 								export: ["default"]
 							}
 						],
+						priority: 1,
 						dependencies: [from.module]
 					};
 				}
@@ -584,6 +586,7 @@ class HarmonyExportImportedSpecifierDependency extends HarmonyImportDependency {
 							]
 						}
 					],
+					priority: 1,
 					dependencies: [from.module]
 				};
 			}
@@ -597,6 +600,7 @@ class HarmonyExportImportedSpecifierDependency extends HarmonyImportDependency {
 							export: null
 						}
 					],
+					priority: 1,
 					dependencies: [from.module]
 				};
 			}
@@ -610,6 +614,7 @@ class HarmonyExportImportedSpecifierDependency extends HarmonyImportDependency {
 							export: ["default"]
 						}
 					],
+					priority: 1,
 					dependencies: [from.module]
 				};
 			}

diff --git a/lib/dependencies/HarmonyExportSpecifierDependency.js b/lib/dependencies/HarmonyExportSpecifierDependency.js
@@ -35,6 +35,7 @@ class HarmonyExportSpecifierDependency extends NullDependency {
 	getExports(moduleGraph) {
 		return {
 			exports: [this.name],
+			priority: 1,
 			terminalBinding: true,
 			dependencies: undefined
 		};

diff --git a/test/configCases/side-effects/type-reexports/a.js b/test/configCases/side-effects/type-reexports/a.js
@@ -0,0 +1 @@
+export const a = "a";
diff --git a/test/configCases/side-effects/type-reexports/b.js b/test/configCases/side-effects/type-reexports/b.js
@@ -0,0 +1 @@
+export const b = "b";
diff --git a/test/configCases/side-effects/type-reexports/empty.js b/test/configCases/side-effects/type-reexports/empty.js
diff --git a/test/configCases/side-effects/type-reexports/index.js b/test/configCases/side-effects/type-reexports/index.js
@@ -0,0 +1,14 @@
+import { a, b } from "./module";
+import * as empty from "./empty";
+
+it("should skip over module", () => {
+	empty.a = "not a";
+	empty.b = "not b";
+	expect(a).toBe("a");
+	expect(b).toBe("b");
+	expect(__STATS__.children.length).toBe(2);
+	for (const stats of __STATS__.children) {
+		const module = stats.modules.find(m => m.name.endsWith("module.js"));
+		expect(module).toHaveProperty("orphan", true);
+	}
+});
diff --git a/test/configCases/side-effects/type-reexports/module.js b/test/configCases/side-effects/type-reexports/module.js
@@ -0,0 +1,3 @@
+export * from "./a";
+export * from "./empty";
+export * from "./b";
diff --git a/test/configCases/side-effects/type-reexports/webpack.config.js b/test/configCases/side-effects/type-reexports/webpack.config.js
@@ -0,0 +1,22 @@
+module.exports = [
+	{
+		output: {
+			pathinfo: "verbose"
+		},
+		optimization: {
+			concatenateModules: true,
+			sideEffects: true,
+			usedExports: true
+		}
+	},
+	{
+		output: {
+			pathinfo: "verbose"
+		},
+		optimization: {
+			concatenateModules: false,
+			sideEffects: true,
+			usedExports: true
+		}
+	}
+];