docs(buildChunkGraph): explain why blocksWithNestedBlocks gates the skip#21025
Conversation
The previous `// TODO is this needed?` left the rationale ambiguous. Skipping a block disconnects it from its chunk group, which orphans any nested block's chunk group from this block's chunk group parent, so the check must stay.
Pins down the case where buildChunkGraph's connectChunkGroups would otherwise skip a block whose modules are already in the entry chunk. Without the `!blocksWithNestedBlocks.has(block)` guard, the outer require.ensure's chunk group is left without a parent and gets cleaned up together with the nested chunk group, so the inner sync require fails at runtime. With the guard the test passes; removing the guard makes the test time out waiting for the inner callback.
|
|
This PR is packaged and the instant preview is available (12cb825). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@12cb825
yarn add -D webpack@https://pkg.pr.new/webpack@12cb825
pnpm add -D webpack@https://pkg.pr.new/webpack@12cb825 |
There was a problem hiding this comment.
Pull request overview
This PR clarifies an important invariant in buildChunkGraph and adds a regression test to ensure nested async blocks are not incorrectly skipped when their direct dependencies are already available in the parent/entry chunk.
Changes:
- Replace an ambiguous TODO in
connectChunkGroupswith a rationale explaining whyblocksWithNestedBlocksmust prevent the “all dependencies already available” skip. - Add a new chunking test case that reproduces the orphaned-parent scenario and guards against regressions.
- Add minimal fixture modules (
a.js,b.js) used by the new test case.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/buildChunkGraph.js | Documents why blocks with nested blocks must remain connected to preserve chunk group parentage. |
| test/cases/chunks/nested-blocks-with-available-parent-modules/index.js | Adds a regression test ensuring nested async chunks remain reachable even when outer deps are already in the entry chunk. |
| test/cases/chunks/nested-blocks-with-available-parent-modules/a.js | Fixture module used to ensure ./a is already in the entry chunk. |
| test/cases/chunks/nested-blocks-with-available-parent-modules/b.js | Fixture module loaded from the nested async block to verify reachability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21025 +/- ##
==========================================
- Coverage 91.59% 91.58% -0.01%
==========================================
Files 573 573
Lines 59541 59541
Branches 16077 16077
==========================================
- Hits 54534 54530 -4
- Misses 5007 5011 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Merging this PR will degrade performance by 37.39%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
The previous
// TODO is this needed?left the rationale ambiguous.Skipping a block disconnects it from its chunk group, which orphans any
nested block's chunk group from this block's chunk group parent, so the
check must stay.