Skip to content

Commit

Permalink
Merge pull request #17718 from bworline/require-preserve-chains
Browse files Browse the repository at this point in the history
Make CommonJS import preserve chained expressions
  • Loading branch information
TheLarkInn committed Oct 13, 2023
2 parents 1f13ff9 + b14922c commit 21c80e4
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 98 deletions.
33 changes: 27 additions & 6 deletions lib/dependencies/CommonJsFullRequireDependency.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
};

Expand Down
24 changes: 20 additions & 4 deletions lib/dependencies/CommonJsImportsParserPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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]
Expand All @@ -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 (
Expand All @@ -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(
Expand Down
76 changes: 13 additions & 63 deletions lib/dependencies/HarmonyImportSpecifierDependency.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}

/**
Expand Down
16 changes: 10 additions & 6 deletions lib/javascript/JavascriptParser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>} */
Expand Down Expand Up @@ -3274,7 +3276,8 @@ class JavascriptParser extends Parser {
expression,
exprInfo.getCalleeMembers(),
exprInfo.call,
exprInfo.getMembers()
exprInfo.getMembers(),
exprInfo.getMemberRanges()
);
if (result === true) return;
}
Expand Down Expand Up @@ -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
Expand Down
96 changes: 96 additions & 0 deletions lib/util/chainedImports.js
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit 21c80e4

Please sign in to comment.