fix: match harmony accept dependencies by module reference#21303
Conversation
The HMR accept template related dependencies via chunkGraph.getModuleId.
An unresolved import (ignored/failed, or a deferred lazy-barrel re-export)
has no module, and a module not in any chunk has a null id, so comparing
ids could crash ("Invalid value used as weak map key") or miss real
matches. Compare modules by reference and skip dependencies without a
module instead.
🦋 Changeset detectedLatest commit: b001bae 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 (99286e2). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@99286e2
yarn add -D webpack@https://pkg.pr.new/webpack@99286e2
pnpm add -D webpack@https://pkg.pr.new/webpack@99286e2 |
Types CoverageCoverage after merging fix/harmony-accept-unresolved-module into main will be
Coverage Report |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21303 +/- ##
=======================================
Coverage 92.86% 92.86%
=======================================
Files 594 594
Lines 65214 65210 -4
Branches 18144 18143 -1
=======================================
- Hits 60562 60559 -3
+ Misses 4652 4651 -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:
|
Merging this PR will degrade performance by 35.17%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "asset-modules-bytes", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
245.9 KB | 878.5 KB | -72.01% |
| ❌ | Memory | benchmark "asset-modules-inline", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
383.8 KB | 1,199 KB | -67.99% |
| ❌ | Memory | benchmark "many-modules-esm", scenario '{"name":"mode-development","mode":"development"}' |
1.1 MB | 1.9 MB | -40.53% |
| ❌ | Memory | benchmark "wasm-modules-async", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
189.9 KB | 247.6 KB | -23.3% |
| ⚡ | Memory | benchmark "wasm-modules-sync", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
356.6 KB | 127.2 KB | ×2.8 |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/harmony-accept-unresolved-module (b001bae) with main (0abec4d)
Summary
Follow-up to #21302 (Fixes #21300). The HMR accept template related dependencies via
chunkGraph.getModuleId(moduleGraph.getModule(dep)). An unresolved import (ignored/failed, or a deferred lazy-barrel re-export) has no module, and a module that is in no chunk has anullid — so comparing ids can crash (Invalid value used as weak map key) or silently miss real matches. This compares modules by reference instead, which is correct in both cases and drops the redundantgetModuleIdlookup on a codegen hot path.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
yes —
test/hotCases/harmony/accept-import-without-module/reproduces the crash via an unresolved (IgnorePlugin) accept dependency in a non-executed entry; it fails against the original 5.108 codegen and passes with the guard.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) was used to locate the crashing comparison, implement the reference-based fix, and write the regression test; all changes were reviewed by me.
Generated by Claude Code