fix: keep all CommonJS exports when an exported function accesses them via this#21179
Conversation
🦋 Changeset detectedLatest commit: a572761 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 (c13cf1e). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@c13cf1e
yarn add -D webpack@https://pkg.pr.new/webpack@c13cf1e
pnpm add -D webpack@https://pkg.pr.new/webpack@c13cf1e |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21179 +/- ##
==========================================
- Coverage 92.68% 92.67% -0.01%
==========================================
Files 587 587
Lines 63962 64015 +53
Branches 17726 17746 +20
==========================================
+ Hits 59280 59325 +45
- Misses 4682 4690 +8
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 degrade performance by 38.09%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "asset-modules-inline", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
317.3 KB | 1,298.7 KB | -75.57% |
| ❌ | Memory | benchmark "side-effects-reexport", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
761.8 KB | 1,191.8 KB | -36.08% |
| ❌ | Memory | benchmark "asset-modules-bytes", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
247.2 KB | 320.7 KB | -22.92% |
| ⚡ | Memory | benchmark "many-modules-commonjs", scenario '{"name":"mode-production","mode":"production"}' |
8.7 MB | 7.1 MB | +22.02% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/cjs-this-in-exported-function (a572761) with main (0bf661b)
f835398 to
98273e5
Compare
|
Note on the CodSpeed memory report: the flagged fixtures never execute the changed code path. The scan only runs on Generated by Claude Code |
98273e5 to
edbe1ef
Compare
…m via this Tree-shaking individual CommonJS exports (#21003) dropped exports that are only reached through this from a sibling exported function, e.g. exports.a = function () { return this.b(); }. When such a function is called as a method of the exports object, this is the exports object, so no export may be removed or mangled. Function expressions assigned to exports (or used as value/get/set in an Object.defineProperty(exports, ...) descriptor) are now scanned for a this belonging to the function's own this-scope (descending into arrow functions, skipping nested functions and class element bodies); when found, the whole exports object is kept used via CommonJsSelfReferenceDependency. Fixes #21178
edbe1ef to
a572761
Compare
Summary
Fixes #21178. Since #21003 (5.107.1), CommonJS tree-shaking dropped exports that are only reached via
thisfrom a sibling exported function (e.g.exports.a = function () { return this.b(); }inali-oss), breaking them at runtime; exported function expressions (includingObject.defineProperty(exports, ...)descriptor functions) are now scanned for own-thisusage and the whole exports object is kept used (and unmangled) when found, while exported classes andthisin nested functions still tree-shake.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes,
test/cases/cjs-tree-shaking/this-in-exported-function/with 10 scenarios (sibling call viathis,module.exports/top-levelthisbases, arrow capture, defineProperty value/getter/setter, generator/async/default-param/computed access, ESM consumer, plus controls proving classes, nested-thisfunctions andthis-free modules still tree-shake).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
This PR was developed with Claude Code under human direction: the AI reproduced the issue, implemented the fix and tests, and ran the test suites; the approach and trade-offs were reviewed and chosen by the requester.
Generated by Claude Code