fix: keep full exports when a require() binding is re-exported#21144
Conversation
A `const NAME = require(LITERAL)` binding only tracked member accesses to
compute its referenced exports. When NAME was re-exported instead of used
locally, neither the value-read nor member-access hook fired, so the
referenced-exports list stayed empty and the required module was shaken to
`module.exports = {}`. Mark the whole exports object as observable whenever
the binding is re-exported.
Closes #21135
Add the exact reported shapes — `export const data = require(json)` and
`const data = require(json); export { data }` — plus the `import` variant the
reporter noted still works, asserting the whole JSON survives across modes.
🦋 Changeset detectedLatest commit: bbfeddc 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21144 +/- ##
==========================================
- Coverage 92.34% 92.33% -0.01%
==========================================
Files 581 581
Lines 63258 63271 +13
Branches 17494 17500 +6
==========================================
+ Hits 58416 58422 +6
- Misses 4842 4849 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "many-modules-commonjs", scenario '{"name":"mode-production","mode":"production"}' |
7.1 MB | 9.1 MB | -22.18% |
| ⚡ | Memory | benchmark "side-effects-reexport", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
1,185.9 KB | 759.9 KB | +56.06% |
| ⚡ | Memory | benchmark "many-modules-esm", scenario '{"name":"mode-development","mode":"development"}' |
1.7 MB | 1.3 MB | +29.43% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/reexport-require-binding (bbfeddc) with main (f9bb186)1
Footnotes
|
This PR is packaged and the instant preview is available (6475c4f). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@6475c4f
yarn add -D webpack@https://pkg.pr.new/webpack@6475c4f
pnpm add -D webpack@https://pkg.pr.new/webpack@6475c4f |
Summary
Fixes #21135. A
const NAME = require(LITERAL)binding only tracked member accesses (NAME.x) and value reads ofNAMEto compute its referenced exports. WhenNAMEwas re-exported (export const NAME = require(...)orexport { NAME }) instead of used locally, neither hook fired, so the referenced-exports list stayed empty and the required module was tree-shaken tomodule.exports = {}in production. The fix marks the whole exports object as observable whenever such a binding is re-exported.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes —
test/cases/cjs-tree-shaking/reexport-require-binding/(CJS module, withusedExportsassertions and a tree-shaking-still-narrows guard) andtest/cases/cjs-tree-shaking/reexport-require-json/(the reported JSON case plus theimportvariant that already worked).Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
AI (Claude Code) was used to locate the regression in
CommonJsImportsParserPlugin, implement the fix, and write the test cases; all changes were reviewed and verified against the issue's reproduction.Generated by Claude Code