Skip to content

Commit 72ef0fb

Browse files
alexander-akaitalexander-akait
authored
fix: tree-shake CommonJS exports through const NAME = require(LITERAL) bindings (#21003)
1 parent 6c5f2f8 commit 72ef0fb

11 files changed

Lines changed: 242 additions & 1 deletion

File tree

‎.changeset/cjs-require-binding-tree-shake.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack": patch
3+
---
4+
5+
Tree-shake CommonJS modules imported through a `const NAME = require(LITERAL)` binding when only static members of `NAME` are read. Previously webpack treated every export of such modules as referenced (because the bare `require()` dependency reports `EXPORTS_OBJECT_REFERENCED`), so unused `exports.x = ...` assignments remained in the bundle even with `usedExports` enabled. The parser now forwards `NAME.x` / `NAME.x()` / `NAME["x"]` accesses to the underlying `CommonJsRequireDependency` as referenced exports, falling back to the full exports object the moment `NAME` is read in any other context (passed by value, destructured later, accessed with a dynamic key, …). This brings the binding form to parity with the existing destructuring form (`const { x } = require(...)`).

‎lib/dependencies/CommonJsImportsParserPlugin.js‎

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,22 @@ const RequireResolveHeaderDependency = require("./RequireResolveHeaderDependency
4848
* @property {string} context
4949
*/
5050

51+
/**
52+
* Per-`const NAME = require(LITERAL)` binding state used to forward
53+
* member-access references on `NAME` to the `CommonJsRequireDependency`
54+
* created for the `require()` call.
55+
* @typedef {object} RequireBindingData
56+
* @property {RawReferencedExports} referencedExports mutable list shared with the dependency; pushed to as `NAME.x.y` accesses are walked
57+
* @property {InstanceType<typeof import("./CommonJsRequireDependency")> | null} dep dependency for the `require()` call (assigned during walk)
58+
*/
59+
60+
/** @type {WeakMap<CallExpression, RequireBindingData>} */
61+
const requireBindingData = new WeakMap();
62+
63+
const REQUIRE_BINDING_TAG = Symbol(
64+
"CommonJsImportsParserPlugin require binding"
65+
);
66+
5167
const PLUGIN_NAME = "CommonJsImportsParserPlugin";
5268

5369
/**
@@ -152,17 +168,26 @@ const createRequireCallHandler = (parser, options, getContext) => {
152168
*/
153169
const processRequireItem = (expr, param) => {
154170
if (param.isString()) {
155-
const referencedExports = getRequireReferencedExportsFromDestructuring(
171+
let referencedExports = getRequireReferencedExportsFromDestructuring(
156172
parser,
157173
expr
158174
);
175+
const binding = requireBindingData.get(
176+
/** @type {CallExpression} */ (expr)
177+
);
178+
if (binding && !referencedExports) {
179+
// `const NAME = require(LITERAL)` — let later member-access walks
180+
// on `NAME` populate the dependency's referenced exports.
181+
referencedExports = binding.referencedExports;
182+
}
159183
const dep = new CommonJsRequireDependency(
160184
/** @type {string} */ (param.string),
161185
/** @type {Range} */ (param.range),
162186
getContext(),
163187
referencedExports,
164188
/** @type {Range} */ (expr.range)
165189
);
190+
if (binding) binding.dep = dep;
166191
dep.loc = /** @type {DependencyLocation} */ (expr.loc);
167192
dep.optional = Boolean(parser.scope.inTry);
168193
parser.state.current.addDependency(dep);
@@ -662,6 +687,88 @@ class CommonJsImportsParserPlugin {
662687
.tap(PLUGIN_NAME, callChainHandler);
663688
// #endregion
664689

690+
// #region Require bound to a const variable
691+
// Track `const NAME = require(LITERAL)` so that static member accesses on
692+
// `NAME` (e.g. `NAME.foo`, `NAME.foo()`) are forwarded to the same
693+
// `CommonJsRequireDependency` as referenced exports — enabling tree
694+
// shaking of CommonJS modules that are imported into a named binding
695+
// rather than destructured.
696+
parser.hooks.preDeclarator.tap(PLUGIN_NAME, (declarator, statement) => {
697+
if (statement.kind !== "const") return;
698+
if (declarator.id.type !== "Identifier") return;
699+
if (!declarator.init || declarator.init.type !== "CallExpression") {
700+
return;
701+
}
702+
const init = declarator.init;
703+
if (
704+
init.callee.type !== "Identifier" ||
705+
init.callee.name !== "require" ||
706+
init.arguments.length !== 1
707+
) {
708+
return;
709+
}
710+
const arg = init.arguments[0];
711+
if (arg.type !== "Literal" || typeof arg.value !== "string") return;
712+
// Only attach binding state when `require` resolves to the free
713+
// `require` (i.e. it isn't shadowed in the current scope).
714+
const requireInfo = parser.getFreeInfoFromVariable("require");
715+
if (!requireInfo || requireInfo.name !== "require") return;
716+
/** @type {RequireBindingData} */
717+
const binding = {
718+
referencedExports: [],
719+
dep: null
720+
};
721+
requireBindingData.set(init, binding);
722+
parser.tagVariable(declarator.id.name, REQUIRE_BINDING_TAG, binding);
723+
return true;
724+
});
725+
726+
parser.hooks.expression.for(REQUIRE_BINDING_TAG).tap(PLUGIN_NAME, () => {
727+
const binding =
728+
/** @type {RequireBindingData} */
729+
(parser.currentTagData);
730+
if (binding && binding.dep) {
731+
// `NAME` is read as a value (not as the object of a static member
732+
// chain), so we have to assume the whole exports object is used.
733+
binding.dep.referencedExports = null;
734+
}
735+
});
736+
737+
parser.hooks.expressionMemberChain
738+
.for(REQUIRE_BINDING_TAG)
739+
.tap(PLUGIN_NAME, (_expr, members) => {
740+
const binding =
741+
/** @type {RequireBindingData} */
742+
(parser.currentTagData);
743+
if (binding && binding.dep && binding.dep.referencedExports) {
744+
binding.dep.referencedExports.push(members);
745+
}
746+
// Returning truthy suppresses the parser's fallback chain (which
747+
// would otherwise walk `NAME` as a bare expression and trigger our
748+
// `expression` hook above, marking the whole namespace as used).
749+
return true;
750+
});
751+
752+
parser.hooks.callMemberChain
753+
.for(REQUIRE_BINDING_TAG)
754+
.tap(PLUGIN_NAME, (expr, members) => {
755+
const binding =
756+
/** @type {RequireBindingData} */
757+
(parser.currentTagData);
758+
if (binding && binding.dep && binding.dep.referencedExports) {
759+
if (members.length === 0) {
760+
// `NAME(...)` — calling the require result directly; the
761+
// whole exports object is observable.
762+
binding.dep.referencedExports = null;
763+
} else {
764+
binding.dep.referencedExports.push(members);
765+
}
766+
}
767+
if (expr.arguments) parser.walkExpressions(expr.arguments);
768+
return true;
769+
});
770+
// #endregion
771+
665772
// #region Require.resolve
666773
/**
667774
* Processes the provided expr.

‎test/cases/cjs-tree-shaking/require-member-access/index.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
it("should tree-shake CommonJS exports when accessed via a member call", () => {
2+
const m = require("./module");
3+
expect(m.a()).toBe("a");
4+
expect(m.usedExports).toEqual(["a", "usedExports"]);
5+
});
6+
7+
it("should tree-shake CommonJS exports when accessed as a property", () => {
8+
const m = require("./module");
9+
const a = m.a;
10+
const u = m.usedExports;
11+
expect(a()).toBe("a");
12+
expect(u).toEqual(["a", "usedExports"]);
13+
});
14+
15+
it("should tree-shake CommonJS exports when accessed with a string literal", () => {
16+
const m = require("./module");
17+
expect(m["a"]()).toBe("a");
18+
expect(m.usedExports).toEqual(["a", "usedExports"]);
19+
});
20+
21+
it("should bail out when the require result is read as a value (spread)", () => {
22+
const m = require("./module-rest");
23+
// Spreading the namespace forces every export to remain reachable.
24+
const all = { ...m };
25+
expect(all.a).toBe("a");
26+
expect(all.b).toBe("b");
27+
expect(all.usedExports).toBe(true);
28+
});
29+
30+
it("should bail out when the require result is accessed with a dynamic key", () => {
31+
const m = require("./module-rest");
32+
const key = "a";
33+
expect(m[key]).toBe("a");
34+
// Dynamic-key access cannot be statically tracked, so every export must
35+
// remain reachable.
36+
expect(m.usedExports).toBe(true);
37+
});
38+
39+
it("should bail out when the require result is called directly", () => {
40+
const m = require("./module-call");
41+
expect(m()).toBe("direct-call");
42+
// Calling the namespace itself means the whole exports object is observable;
43+
// every attached property must still be reachable.
44+
expect(m.x).toBe("x");
45+
// `module.exports = fn` already prevents named-export tracking, so usage
46+
// reports `null` ("no specific usage tracked").
47+
expect(m.usedExports).toBe(null);
48+
});

‎test/cases/cjs-tree-shaking/require-member-access/module-call.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
const fn = function () {
2+
return "direct-call";
3+
};
4+
fn.x = "x";
5+
fn.usedExports = __webpack_exports_info__.usedExports;
6+
module.exports = fn;

‎test/cases/cjs-tree-shaking/require-member-access/module-rest.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
exports.a = "a";
2+
exports.b = "b";
3+
exports.usedExports = __webpack_exports_info__.usedExports;

‎test/cases/cjs-tree-shaking/require-member-access/module.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
exports.a = function a() {
2+
return "a";
3+
};
4+
exports.b = "b";
5+
exports.usedExports = __webpack_exports_info__.usedExports;

‎test/cases/cjs-tree-shaking/require-member-access/test.filter.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
"use strict";
2+
3+
module.exports = function filter(config) {
4+
// usedExports analysis only runs outside development mode.
5+
return config.mode !== "development";
6+
};

‎test/configCases/issues/issue-14054/index.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
const utils = require("./utils");
2+
const allUsed = require("./utils-fullref");
3+
4+
it("should run the used CommonJS export", () => {
5+
expect(utils.foo()).toBe(["USED", "FOO", "MARKER"].join("_"));
6+
});
7+
8+
it("should keep all exports when the require result is read directly", () => {
9+
// Reading `allUsed` itself (not through a static member) must keep every
10+
// export reachable — Object.keys is a side-effecting use of the namespace.
11+
expect(Object.keys(allUsed).sort()).toEqual(["bar", "foo"]);
12+
});
13+
14+
it("should tree-shake unused CommonJS exports when only static members are accessed", () => {
15+
const fs = require("fs");
16+
const content = fs.readFileSync(__filename, "utf-8");
17+
// Build marker strings at runtime so the assertion source itself does not
18+
// embed the patterns we are about to look up.
19+
const usedFoo = ["USED", "FOO", "MARKER"].join("_");
20+
const unusedBar = ["UNUSED", "BAR", "MARKER"].join("_");
21+
const unusedBaz = ["UNUSED", "BAZ", "MARKER"].join("_");
22+
const fullrefFoo = ["FULLREF", "FOO", "MARKER"].join("_");
23+
const fullrefBar = ["FULLREF", "BAR", "MARKER"].join("_");
24+
expect(content.includes(usedFoo)).toBe(true);
25+
expect(content.includes(unusedBar)).toBe(false);
26+
expect(content.includes(unusedBaz)).toBe(false);
27+
// Counter-test: when the require result is read directly, exports are kept.
28+
expect(content.includes(fullrefFoo)).toBe(true);
29+
expect(content.includes(fullrefBar)).toBe(true);
30+
});

‎test/configCases/issues/issue-14054/utils-fullref.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
exports.foo = function foo() {
2+
return "FULLREF_FOO_MARKER";
3+
};
4+
5+
exports.bar = function bar() {
6+
return "FULLREF_BAR_MARKER";
7+
};

‎test/configCases/issues/issue-14054/utils.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
exports.foo = function foo() {
2+
return "USED_FOO_MARKER";
3+
};
4+
5+
exports.bar = function bar() {
6+
return "UNUSED_BAR_MARKER";
7+
};
8+
9+
exports.baz = function baz() {
10+
return "UNUSED_BAZ_MARKER";
11+
};

0 commit comments

Comments
 (0)