fix: tree-shake CommonJS exports through const NAME = require(LITERAL) bindings#21003
Conversation
…L)` bindings Closes #14054. Previously webpack treated every export of a CommonJS module as referenced when the module was imported via a named binding (`const utils = require("./utils")`) — the bare `CommonJsRequireDependency` reports `EXPORTS_OBJECT_REFERENCED` because it can't see how `utils` is later used. Only the destructuring form (`const { foo } = require("./utils")`) was actually tree-shakeable. CommonJsImportsParserPlugin now tags the declared binding and forwards later static member accesses on it — `utils.foo`, `utils.foo()`, `utils["foo"]` — to the same `CommonJsRequireDependency` as referenced exports. The moment `utils` is read in any other context (passed by value, spread, destructured later, accessed with a dynamic key, called directly), the dependency is restored to `EXPORTS_OBJECT_REFERENCED` so no usage is silently dropped.
🦋 Changeset detectedLatest commit: 82849d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This PR is packaged and the instant preview is available (72ef0fb). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@72ef0fb
yarn add -D webpack@https://pkg.pr.new/webpack@72ef0fb
pnpm add -D webpack@https://pkg.pr.new/webpack@72ef0fb |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21003 +/- ##
==========================================
+ Coverage 90.93% 91.60% +0.67%
==========================================
Files 573 573
Lines 58986 59277 +291
Branches 15898 16012 +114
==========================================
+ Hits 53636 54301 +665
+ Misses 5350 4976 -374
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves CommonJS tree-shaking when a module is imported via a named const binding (const ns = require("./mod")). It does so by tracking that binding in the parser and forwarding subsequent static member accesses on the binding to the same CommonJsRequireDependency, enabling usedExports to drop unused exports.* assignments while still bailing out to full-namespace usage when the binding is used in non-static ways.
Changes:
- Track
const NAME = require("literal")bindings and record later static member-chain reads/calls (NAME.x,NAME["x"],NAME.x()) as referenced exports on the underlyingCommonJsRequireDependency. - Add regression tests for issue #14054 and a dedicated
cjs-tree-shaking/require-member-accesscase to validate partial namespace usage and bailout behavior. - Add a changeset documenting the patch-level behavior change.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/dependencies/CommonJsImportsParserPlugin.js |
Tags const require bindings and forwards static member-chain usage into CommonJsRequireDependency.referencedExports, with bailout to full namespace on value-usage. |
test/configCases/issues/issue-14054/webpack.config.js |
Production config for the regression case validating minified output markers. |
test/configCases/issues/issue-14054/index.js |
Regression assertions that unused CommonJS exports are removed when only static members are accessed. |
test/configCases/issues/issue-14054/utils.js |
Fixture module with used/unused exported markers. |
test/configCases/issues/issue-14054/utils-fullref.js |
Fixture module for the “full namespace read” counter-test (ensures exports are retained). |
test/cases/cjs-tree-shaking/require-member-access/test.filter.js |
Skips this case in development mode where usedExports analysis is not active. |
test/cases/cjs-tree-shaking/require-member-access/index.js |
Verifies used-exports tracking through a require binding and a bailout scenario via value usage. |
test/cases/cjs-tree-shaking/require-member-access/module.js |
Exposes __webpack_exports_info__.usedExports for assertions. |
test/cases/cjs-tree-shaking/require-member-access/module-rest.js |
Same fixture used for the bailout (namespace read) test. |
.changeset/cjs-require-binding-tree-shake.md |
Patch changeset describing the new tree-shaking behavior for const NAME = require(...) bindings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it("should tree-shake CommonJS exports through a const binding (call form)", () => { | ||
| const m = require("./module"); | ||
| expect(m.a).toBe("a"); | ||
| expect(m.usedExports).toEqual(["a", "usedExports"]); | ||
| }); |
| if (members.length === 0) { | ||
| // `NAME(...)` — calling the require result directly; the | ||
| // whole exports object is observable. | ||
| binding.dep.referencedExports = null; | ||
| } else { |
Addresses Copilot review comments on #21003: 1. The first member-access test claimed "(call form)" but only performed a property read. Make the fixture export a function so the test actually calls `m.a()`, and add matching call-style assertions to the property and string-literal tests so all three trigger the same referenced-exports set. 2. Add explicit bailout tests for non-static uses of the binding — spread (already covered), dynamic-key access `m[key]`, and direct call `m(...)` against a `module.exports = fn` style module — so the `EXPORTS_OBJECT_REFERENCED` fallback path is exercised.
Merging this PR will not alter performance
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
| * @typedef {object} RequireBindingData | ||
| * @property {RawReferencedExports} referencedExports mutable list shared with the dependency; pushed to as `NAME.x.y` accesses are walked | ||
| * @property {InstanceType<typeof import("./CommonJsRequireDependency")> | null} dep dependency for the `require()` call (assigned during walk) | ||
| */ |
Closes #14054.
Previously webpack treated every export of a CommonJS module as referenced
when the module was imported via a named binding (
const utils = require("./utils")) — the bareCommonJsRequireDependencyreportsEXPORTS_OBJECT_REFERENCEDbecause it can't see howutilsis laterused. Only the destructuring form (
const { foo } = require("./utils"))was actually tree-shakeable.
CommonJsImportsParserPlugin now tags the declared binding and forwards
later static member accesses on it —
utils.foo,utils.foo(),utils["foo"]— to the sameCommonJsRequireDependencyas referencedexports. The moment
utilsis read in any other context (passed byvalue, spread, destructured later, accessed with a dynamic key, called
directly), the dependency is restored to
EXPORTS_OBJECT_REFERENCEDsono usage is silently dropped.