Merge pull request webpack#7507 from webpack/feature/dependency-order

sokra · web-flow · commit 6f90257b5b63 · 2018-06-20T12:17:26.000+02:00
add order to dependency reference
diff --git a/lib/ContextModule.js b/lib/ContextModule.js
@@ -10,6 +10,8 @@ const Module = require("./Module");
 const AsyncDependenciesBlock = require("./AsyncDependenciesBlock");
 const Template = require("./Template");
 
+/** @typedef {import("./dependencies/ContextElementDependency")} ContextElementDependency */
+
 class ContextModule extends Module {
 	// type ContextMode = "sync" | "eager" | "weak" | "async-weak" | "lazy" | "lazy-once"
 	// type ContextOptions = { resource: string, recursive: boolean, regExp: RegExp, addon?: string, mode?: ContextMode, chunkName?: string, include?: RegExp, exclude?: RegExp, groupOptions?: Object }
@@ -684,10 +686,10 @@ webpackEmptyAsyncContext.id = ${JSON.stringify(id)};`;
 		const initialSize = 160;
 
 		// if we dont have dependencies we stop here.
-		return this.dependencies.reduce(
-			(size, dependency) => size + 5 + dependency.userRequest.length,
-			initialSize
-		);
+		return this.dependencies.reduce((size, dependency) => {
+			const element = /** @type {ContextElementDependency} */ (dependency);
+			return size + 5 + element.userRequest.length;
+		}, initialSize);
 	}
 }
 
diff --git a/lib/DelegatedModule.js b/lib/DelegatedModule.js
@@ -56,7 +56,7 @@ class DelegatedModule extends Module {
 	}
 
 	source(depTemplates, runtime) {
-		const dep = this.dependencies[0];
+		const dep = /** @type {DelegatedSourceDependency} */ (this.dependencies[0]);
 		const sourceModule = dep.module;
 		let str;
 
diff --git a/lib/DependenciesBlock.js b/lib/DependenciesBlock.js
@@ -7,9 +7,11 @@
 const DependenciesBlockVariable = require("./DependenciesBlockVariable");
 
 /** @typedef {import("./ChunkGroup")} ChunkGroup */
+/** @typedef {import("./Dependency")} Dependency */
 
 class DependenciesBlock {
 	constructor() {
+		/** @type {Dependency[]} */
 		this.dependencies = [];
 		this.blocks = [];
 		this.variables = [];
diff --git a/lib/Dependency.js b/lib/Dependency.js
@@ -10,6 +10,7 @@ const DependencyReference = require("./dependencies/DependencyReference");
 class Dependency {
 	constructor() {
 		this.module = null;
+		// TODO remove in webpack 5
 		this.weak = false;
 		this.optional = false;
 		this.loc = undefined;
diff --git a/lib/FlagDependencyUsagePlugin.js b/lib/FlagDependencyUsagePlugin.js
@@ -72,6 +72,7 @@ class FlagDependencyUsagePlugin {
 					};
 
 					const processDependency = dep => {
+						// TODO remove dep.getReference existance check in webpack 5
 						const reference = dep.getReference && dep.getReference();
 						if (!reference) return;
 						const module = reference.module;
diff --git a/lib/dependencies/DependencyReference.js b/lib/dependencies/DependencyReference.js
@@ -4,14 +4,56 @@
 */
 "use strict";
 
+/** @typedef {import("../Module")} Module */
+
 class DependencyReference {
-	constructor(module, importedNames, weak) {
+	/**
+	 *
+	 * @param {Module} module the referenced module
+	 * @param {string[] | boolean} importedNames imported named from the module
+	 * @param {boolean=} weak if this is a weak reference
+	 * @param {number} order the order information or NaN if don't care
+	 */
+	constructor(module, importedNames, weak = false, order = NaN) {
 		this.module = module;
 		// true: full object
 		// false: only sideeffects/no export
 		// array of strings: the exports with this names
 		this.importedNames = importedNames;
-		this.weak = weak;
+		this.weak = !!weak;
+		this.order = order;
+	}
+
+	/**
+	 * @param {DependencyReference[]} array an array (will be modified)
+	 * @returns {DependencyReference[]} the array again
+	 */
+	static sort(array) {
+		/** @type {WeakMap<DependencyReference, number>} */
+		const originalOrder = new WeakMap();
+		let i = 0;
+		for (const ref of array) {
+			originalOrder.set(ref, i++);
+		}
+		return array.sort((a, b) => {
+			const aOrder = a.order;
+			const bOrder = b.order;
+			if (isNaN(aOrder)) {
+				if (!isNaN(bOrder)) {
+					return 1;
+				}
+			} else {
+				if (isNaN(bOrder)) {
+					return -1;
+				}
+				if (aOrder !== bOrder) {
+					return aOrder - bOrder;
+				}
+			}
+			const aOrg = originalOrder.get(a);
+			const bOrg = originalOrder.get(b);
+			return aOrg - bOrg;
+		});
 	}
 }
 
diff --git a/lib/dependencies/HarmonyExportImportedSpecifierDependency.js b/lib/dependencies/HarmonyExportImportedSpecifierDependency.js
@@ -221,13 +221,23 @@ class HarmonyExportImportedSpecifierDependency extends HarmonyImportDependency {
 
 			case "reexport-non-harmony-default":
 			case "reexport-named-default":
-				return new DependencyReference(mode.module, ["default"], false);
+				return new DependencyReference(
+					mode.module,
+					["default"],
+					false,
+					this.sourceOrder
+				);
 
 			case "reexport-namespace-object":
 			case "reexport-non-harmony-default-strict":
 			case "reexport-fake-namespace-object":
 			case "rexport-non-harmony-undefined":
-				return new DependencyReference(mode.module, true, false);
+				return new DependencyReference(
+					mode.module,
+					true,
+					false,
+					this.sourceOrder
+				);
 
 			case "safe-reexport":
 			case "checked-reexport":
@@ -238,7 +248,12 @@ class HarmonyExportImportedSpecifierDependency extends HarmonyImportDependency {
 				);
 
 			case "dynamic-reexport":
-				return new DependencyReference(mode.module, true, false);
+				return new DependencyReference(
+					mode.module,
+					true,
+					false,
+					this.sourceOrder
+				);
 
 			default:
 				throw new Error(`Unknown mode ${mode.type}`);
diff --git a/lib/dependencies/HarmonyImportDependency.js b/lib/dependencies/HarmonyImportDependency.js
@@ -23,7 +23,12 @@ class HarmonyImportDependency extends ModuleDependency {
 
 	getReference() {
 		if (!this._module) return null;
-		return new DependencyReference(this._module, false, this.weak);
+		return new DependencyReference(
+			this._module,
+			false,
+			this.weak,
+			this.sourceOrder
+		);
 	}
 
 	getImportVar() {
diff --git a/lib/dependencies/HarmonyImportSpecifierDependency.js b/lib/dependencies/HarmonyImportSpecifierDependency.js
@@ -45,7 +45,8 @@ class HarmonyImportSpecifierDependency extends HarmonyImportDependency {
 		return new DependencyReference(
 			this._module,
 			this._id && !this.namespaceObjectAsContext ? [this._id] : true,
-			false
+			false,
+			this.sourceOrder
 		);
 	}
 
diff --git a/lib/optimize/ConcatenatedModule.js b/lib/optimize/ConcatenatedModule.js
@@ -9,6 +9,7 @@ const Template = require("../Template");
 const Parser = require("../Parser");
 const eslintScope = require("eslint-scope");
 const { ConcatSource, ReplaceSource } = require("webpack-sources");
+const DependencyReference = require("../dependencies/DependencyReference");
 const HarmonyImportDependency = require("../dependencies/HarmonyImportDependency");
 const HarmonyImportSideEffectDependency = require("../dependencies/HarmonyImportSideEffectDependency");
 const HarmonyImportSpecifierDependency = require("../dependencies/HarmonyImportSpecifierDependency");
@@ -18,6 +19,8 @@ const HarmonyExportImportedSpecifierDependency = require("../dependencies/Harmon
 const HarmonyCompatibilityDependency = require("../dependencies/HarmonyCompatibilityDependency");
 const createHash = require("../util/createHash");
 
+/** @typedef {import("../Dependency")} Dependency */
+
 const ensureNsObjSource = (
 	info,
 	moduleToInfoMap,
@@ -410,14 +413,26 @@ class ConcatenatedModule extends Module {
 		const list = [];
 		const set = new Set();
 
+		/**
+		 * @param {Module} module a module
+		 * @returns {(function(): Module)[]} imported modules in order
+		 */
 		const getConcatenatedImports = module => {
-			return module.dependencies
+			/** @type {WeakMap<DependencyReference, Dependency>} */
+			const map = new WeakMap();
+			const references = module.dependencies
 				.filter(dep => dep instanceof HarmonyImportDependency)
-				.sort((a, b) => a.sourceOrder - b.sourceOrder)
-				.map(dep => () => {
+				.map(dep => {
 					const ref = dep.getReference();
-					return ref && ref.module;
-				});
+					if (ref) map.set(ref, dep);
+					return ref;
+				})
+				.filter(ref => ref);
+			DependencyReference.sort(references);
+			return references.map(ref => {
+				const dep = map.get(ref);
+				return () => dep.getReference().module;
+			});
 		};
 
 		const enterModule = getModule => {
diff --git a/lib/optimize/ModuleConcatenationPlugin.js b/lib/optimize/ModuleConcatenationPlugin.js
@@ -321,29 +321,27 @@ class ModuleConcatenationPlugin {
 	}
 
 	getImports(module) {
-		return Array.from(
-			new Set(
-				module.dependencies
-
-					// Only harmony Dependencies
-					.filter(dep => dep instanceof HarmonyImportDependency)
-
-					// Get reference info for this dependency
-					.map(dep => dep.getReference())
-
-					// Reference is valid and has a module
-					.filter(ref => ref && ref.module)
-
-					// Dependencies are simple enough to concat them
-					.filter(
-						ref =>
-							Array.isArray(ref.importedNames) ||
-							Array.isArray(ref.module.buildMeta.providedExports)
-					)
-
-					// Take the imported module
-					.map(ref => ref.module)
-			)
+		return new Set(
+			module.dependencies
+
+				// Get reference info only for harmony Dependencies
+				.map(
+					dep =>
+						dep instanceof HarmonyImportDependency ? dep.getReference() : null
+				)
+
+				// Reference is valid and has a module
+				// Dependencies are simple enough to concat them
+				.filter(
+					ref =>
+						ref &&
+						ref.module &&
+						(Array.isArray(ref.importedNames) ||
+							Array.isArray(ref.module.buildMeta.providedExports))
+				)
+
+				// Take the imported module
+				.map(ref => ref.module)
 		);
 	}
 
diff --git a/test/cases/scope-hoisting/order-without-side-effects/a.js b/test/cases/scope-hoisting/order-without-side-effects/a.js
@@ -0,0 +1,3 @@
+import { array } from "./tracker";
+array.push("a");
+export var a = 1;
diff --git a/test/cases/scope-hoisting/order-without-side-effects/b.js b/test/cases/scope-hoisting/order-without-side-effects/b.js
@@ -0,0 +1,3 @@
+import { array } from "./tracker";
+array.push("b");
+export var b = 2;
diff --git a/test/cases/scope-hoisting/order-without-side-effects/index.js b/test/cases/scope-hoisting/order-without-side-effects/index.js
@@ -0,0 +1,9 @@
+import { array } from "./tracker";
+import { a } from "./a";
+import { b } from "./b";
+
+it("should concatenate in correct order", function() {
+	expect(b).toBe(2);
+	expect(a).toBe(1);
+	expect(array).toEqual(["a", "b"]);
+});
diff --git a/test/cases/scope-hoisting/order-without-side-effects/package.json b/test/cases/scope-hoisting/order-without-side-effects/package.json
@@ -0,0 +1,3 @@
+{
+  "sideEffects": false
+}
diff --git a/test/cases/scope-hoisting/order-without-side-effects/tracker.js b/test/cases/scope-hoisting/order-without-side-effects/tracker.js
@@ -0,0 +1 @@
+export var array = [];

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ class DelegatedModule extends Module {`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`source(depTemplates, runtime) {`
`59`		`- const dep = this.dependencies[0];`
	`59`	`+ const dep = /** @type {DelegatedSourceDependency} */ (this.dependencies[0]);`
`60`	`60`	`const sourceModule = dep.module;`
`61`	`61`	`let str;`
`62`	`62`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,8 @@ class HarmonyImportSpecifierDependency extends HarmonyImportDependency {`
`45`	`45`	`return new DependencyReference(`
`46`	`46`	`this._module,`
`47`	`47`	`this._id && !this.namespaceObjectAsContext ? [this._id] : true,`
`48`		`- false`
	`48`	`+ false,`
	`49`	`+ this.sourceOrder`
`49`	`50`	`);`
`50`	`51`	`}`
`51`	`52`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import { array } from "./tracker";`
	`2`	`+array.push("a");`
	`3`	`+export var a = 1;`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import { array } from "./tracker";`
	`2`	`+array.push("b");`
	`3`	`+export var b = 2;`