Make CommonJS import preserve chained expressions

lib/dependencies/CommonJsFullRequireDependency.js

@@ -7,6 +7,7 @@
 
 const Template = require("../Template");
 const { equals } = require("../util/ArrayHelpers");
+const { getTrimmedIdsAndRange } = require("../util/chainedImports");
 const makeSerializable = require("../util/makeSerializable");
 const propertyAccess = require("../util/propertyAccess");
 const ModuleDependency = require("./ModuleDependency");
@@ -26,11 +27,18 @@ class CommonJsFullRequireDependency extends ModuleDependency {
 	 * @param {string} request the request string
 	 * @param {Range} range location in source code
 	 * @param {string[]} names accessed properties on module
+	 * @param {Range[]=} idRanges ranges for members of ids; the two arrays are right-aligned
 	 */
-	constructor(request, range, names) {
+	constructor(
+		request,
+		range,
+		names,
+		idRanges /* TODO webpack 6 make this non-optional. It must always be set to properly trim ids. */
+	) {
 		super(request);
 		this.range = range;
 		this.names = names;
+		this.idRanges = idRanges;
 		this.call = false;
 		this.asiSafe = undefined;
 	}
@@ -60,6 +68,7 @@ class CommonJsFullRequireDependency extends ModuleDependency {
 	serialize(context) {
 		const { write } = context;
 		write(this.names);
+		write(this.idRanges);
 		write(this.call);
 		write(this.asiSafe);
 		super.serialize(context);
@@ -71,6 +80,7 @@ class CommonJsFullRequireDependency extends ModuleDependency {
 	deserialize(context) {
 		const { read } = context;
 		this.names = read();
+		this.idRanges = read();
 		this.call = read();
 		this.asiSafe = read();
 		super.deserialize(context);
@@ -117,23 +127,34 @@ CommonJsFullRequireDependency.Template = class CommonJsFullRequireDependencyTemp
 			weak: dep.weak,
 			runtimeRequirements
 		});
+
+		const {
+			trimmedRange: [trimmedRangeStart, trimmedRangeEnd],
+			trimmedIds
+		} = getTrimmedIdsAndRange(
+			dep.names,
+			dep.range,
+			dep.idRanges,
+			moduleGraph,
+			dep
+		);
+
 		if (importedModule) {
-			const ids = dep.names;
 			const usedImported = moduleGraph
 				.getExportsInfo(importedModule)
-				.getUsedName(ids, runtime);
+				.getUsedName(trimmedIds, runtime);
 			if (usedImported) {
-				const comment = equals(usedImported, ids)
+				const comment = equals(usedImported, trimmedIds)
 					? ""
-					: Template.toNormalComment(propertyAccess(ids)) + " ";
+					: Template.toNormalComment(propertyAccess(trimmedIds)) + " ";
 				const access = `${comment}${propertyAccess(usedImported)}`;
 				requireExpr =
 					dep.asiSafe === true
 						? `(${requireExpr}${access})`
 						: `${requireExpr}${access}`;
 			}
 		}
-		source.replace(dep.range[0], dep.range[1] - 1, requireExpr);
+		source.replace(trimmedRangeStart, trimmedRangeEnd - 1, requireExpr);
 	}
 };
 

lib/dependencies/CommonJsImportsParserPlugin.js

@@ -379,9 +379,16 @@ class CommonJsImportsParserPlugin {
 		 * @param {string[]} calleeMembers callee members
 		 * @param {CallExpression} callExpr call expression
 		 * @param {string[]} members members
+		 * @param {Range[]} memberRanges member ranges
 		 * @returns {boolean | void} true when handled
 		 */
-		const chainHandler = (expr, calleeMembers, callExpr, members) => {
+		const chainHandler = (
+			expr,
+			calleeMembers,
+			callExpr,
+			members,
+			memberRanges
+		) => {
 			if (callExpr.arguments.length !== 1) return;
 			const param = parser.evaluateExpression(callExpr.arguments[0]);
 			if (
@@ -391,7 +398,8 @@ class CommonJsImportsParserPlugin {
 				const dep = new CommonJsFullRequireDependency(
 					/** @type {string} */ (param.string),
 					/** @type {Range} */ (expr.range),
-					members
+					members,
+					/** @type {Range[]} */ memberRanges
 				);
 				dep.asiSafe = !parser.isAsiPosition(
 					/** @type {Range} */ (expr.range)[0]
@@ -407,9 +415,16 @@ class CommonJsImportsParserPlugin {
 		 * @param {string[]} calleeMembers callee members
 		 * @param {CallExpression} callExpr call expression
 		 * @param {string[]} members members
+		 * @param {Range[]} memberRanges member ranges
 		 * @returns {boolean | void} true when handled
 		 */
-		const callChainHandler = (expr, calleeMembers, callExpr, members) => {
+		const callChainHandler = (
+			expr,
+			calleeMembers,
+			callExpr,
+			members,
+			memberRanges
+		) => {
 			if (callExpr.arguments.length !== 1) return;
 			const param = parser.evaluateExpression(callExpr.arguments[0]);
 			if (
@@ -419,7 +434,8 @@ class CommonJsImportsParserPlugin {
 				const dep = new CommonJsFullRequireDependency(
 					/** @type {string} */ (param.string),
 					/** @type {Range} */ (expr.callee.range),
-					members
+					members,
+					/** @type {Range[]} */ memberRanges
 				);
 				dep.call = true;
 				dep.asiSafe = !parser.isAsiPosition(

lib/dependencies/HarmonyImportSpecifierDependency.js

@@ -9,6 +9,7 @@ const Dependency = require("../Dependency");
 const {
 	getDependencyUsedByExportsCondition
 } = require("../optimize/InnerGraph");
+const { getTrimmedIdsAndRange } = require("../util/chainedImports");
 const makeSerializable = require("../util/makeSerializable");
 const propertyAccess = require("../util/propertyAccess");
 const HarmonyImportDependency = require("./HarmonyImportDependency");
@@ -324,30 +325,16 @@ HarmonyImportSpecifierDependency.Template = class HarmonyImportSpecifierDependen
 		// Skip rendering depending when dependency is conditional
 		if (connection && !connection.isTargetActive(runtime)) return;
 
-		const ids = dep.getIds(moduleGraph); // determine minimal set of IDs.
-		let trimmedIds = this._trimIdsToThoseImported(ids, moduleGraph, dep);
-
-		let [rangeStart, rangeEnd] = dep.range;
-		if (trimmedIds.length !== ids.length) {
-			// The array returned from dep.idRanges is right-aligned with the array returned from dep.getIds.
-			// Meaning, the two arrays may not always have the same number of elements, but the last element of
-			// dep.idRanges corresponds to [the expression fragment to the left of] the last element of dep.getIds.
-			// Use this to find the correct replacement range based on the number of ids that were trimmed.
-			const idx =
-				dep.idRanges === undefined
-					? -1 /* trigger failure case below */
-					: dep.idRanges.length + (trimmedIds.length - ids.length);
-			if (idx < 0 || idx >= dep.idRanges.length) {
-				// cspell:ignore minifiers
-				// Should not happen but we can't throw an error here because of backward compatibility with
-				// external plugins in wp5.  Instead, we just disable trimming for now.  This may break some minifiers.
-				trimmedIds = ids;
-				// TODO webpack 6 remove the "trimmedIds = ids" above and uncomment the following line instead.
-				// throw new Error("Missing range starts data for id replacement trimming.");
-			} else {
-				[rangeStart, rangeEnd] = dep.idRanges[idx];
-			}
-		}
+		const {
+			trimmedRange: [trimmedRangeStart, trimmedRangeEnd],
+			trimmedIds
+		} = getTrimmedIdsAndRange(
+			dep.getIds(moduleGraph),
+			dep.range,
+			dep.idRanges,
+			moduleGraph,
+			dep
+		);
 
 		const exportExpr = this._getCodeForIds(
 			dep,
@@ -356,47 +343,10 @@ HarmonyImportSpecifierDependency.Template = class HarmonyImportSpecifierDependen
 			trimmedIds
 		);
 		if (dep.shorthand) {
-			source.insert(rangeEnd, `: ${exportExpr}`);
+			source.insert(trimmedRangeEnd, `: ${exportExpr}`);
 		} else {
-			source.replace(rangeStart, rangeEnd - 1, exportExpr);
-		}
-	}
-
-	/**
-	 * @summary Determine which IDs in the id chain are actually referring to namespaces or imports,
-	 * and which are deeper member accessors on the imported object.  Only the former should be re-rendered.
-	 * @param {string[]} ids ids
-	 * @param {ModuleGraph} moduleGraph moduleGraph
-	 * @param {HarmonyImportSpecifierDependency} dependency dependency
-	 * @returns {string[]} generated code
-	 */
-	_trimIdsToThoseImported(ids, moduleGraph, dependency) {
-		/** @type {string[]} */
-		let trimmedIds = [];
-		const exportsInfo = moduleGraph.getExportsInfo(
-			/** @type {Module} */ (moduleGraph.getModule(dependency))
-		);
-		let currentExportsInfo = /** @type {ExportsInfo=} */ exportsInfo;
-		for (let i = 0; i < ids.length; i++) {
-			if (i === 0 && ids[i] === "default") {
-				continue; // ExportInfo for the next level under default is still at the root ExportsInfo, so don't advance currentExportsInfo
-			}
-			const exportInfo = currentExportsInfo.getExportInfo(ids[i]);
-			if (exportInfo.provided === false) {
-				// json imports have nested ExportInfo for elements that things that are not actually exported, so check .provided
-				trimmedIds = ids.slice(0, i);
-				break;
-			}
-			const nestedInfo = exportInfo.getNestedExportsInfo();
-			if (!nestedInfo) {
-				// once all nested exports are traversed, the next item is the actual import so stop there
-				trimmedIds = ids.slice(0, i + 1);
-				break;
-			}
-			currentExportsInfo = nestedInfo;
+			source.replace(trimmedRangeStart, trimmedRangeEnd - 1, exportExpr);
 		}
-		// Never trim to nothing.  This can happen for invalid imports (e.g. import { notThere } from "./module", or import { anything } from "./missingModule")
-		return trimmedIds.length ? trimmedIds : ids;
 	}
 
 	/**

lib/javascript/JavascriptParser.js

@@ -363,25 +363,27 @@ class JavascriptParser extends Parser {
 					])
 			),
 			/** Something like "a.b().c.d" */
-			/** @type {HookMap<SyncBailHook<[Expression, string[], CallExpression, string[]], boolean | void>>} */
+			/** @type {HookMap<SyncBailHook<[Expression, string[], CallExpression, string[], Range[]], boolean | void>>} */
 			memberChainOfCallMemberChain: new HookMap(
 				() =>
 					new SyncBailHook([
 						"expression",
 						"calleeMembers",
 						"callExpression",
-						"members"
+						"members",
+						"memberRanges"
 					])
 			),
 			/** Something like "a.b().c.d()"" */
-			/** @type {HookMap<SyncBailHook<[CallExpression, string[], CallExpression, string[]], boolean | void>>} */
+			/** @type {HookMap<SyncBailHook<[CallExpression, string[], CallExpression, string[], Range[]], boolean | void>>} */
 			callMemberChainOfCallMemberChain: new HookMap(
 				() =>
 					new SyncBailHook([
 						"expression",
 						"calleeMembers",
 						"innerCallExpression",
-						"members"
+						"members",
+						"memberRanges"
 					])
 			),
 			/** @type {SyncBailHook<[ChainExpression], boolean | void>} */
@@ -3274,7 +3276,8 @@ class JavascriptParser extends Parser {
 						expression,
 						exprInfo.getCalleeMembers(),
 						exprInfo.call,
-						exprInfo.getMembers()
+						exprInfo.getMembers(),
+						exprInfo.getMemberRanges()
 					);
 					if (result === true) return;
 				}
@@ -3365,7 +3368,8 @@ class JavascriptParser extends Parser {
 						expression,
 						exprInfo.getCalleeMembers(),
 						exprInfo.call,
-						exprInfo.getMembers()
+						exprInfo.getMembers(),
+						exprInfo.getMemberRanges()
 					);
 					if (result === true) return;
 					// Fast skip over the member chain as we already called memberChainOfCallMemberChain

lib/util/chainedImports.js

@@ -0,0 +1,96 @@
+/*
+	MIT License http://www.opensource.org/licenses/mit-license.php
+	Author Tobias Koppers @sokra
+*/
+
+"use strict";
+
+/** @typedef {import("../Dependency")} Dependency */
+/** @typedef {import("../ModuleGraph")} ModuleGraph */
+/** @typedef {import("../javascript/JavascriptParser").Range} Range */
+
+/**
+ * @summary Get the subset of ids and their corresponding range in an id chain that should be re-rendered by webpack.
+ * Only those in the chain that are actually referring to namespaces or imports should be re-rendered.
+ * Deeper member accessors on the imported object should not be re-rendered.  If deeper member accessors are re-rendered,
+ * there is a potential loss of meaning with rendering a quoted accessor as an unquoted accessor, or vice versa,
+ * because minifiers treat quoted accessors differently.  e.g. import { a } from "./module"; a["b"] vs a.b
+ * @param {string[]} untrimmedIds chained ids
+ * @param {Range} untrimmedRange range encompassing allIds
+ * @param {Range[]} ranges cumulative range of ids for each of allIds
+ * @param {ModuleGraph} moduleGraph moduleGraph
+ * @param {Dependency} dependency dependency
+ * @returns {{trimmedIds: string[], trimmedRange: Range}} computed trimmed ids and cumulative range of those ids
+ */
+exports.getTrimmedIdsAndRange = (
+	untrimmedIds,
+	untrimmedRange,
+	ranges,
+	moduleGraph,
+	dependency
+) => {
+	let trimmedIds = trimIdsToThoseImported(
+		untrimmedIds,
+		moduleGraph,
+		dependency
+	);
+	let trimmedRange = untrimmedRange;
+	if (trimmedIds.length !== untrimmedIds.length) {
+		// The array returned from dep.idRanges is right-aligned with the array returned from dep.names.
+		// Meaning, the two arrays may not always have the same number of elements, but the last element of
+		// dep.idRanges corresponds to [the expression fragment to the left of] the last element of dep.names.
+		// Use this to find the correct replacement range based on the number of ids that were trimmed.
+		const idx =
+			ranges === undefined
+				? -1 /* trigger failure case below */
+				: ranges.length + (trimmedIds.length - untrimmedIds.length);
+		if (idx < 0 || idx >= ranges.length) {
+			// cspell:ignore minifiers
+			// Should not happen but we can't throw an error here because of backward compatibility with
+			// external plugins in wp5.  Instead, we just disable trimming for now.  This may break some minifiers.
+			trimmedIds = untrimmedIds;
+			// TODO webpack 6 remove the "trimmedIds = ids" above and uncomment the following line instead.
+			// throw new Error("Missing range starts data for id replacement trimming.");
+		} else {
+			trimmedRange = ranges[idx];
+		}
+	}
+
+	return { trimmedIds, trimmedRange };
+};
+
+/**
+ * @summary Determine which IDs in the id chain are actually referring to namespaces or imports,
+ * and which are deeper member accessors on the imported object.
+ * @param {string[]} ids untrimmed ids
+ * @param {ModuleGraph} moduleGraph moduleGraph
+ * @param {Dependency} dependency dependency
+ * @returns {string[]} trimmed ids
+ */
+function trimIdsToThoseImported(ids, moduleGraph, dependency) {
+	let trimmedIds = [];
+	const exportsInfo = moduleGraph.getExportsInfo(
+		moduleGraph.getModule(dependency)
+	);
+	let currentExportsInfo = /** @type {ExportsInfo=} */ exportsInfo;
+	for (let i = 0; i < ids.length; i++) {
+		if (i === 0 && ids[i] === "default") {
+			continue; // ExportInfo for the next level under default is still at the root ExportsInfo, so don't advance currentExportsInfo
+		}
+		const exportInfo = currentExportsInfo.getExportInfo(ids[i]);
+		if (exportInfo.provided === false) {
+			// json imports have nested ExportInfo for elements that things that are not actually exported, so check .provided
+			trimmedIds = ids.slice(0, i);
+			break;
+		}
+		const nestedInfo = exportInfo.getNestedExportsInfo();
+		if (!nestedInfo) {
+			// once all nested exports are traversed, the next item is the actual import so stop there
+			trimmedIds = ids.slice(0, i + 1);
+			break;
+		}
+		currentExportsInfo = nestedInfo;
+	}
+	// Never trim to nothing.  This can happen for invalid imports (e.g. import { notThere } from "./module", or import { anything } from "./missingModule")
+	return trimmedIds.length ? trimmedIds : ids;
+}