fix: skip esm interop for require() module outside the concatenation - #21884
Conversation
🦋 Changeset detectedLatest commit: 8bb397e 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughChangesThe concatenation export-resolution path now skips ESM interop for Require interop correction
Merge Risk: ⚪ Minimal · up to This localized change adjusts generated require() behavior for modules outside concatenation and adds regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The implementation bypasses ESM interop for requires targeting modules outside concatenation. The added tests verify raw CommonJS exports, asset URL behavior, and split-module placement. Verification of the excluded SVG fixture is incomplete because
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 |
3b0a719 to
8bb397e
Compare
|
This PR is packaged and the instant preview is available (5352fd8). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@5352fd8
yarn add -D webpack@https://pkg.pr.new/webpack@5352fd8
pnpm add -D webpack@https://pkg.pr.new/webpack@5352fd8 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21884 +/- ##
=======================================
Coverage 95.08% 95.09%
=======================================
Files 702 702
Lines 90697 90750 +53
Branches 27368 27395 +27
=======================================
+ Hits 86242 86299 +57
+ Misses 4455 4451 -4
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:
|
Generated code sizeComparing
4 asset(s) changed size
3 asset(s) this pull request adds
4 runtime(s) changed which runtime modules they carry
1 runtime(s) this pull request adds or no longer builds
Built |
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "many-modules-esm", scenario '{"name":"mode-production","mode":"production"}' |
9.2 MB | 11.5 MB | -20.39% |
| ⚡ | Memory | benchmark "asset-modules-source", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
1,368.4 KB | 656 KB | ×2.1 |
| ⚡ | Memory | benchmark "wasm-modules-sync", scenario '{"name":"mode-production","mode":"production"}' |
9.3 MB | 7.3 MB | +26.1% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/concat-external-require-interop (8bb397e) with main (c961f61)
Summary
Fixes #21882.
Keep raw access for
require()targets outside the concatenation. And no fake namespace is needed to keep ESM interop.What kind of change does this PR introduce?
Fix
Did you add tests for your changes?
Yes
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
Yes
Summary by CodeRabbit
Bug Fixes
require()behavior for modules outside concatenated bundles.Tests