Repository navigation
fix(cjs): recompute a re-export whose required module was missing - #22631
Conversation
`exports.x = require("./m").y` and `module.exports = require("./m")` gave no exports while `./m` was missing, which flagged the module cacheable. Once `./m` came back the module had not changed, so its exports were restored without the re-export and an `export *` of it lost the name. Report them uncacheable, as webpack#22432 did for ESM.
|
|
Reported by Types Coverage #1265 for 29429c1, computed by this pull request's own code. Types CoverageCoverage after merging fix/cjs-reexport-restored-target into main will be
Coverage Report |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Change: Bug fix Merge Risk: ⚪ Minimal · up to The change allows missing CommonJS re-exports to recover when dependencies return, and the added watch test checks the corresponding fixture states. No actionable merge risk remains. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title uses the required Conventional Commit form, fix(cjs): subject, and accurately describes the CommonJS re-export fix. The fix type is allowed; branch-prefix alignment cannot be independent… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22631 +/- ##
=======================================
Coverage 96.26% 96.26%
=======================================
Files 774 774
Lines 129796 129796
Branches 43189 43189
=======================================
+ Hits 124947 124948 +1
+ Misses 4849 4848 -1
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:
|
|
Reported by Code Size #4503 for 29429c1, computed by this pull request's own code. Generated code sizeComparing base (
Read No asset changed size. No runtime gained or lost a runtime module. Built |
|
This PR is packaged and the instant preview is available (b19519a). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@b19519a
yarn add -D webpack@https://pkg.pr.new/webpack@b19519a
pnpm add -D webpack@https://pkg.pr.new/webpack@b19519a |
Summary
Found by the rebuild fuzzer. While
./depwas missing,exports.r = require("./dep").aandmodule.exports = require("./dep")reported no exports, which left the module's provided exports cacheable. When./depcame back the module itself had not changed, so watch restored its exports without the re-export:In production
module.exports = require("./dep")lost every export (/* unused reexport */). A missing target now reports an uncacheable empty export set, as #22432 does for ESM. A sweep of 5 CommonJS shapes × ESM/CommonJS targets × 3 importers × 3 delete/restore orders × 2 modes fails 153 of 180 on main and 0 with this change.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes:
watchCases/recover-from-error/restored-commonjs-reexport, memory and filesystem cache; reverting either changed line fails it.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
I used Claude Code to find the bug with a rebuild fuzzer, write the fix and the tests. I reviewed every change myself.
Summary by CodeRabbit