fix: don't cache incomplete exports of lazy-barrel-deferred re-exports#21322
fix: don't cache incomplete exports of lazy-barrel-deferred re-exports#21322alexander-akait wants to merge 1 commit into
Conversation
A side-effect-free barrel whose re-export target is deferred by the lazy barrel reports no exports for that dependency, so the module was flagged cacheable and persisted with a closed export set omitting those names. On a later build that imports one of them, the stale provided exports were restored from the persistent cache and the name was reported as missing. Treat a lazy-deferred dependency as non-cacheable, matching a resolved re-export. Closes #21318
🦋 Changeset detectedLatest commit: 5c72600 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 (5c72600). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@5c72600
yarn add -D webpack@https://pkg.pr.new/webpack@5c72600
pnpm add -D webpack@https://pkg.pr.new/webpack@5c72600 |
Types CoverageCoverage after merging claude/webpack-issue-21318-tr4k63 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 #21322 +/- ##
==========================================
- Coverage 92.45% 92.44% -0.01%
==========================================
Files 590 594 +4
Lines 65123 65213 +90
Branches 18135 18144 +9
==========================================
+ Hits 60209 60288 +79
- Misses 4914 4925 +11
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 24.51%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "wasm-modules-async", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
245.6 KB | 1,035.5 KB | -76.29% |
| ❌ | Memory | benchmark "asset-modules-inline", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
384.4 KB | 1,194.7 KB | -67.83% |
| ❌ | Memory | benchmark "asset-modules-bytes", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
325.5 KB | 858.9 KB | -62.1% |
| ⚡ | Memory | benchmark "side-effects-reexport", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
859.3 KB | 132.8 KB | ×6.5 |
| ⚡ | Memory | benchmark "many-modules-esm", scenario '{"name":"mode-production","mode":"production"}' |
9.7 MB | 7.4 MB | +31.02% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/webpack-issue-21318-tr4k63 (5c72600) with main (e848598)
There was a problem hiding this comment.
Pull request overview
Fixes a persistent-filesystem-cache correctness issue where lazy-barrel deferred export * re-exports could be stored with an incomplete “provided exports” set, causing later compilations (e.g. when additional entries activate) to incorrectly report missing exports after restoring from cache.
Changes:
- Mark modules as non-cacheable for provided-exports caching when encountering a lazy-barrel deferred dependency in
FlagDependencyExportsPlugin. - Add a regression test that performs two filesystem-cache runs where a star re-exported name is imported only on the second run.
- Add a changeset describing the patch-level fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lib/FlagDependencyExportsPlugin.js |
Disables caching of provided-exports data when a dependency is currently deferred by the lazy-barrel optimization. |
test/PersistentCaching.test.js |
Adds an integration-style regression test covering restore-from-cache behavior across two builds. |
.changeset/lazy-barrel-persistent-cache-exports.md |
Records the user-facing patch note for the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // lazy barrel deferred this re-export: its target is not built, so | ||
| // the provided exports are incomplete and must not be cached | ||
| if ("isLazy" in dep && dep.isLazy()) { |
| // A side-effect-free barrel with a local export and a star re-export whose | ||
| // target is unused: the star stays deferred, so the barrel is cacheable and | ||
| // gets persisted as a closed export set omitting the star's exports. A later | ||
| // build that imports one of those names must recompute, not restore the stale | ||
| // exports and report the name as missing. |
|
Closing in favour of #21326, which fixes the same root cause ( #21326 keeps the cache working by folding the deferred-target set into the cache key, whereas this PR simply marks such barrels non-cacheable — the smaller change, but it drops the cache for those modules. Since #21326 is from the lazy-barrel author and preserves the cache, it's the better fix. Leaving the minimal "treat a lazy-deferred dependency as non-cacheable" approach here as a reference in case a smaller patch is ever preferred. Generated by Claude Code |
Summary
Fixes #21318. A side-effect-free barrel whose re-export target is deferred by the lazy barrel (e.g.
mobx-react'sexport * from "mobx-react-lite") reports no exports for that dependency, soFlagDependencyExportsPluginflagged the module cacheable and persisted a closed export set omitting those names. A later build that imports one of them (typical withlazyCompilationwhere an entry activates afterwards) restored the stale provided exports from the filesystem cache instead of recomputing, and reported the name as missing (export 'X' ... was not found). A resolved re-export already makes the module non-cacheable; this treats a lazy-deferred dependency the same way.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes —
test/PersistentCaching.test.js(two filesystem-cache runs where the deferred star re-export's name is imported only on the second run). It fails on the old code and passes with the fix.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 was used to help investigate the regression and draft the fix and test; the change and test were reviewed and verified locally.
Generated by Claude Code