Skip to content

Commit d20258f

Browse files
hai-xhai-x
committed
feat: recognize a required binding assigned to exports as a re-export
1 parent 448a912 commit d20258f

22 files changed

Lines changed: 241 additions & 23 deletions

File tree

‎.changeset/009-cjs-require-binding-reexport.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack": minor
3+
---
4+
5+
Support CJS tree-shaking better by recognizing a required `const` binding assigned to `exports` as a re-export.

‎lib/dependencies/commonjs/CommonJsExportRequireDependency.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,28 @@ class CommonJsExportRequireDependency extends ModuleDependency {
241241
: ids.length === 0
242242
? null
243243
: ids;
244+
245+
// `require()` of a namespace module returns an object carrying `__esModule`
246+
let namespaceMarker = false;
247+
if (
248+
!esmUnwrap &&
249+
ids.length === 0 &&
250+
importedModule &&
251+
importedModule.getExportsType(moduleGraph, false) === "namespace"
252+
) {
253+
const importedExportsInfo = moduleGraph.getExportsInfo(importedModule);
254+
namespaceMarker = !(
255+
importedExportsInfo.otherExportsInfo.provided === false &&
256+
importedExportsInfo.ownedExports[Symbol.iterator]().next().done
257+
);
258+
}
244259
return {
245260
exports: [
246261
{
247262
name,
248263
from,
249264
export: exportChain,
265+
exports: namespaceMarker ? ["__esModule"] : undefined,
250266
// we can't mangle names that are in an empty object
251267
// because one could access the prototype property
252268
// when export isn't set yet

‎lib/dependencies/commonjs/CommonJsExportsParserPlugin.js‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ const {
1111
const RuntimeGlobals = require("../../runtime/RuntimeGlobals");
1212
const { recordOptimizationBailout } = require("../../util/buildDiagnostics");
1313
const formatLocation = require("../../util/formatLocation");
14+
const memoize = require("../../util/memoize");
1415
const { propertyAccess } = require("../../util/property");
16+
1517
const RuntimeRequirementsDependency = require("../core/RuntimeRequirementsDependency");
1618
const ESMExports = require("../esm/ESMExports");
1719
const DynamicExports = require("../helpers/DynamicExports");
@@ -23,6 +25,10 @@ const CommonJsRequireDependency = require("./CommonJsRequireDependency");
2325
const CommonJsSelfReferenceDependency = require("./CommonJsSelfReferenceDependency");
2426
const ModuleDecoratorDependency = require("./ModuleDecoratorDependency");
2527

28+
const getRequireBindingTag = memoize(
29+
() => require("./CommonJsImportsParserPlugin").REQUIRE_BINDING_TAG
30+
);
31+
2632
const FUNCTION_PROPERTY_DROP_HEAD =
2733
CommonJsObjectExportDependency.FUNCTION_PROPERTY_DROP_HEAD;
2834

@@ -47,7 +53,7 @@ const FUNCTION_PROPERTY_DROP_HEAD =
4753
*/
4854
/** @import { ExportInfoName } from "../../graph/Dependency" */
4955
/** @import { BuildInfo } from "../../module/Module" */
50-
/** @import BasicEvaluatedExpression from "../../javascript/BasicEvaluatedExpression" */
56+
5157
/**
5258
* @import JavascriptParser, {
5359
* Range,
@@ -65,6 +71,7 @@ const FUNCTION_PROPERTY_DROP_HEAD =
6571
* CommonJsObjectExportKind
6672
* } from "./CommonJsObjectExportDependency"
6773
*/
74+
/** @import { RequireBindingData } from "./CommonJsImportsParserPlugin" */
6875
/**
6976
* @import {
7077
* JavascriptModuleBuildMeta
@@ -133,6 +140,9 @@ const getReexportOfPropertyDescriptor = (expr) => {
133140
(getFn.type === "FunctionExpression" ||
134141
getFn.type === "ArrowFunctionExpression")
135142
) {
143+
// A parameter or the function's own name would shadow a `const` binding the body names.
144+
if (getFn.params.length > 0) return;
145+
if (getFn.type === "FunctionExpression" && getFn.id) return;
136146
// Arrow with expression body: `() => require("./x")`.
137147
if (getFn.body.type !== "BlockStatement") {
138148
return { expr: /** @type {Expression} */ (getFn.body), getter: true };
@@ -183,10 +193,10 @@ const isFalsyLiteral = (expr) => {
183193
};
184194

185195
/**
186-
* Parses require call.
196+
* Parses a `require()` call, or a `const` binding of one, under a static member chain.
187197
* @param {JavascriptParser} parser the parser
188198
* @param {Expression} expr expression
189-
* @returns {{ argument: BasicEvaluatedExpression, ids: ExportInfoName[] } | undefined} parsed call
199+
* @returns {{ request: string, ids: ExportInfoName[] } | undefined} parsed call
190200
*/
191201
const parseRequireCall = (parser, expr) => {
192202
/** @type {ExportInfoName[]} */
@@ -204,6 +214,13 @@ const parseRequireCall = (parser, expr) => {
204214
}
205215
expr = expr.object;
206216
}
217+
if (expr.type === "Identifier") {
218+
const binding =
219+
/** @type {RequireBindingData | undefined} */
220+
(parser.getTagData(expr.name, getRequireBindingTag()));
221+
if (!binding) return;
222+
return { request: binding.request, ids: ids.reverse() };
223+
}
207224
if (expr.type !== "CallExpression" || expr.arguments.length !== 1) return;
208225
const callee = expr.callee;
209226
if (
@@ -215,7 +232,11 @@ const parseRequireCall = (parser, expr) => {
215232
const arg = expr.arguments[0];
216233
if (arg.type === "SpreadElement") return;
217234
const argValue = parser.evaluateExpression(arg);
218-
return { argument: argValue, ids: ids.reverse() };
235+
if (!argValue.isString()) return;
236+
return {
237+
request: /** @type {string} */ (argValue.string),
238+
ids: ids.reverse()
239+
};
219240
};
220241

221242
/**
@@ -474,7 +495,6 @@ class CommonJsExportsParserPlugin {
474495
const requireCall = parseRequireCall(parser, expr.right);
475496
if (
476497
requireCall &&
477-
requireCall.argument.isString() &&
478498
(members.length === 0 || members[0] !== "__esModule")
479499
) {
480500
enableStructuredExports();
@@ -485,7 +505,7 @@ class CommonJsExportsParserPlugin {
485505
null,
486506
base,
487507
members,
488-
/** @type {string} */ (requireCall.argument.string),
508+
requireCall.request,
489509
requireCall.ids,
490510
!parser.isStatementLevelExpression(expr)
491511
);
@@ -676,13 +696,13 @@ class CommonJsExportsParserPlugin {
676696
const reexport = getReexportOfPropertyDescriptor(descArg);
677697
const requireCall =
678698
reexport && parseRequireCall(parser, reexport.expr);
679-
if (reexport && requireCall && requireCall.argument.isString()) {
699+
if (reexport && requireCall) {
680700
const dep = new CommonJsExportRequireDependency(
681701
/** @type {Range} */ (expr.range),
682702
/** @type {Range} */ (reexport.expr.range),
683703
`Object.defineProperty(${exportsArg.identifier})`,
684704
[property],
685-
/** @type {string} */ (requireCall.argument.string),
705+
requireCall.request,
686706
requireCall.ids,
687707
false
688708
);

‎lib/dependencies/commonjs/CommonJsImportsParserPlugin.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ const getCommentCompilationWarning = memoize(() =>
6565
* member-access references on `NAME` to the `CommonJsRequireDependency`
6666
* created for the `require()` call.
6767
* @typedef {object} RequireBindingData
68+
* @property {string} request request passed to `require()`
6869
* @property {RawReferencedExports | null} referencedExports mutable list, pushed to as `NAME.x.y` accesses are walked; null means the whole namespace is used. Separate from `dep` so a deferred require (walked after accesses on `NAME`) still collects them; becomes the dep's shared list on creation.
6970
* @property {InstanceType<typeof import("./CommonJsRequireDependency")> | null} dep dependency for the `require()` call, null until the `require()` is walked; kept so a later whole-namespace use can null its list directly
7071
*/
@@ -764,6 +765,7 @@ class CommonJsImportsParserPlugin {
764765
if (!requireInfo || requireInfo.name !== "require") return;
765766
/** @type {RequireBindingData} */
766767
const binding = {
768+
request: arg.value,
767769
referencedExports: [],
768770
dep: null
769771
};
@@ -883,5 +885,6 @@ CommonJsImportsParserPlugin.createRequireAsExpressionHandler =
883885
CommonJsImportsParserPlugin.createRequireCacheDependency =
884886
createRequireCacheDependency;
885887
CommonJsImportsParserPlugin.createRequireHandler = createRequireCallHandler;
888+
CommonJsImportsParserPlugin.REQUIRE_BINDING_TAG = REQUIRE_BINDING_TAG;
886889

887890
module.exports = CommonJsImportsParserPlugin;

‎lib/graph/ExportsInfo.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ const makeSerializable = require("../util/makeSerializable");
3232
/** @typedef {typeof UsageState.Unused | RuntimeUsageStateType} UsageStateType */
3333

3434
/** @typedef {Map<string, RuntimeUsageStateType>} UsedInRuntime */
35-
/** @typedef {{ module: Module, export: ExportInfoName[], deferred: boolean }} TargetItemWithoutConnection */
35+
/** @typedef {{ module: Module, connection: ModuleGraphConnection, export: ExportInfoName[], deferred: boolean }} FoundTargetItem */
3636
/** @typedef {{ module: Module, connection: ModuleGraphConnection, export: ExportInfoName[] | undefined }} TargetItemWithConnection */
3737
/** @typedef {(target: TargetItemWithConnection) => boolean} ResolveTargetFilter */
3838
/** @typedef {(module: Module) => boolean} ValidTargetModuleFilter */
@@ -1708,7 +1708,7 @@ class ExportInfo {
17081708
* when the chain ends without a valid module.
17091709
* @param {ModuleGraph} moduleGraph the module graph
17101710
* @param {ValidTargetModuleFilter} validTargetModuleFilter a valid target module
1711-
* @returns {TargetItemWithoutConnection | null | undefined | false} the target, undefined when there is no target, false when no target is valid
1711+
* @returns {FoundTargetItem | null | undefined | false} the target, undefined when there is no target, false when no target is valid
17121712
*/
17131713
findTarget(moduleGraph, validTargetModuleFilter) {
17141714
return this._findTarget(
@@ -1723,17 +1723,18 @@ class ExportInfo {
17231723
* @param {ModuleGraph} moduleGraph the module graph
17241724
* @param {ValidTargetModuleFilter} validTargetModuleFilter a valid target module
17251725
* @param {AlreadyVisitedExportInfo} alreadyVisited set of already visited export info to avoid circular references
1726-
* @returns {TargetItemWithoutConnection | null | undefined | false} the target, undefined when there is no target, false when no target is valid
1726+
* @returns {FoundTargetItem | null | undefined | false} the target, undefined when there is no target, false when no target is valid
17271727
*/
17281728
_findTarget(moduleGraph, validTargetModuleFilter, alreadyVisited) {
17291729
if (!this._target || this._target.size === 0) return;
17301730
const rawTarget =
17311731
/** @type {Target} */
17321732
(this._getMaxTarget()).values().next().value;
17331733
if (!rawTarget) return;
1734-
/** @type {TargetItemWithoutConnection} */
1734+
/** @type {FoundTargetItem} */
17351735
let target = {
17361736
module: rawTarget.connection.module,
1737+
connection: rawTarget.connection,
17371738
export: rawTarget.export,
17381739
deferred: Boolean(
17391740
rawTarget.connection.dependency &&
@@ -1759,6 +1760,7 @@ class ExportInfo {
17591760
} else {
17601761
target = {
17611762
module: newTarget.module,
1763+
connection: newTarget.connection,
17621764
export: newTarget.export
17631765
? [...newTarget.export, ...target.export.slice(1)]
17641766
: target.export.slice(1),

‎lib/optimize/ConcatenatedModule.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -822,9 +822,13 @@ const getFinalBinding = (
822822
` * ${info.type} ${m.readableIdentifier(requestShortener)}`
823823
).join("\n")}`
824824
);
825-
}
826-
if (reexport) {
825+
} else if (reexport) {
827826
const refInfo = moduleToInfoMap.get(reexport.module);
827+
// a CommonJS `exports.x = require()` re-export keeps raw exports semantics
828+
const moduleExportsAccess =
829+
reexport.connection.dependency instanceof
830+
CommonJsExportRequireDependency;
831+
828832
return getFinalBinding(
829833
moduleGraph,
830834
/** @type {ModuleInfo} */ (refInfo),
@@ -841,7 +845,7 @@ const getFinalBinding = (
841845
/** @type {BuildMeta} */
842846
(info.module.buildMeta).strictHarmonyModule,
843847
asiSafe,
844-
false,
848+
moduleExportsAccess,
845849
alreadyVisited
846850
);
847851
}

‎test/cases/cjs-tree-shaking/reexport-require-binding/index.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ns as whole } from "./reexport-whole";
22
import { ns as viaSpecifier } from "./reexport-specifier";
33
import { picked, flag } from "./partial";
4+
import { ns as viaExports, a, lazy } from "./reexport-cjs";
45

56
it("should keep the whole exports object when an `export const` require binding is re-exported (#21135)", () => {
67
expect(whole.a).toBe("a");
@@ -20,3 +21,11 @@ it("should still tree-shake when only specific members of a require binding are
2021
// `b` is never touched, so it is shaken out of the required module.
2122
expect(flag).toEqual(["a", "usedExports"]);
2223
});
24+
25+
it("should tree-shake a require binding assigned to CommonJS exports", () => {
26+
expect(viaExports.a).toBe("a");
27+
expect(a).toBe("a");
28+
expect(lazy.a).toBe("a");
29+
// The binding is re-exported, so only reads through the re-export count.
30+
expect(viaExports.usedExports).toEqual(["a", "usedExports"]);
31+
});

‎test/cases/cjs-tree-shaking/reexport-require-binding/reexport-cjs-shadowed.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
const ns = require("./module?shadowed");
2+
exports.ns = ns;
3+
Object.defineProperty(exports, "namedGetter", {
4+
get: function ns() {
5+
return ns;
6+
}
7+
});
8+
Object.defineProperty(exports, "paramGetter", {
9+
get(ns = "param") {
10+
return ns;
11+
}
12+
});

‎test/cases/cjs-tree-shaking/reexport-require-binding/reexport-cjs.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
const ns = require("./module?cjs");
2+
exports.ns = ns;
3+
exports.a = ns.a;
4+
Object.defineProperty(exports, "lazy", { get: () => ns });

‎test/configCases/cjs-tree-shaking/unused-reexport-require-binding-drop/effect.cjs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
"use strict";
2+
3+
global.__cjs_binding_effect_ran = true;
4+
exports.value = "EFFECT_VALUE_MARKER";
5+
exports.usedExports = __webpack_exports_info__.usedExports;

0 commit comments

Comments
 (0)