Merge pull request #15868 from webpack/add-concatenate-modules-test-case

add concatenate modules side effects test case, fix imports order

lib/optimize/ConcatenatedModule.js

@@ -21,7 +21,7 @@ const HarmonyImportDependency = require("../dependencies/HarmonyImportDependency
 const JavascriptParser = require("../javascript/JavascriptParser");
 const { equals } = require("../util/ArrayHelpers");
 const LazySet = require("../util/LazySet");
-const { concatComparators, keepOriginalOrder } = require("../util/comparators");
+const { concatComparators } = require("../util/comparators");
 const createHash = require("../util/createHash");
 const { makePathsRelative } = require("../util/identifier");
 const makeSerializable = require("../util/makeSerializable");
@@ -185,23 +185,25 @@ const RESERVED_NAMES = new Set(
 		.split(",")
 );
 
-const bySourceOrder = (a, b) => {
-	const aOrder = a.sourceOrder;
-	const bOrder = b.sourceOrder;
-	if (isNaN(aOrder)) {
-		if (!isNaN(bOrder)) {
+const createComparator = (property, comparator) => (a, b) =>
+	comparator(a[property], b[property]);
+const compareNumbers = (a, b) => {
+	if (isNaN(a)) {
+		if (!isNaN(b)) {
 			return 1;
 		}
 	} else {
-		if (isNaN(bOrder)) {
+		if (isNaN(b)) {
 			return -1;
 		}
-		if (aOrder !== bOrder) {
-			return aOrder < bOrder ? -1 : 1;
+		if (a !== b) {
+			return a < b ? -1 : 1;
 		}
 	}
 	return 0;
 };
+const bySourceOrder = createComparator("sourceOrder", compareNumbers);
+const byRangeStart = createComparator("rangeStart", compareNumbers);
 
 const joinIterableWithComma = iterable => {
 	// This is more performant than Array.from().join(", ")
@@ -885,6 +887,9 @@ class ConcatenatedModule extends Module {
 				for (const c of moduleGraph.getOutgoingConnections(this))
 					connections.push(c);
 			}
+			/**
+			 * @type {Array<{ connection: ModuleGraphConnection, sourceOrder: number, rangeStart: number }>}
+			 */
 			const references = connections
 				.filter(connection => {
 					if (!(connection.dependency instanceof HarmonyImportDependency))
@@ -896,15 +901,33 @@ class ConcatenatedModule extends Module {
 						connection.isTargetActive(runtime)
 					);
 				})
-				.map(connection => ({
-					connection,
-					sourceOrder: /** @type {HarmonyImportDependency} */ (
+				.map(connection => {
+					const dep = /** @type {HarmonyImportDependency} */ (
 						connection.dependency
-					).sourceOrder
-				}));
-			references.sort(
-				concatComparators(bySourceOrder, keepOriginalOrder(references))
-			);
+					);
+					return {
+						connection,
+						sourceOrder: dep.sourceOrder,
+						rangeStart: dep.range && dep.range[0]
+					};
+				});
+			/**
+			 * bySourceOrder
+			 * @example
+			 * import a from "a"; // sourceOrder=1
+			 * import b from "b"; // sourceOrder=2
+			 *
+			 * byRangeStart
+			 * @example
+			 * import {a, b} from "a"; // sourceOrder=1
+			 * a.a(); // first range
+			 * b.b(); // second range
+			 *
+			 * If there is no reexport, we have the same source.
+			 * If there is reexport, but module has side effects, this will lead to reexport module only.
+			 * If there is side-effects-free reexport, we can get simple deterministic result with range start comparison.
+			 */
+			references.sort(concatComparators(bySourceOrder, byRangeStart));
 			/** @type {Map<Module, { connection: ModuleGraphConnection, runtimeCondition: RuntimeSpec | true }>} */
 			const referencesMap = new Map();
 			for (const { connection } of references) {

test/configCases/concatenate-modules/side-effects/index.js

@@ -0,0 +1,11 @@
+import { b, a, c } from "dep";
+
+c.cc();
+b.bbb();
+a.aa();
+
+import { order } from "dep/order.js";
+
+it("should import side-effect-free modules in deterministic order (usage order)", () => {
+	expect(order).toEqual(["c", "b", "a"]);
+});

test/configCases/concatenate-modules/side-effects/node_modules/dep/a.js

@@ -0,0 +1,4 @@
+import { track } from "./order.js";
+track("a");
+export function aa() {}
+export function aaa() {}

test/configCases/concatenate-modules/side-effects/node_modules/dep/b.js

@@ -0,0 +1,4 @@
+import { track } from "./order.js";
+track("b");
+export function bb() {}
+export function bbb() {}

test/configCases/concatenate-modules/side-effects/node_modules/dep/c.js

@@ -0,0 +1,4 @@
+import { track } from "./order.js";
+track("c");
+export function cc() {}
+export function ccc() {}

test/configCases/concatenate-modules/side-effects/node_modules/dep/index.js

@@ -0,0 +1,8 @@
+import * as a from "./a.js";
+import * as b from "./b.js";
+import * as c from "./c.js";
+export {
+	a,
+	b,
+	c
+}

test/configCases/concatenate-modules/side-effects/node_modules/dep/order.js

@@ -0,0 +1,4 @@
+export let order = [];
+export function track(name) {
+	order.push(name);
+}

test/configCases/concatenate-modules/side-effects/node_modules/dep/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "dep",
+  "version": "1.0.0",
+  "type": "module",
+  "sideEffects": false
+}

test/configCases/concatenate-modules/side-effects/webpack.config.js

@@ -0,0 +1,6 @@
+/** @type {import("../../../../").Configuration} */
+module.exports = {
+	optimization: {
+		concatenateModules: true
+	}
+};