Skip to content

fix(code-splitting): align dynamicImports metadata with the rewritten import() specifier#10430

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/dynamic-imports-metadata-entry-chunk
Jul 24, 2026
Merged

fix(code-splitting): align dynamicImports metadata with the rewritten import() specifier#10430
graphite-app[bot] merged 1 commit into
mainfrom
fix/dynamic-imports-metadata-entry-chunk

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 24, 2026

Copy link
Copy Markdown
Member

The emitted code and the chunk metadata could disagree about which file a dynamic import loads. The code was always right; chunk.dynamicImports was wrong. This PR makes the metadata name the same file the emitted import() names.

// a.js — emitted code (correct)
import("./chunks/target.js")

// a.js — metadata before this PR (wrong)
dynamicImports: ["chunks/dyn.js"]

Problem

A dynamically imported module becomes a dynamic entry. Normally its body lives in its own entry chunk, so the file you import is the file that holds the code — no ambiguity. The two diverge when only a facade file remains at the entry (importing the body chunk and triggering init) while the body is hosted elsewhere:

  • order-wrap entry facades under strictExecutionOrder;
  • with strict order off: facade elimination skipped (top-level await anywhere in the graph, or chunkOptimization.mergeCommonChunks: false) while a manual group — or a chunk shared by static+dynamic importers — hosts the body.

The root cause is two different lookup tables: the finalizer rewrites the import() specifier through entry_module_to_entry_chunk (→ the facade), while collect_depended_symbols recorded the metadata edge from module_to_chunk[importee] (→ the body chunk).

Runtime behavior is unaffected; only metadata consumers get the wrong edge: build.manifest (backend preload tags point at the body chunk and miss the facade — an extra request waterfall), the devtools chunk graph, and the bundle analyzer. Vite's __vitePreload and SSR manifest re-parse the emitted code, so runtime preloading was never affected.

Fix

Record the edge through the same mapping the finalizer uses: entry_module_to_entry_chunk first, module_to_chunk as fallback. When the mapping is present, every rewrite path derives the emitted path from it; when it is absent the finalizer emits void 0, and the fallback keeps today's metadata byte-for-byte. cross_chunk_dynamic_imports feeds only metadata surfaces (RollupRenderedChunk, OutputChunk.dynamicImports, devtools trace), so emitted code cannot change.

Two pre-existing wrinkles are deliberately left alone (a mapping-absent void 0 import and a same-chunk merged entry can still record an edge with no emitted request): both record identically before and after this change; tracked in #10294.

Validation

  • Two vitest fixtures (output/dynamic-imports-metadata/{strict-facade,kept-facade-tla}) pin both shapes from the public JS API surface: red on an unfixed build (['chunks/dyn.js'] / ['chunks/grp.js'] recorded while the code names the facade), green with the fix. The dual static+dynamic combo was verified with a one-off probe on both builds.
  • Full Rust fixture sweep (--include-ignored): failure list identical to unmodified main on the same machine (the pre-existing local environment set), zero snapshot changes. clippy and fmt clean.

Part of #10294.

@netlify

netlify Bot commented Jul 24, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit 7e13de4
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a63909037f78c000867853b
😎 Deploy Preview https://deploy-preview-10430--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@hyfdev
hyfdev marked this pull request as ready for review July 24, 2026 04:53
Copilot AI review requested due to automatic review settings July 24, 2026 04:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

hyfdev commented Jul 24, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jul 24, 4:53 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 24, 4:17 PM UTC: hyfdev added this pull request to the Graphite merge queue.
  • Jul 24, 4:39 PM UTC: Merged by the Graphite merge queue.

@codspeed-hq

codspeed-hq Bot commented Jul 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/dynamic-imports-metadata-entry-chunk (f94e537) with main (cd742e7)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

… import() specifier (#10430)

The emitted code and the chunk metadata could disagree about which file a dynamic import loads. The code was always right; `chunk.dynamicImports` was wrong. This PR makes the metadata name the same file the emitted `import()` names.

```js
// a.js — emitted code (correct)
import("./chunks/target.js")

// a.js — metadata before this PR (wrong)
dynamicImports: ["chunks/dyn.js"]
```

### Problem

A dynamically imported module becomes a dynamic entry. Normally its body lives in its own entry chunk, so the file you import is the file that holds the code — no ambiguity. The two diverge when only a facade file remains at the entry (importing the body chunk and triggering init) while the body is hosted elsewhere:

- order-wrap entry facades under `strictExecutionOrder`;
- with strict order off: facade elimination skipped (top-level await anywhere in the graph, or `chunkOptimization.mergeCommonChunks: false`) while a manual group — or a chunk shared by static+dynamic importers — hosts the body.

The root cause is two different lookup tables: the finalizer rewrites the `import()` specifier through `entry_module_to_entry_chunk` (→ the facade), while `collect_depended_symbols` recorded the metadata edge from `module_to_chunk[importee]` (→ the body chunk).

Runtime behavior is unaffected; only metadata consumers get the wrong edge: `build.manifest` (backend preload tags point at the body chunk and miss the facade — an extra request waterfall), the devtools chunk graph, and the bundle analyzer. Vite's `__vitePreload` and SSR manifest re-parse the emitted code, so runtime preloading was never affected.

### Fix

Record the edge through the same mapping the finalizer uses: `entry_module_to_entry_chunk` first, `module_to_chunk` as fallback. When the mapping is present, every rewrite path derives the emitted path from it; when it is absent the finalizer emits `void 0`, and the fallback keeps today's metadata byte-for-byte. `cross_chunk_dynamic_imports` feeds only metadata surfaces (RollupRenderedChunk, `OutputChunk.dynamicImports`, devtools trace), so emitted code cannot change.

Two pre-existing wrinkles are deliberately left alone (a mapping-absent `void 0` import and a same-chunk merged entry can still record an edge with no emitted request): both record identically before and after this change; tracked in #10294.

### Validation

- Two vitest fixtures (`output/dynamic-imports-metadata/{strict-facade,kept-facade-tla}`) pin both shapes from the public JS API surface: red on an unfixed build (`['chunks/dyn.js']` / `['chunks/grp.js']` recorded while the code names the facade), green with the fix. The dual static+dynamic combo was verified with a one-off probe on both builds.
- Full Rust fixture sweep (`--include-ignored`): failure list identical to unmodified main on the same machine (the pre-existing local environment set), zero snapshot changes. clippy and fmt clean.

Part of #10294.
@graphite-app
graphite-app Bot force-pushed the fix/dynamic-imports-metadata-entry-chunk branch from f94e537 to 7e13de4 Compare July 24, 2026 16:19
@graphite-app
graphite-app Bot merged commit 7e13de4 into main Jul 24, 2026
35 checks passed
@graphite-app
graphite-app Bot deleted the fix/dynamic-imports-metadata-entry-chunk branch July 24, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants