Skip to content

feat(chunk-optimization): dedupe already-loaded dynamic deps#9305

Merged
graphite-app[bot] merged 1 commit into
mainfrom
chunk-already-loaded2
May 11, 2026
Merged

feat(chunk-optimization): dedupe already-loaded dynamic deps#9305
graphite-app[bot] merged 1 commit into
mainfrom
chunk-already-loaded2

Conversation

@IWANABETHATGUY

@IWANABETHATGUY IWANABETHATGUY commented May 7, 2026

Copy link
Copy Markdown
Member

Adds a pre-materialization chunk optimization pass that removes dynamic-entry dependency bits for modules already guaranteed to be loaded by every importer of that dynamic entry. This lets shared modules stay grouped with the static entry path instead of being split into extra chunks or pulled into dynamic chunks unnecessarily.

Already-Loaded Algorithm

Consider this module graph:

flowchart LR
  main["main entry"]
  shared["shared.js"]
  route["route.js dynamic entry"]

  main -->|"static import"| shared
  main -.->|"dynamic import()"| route
  route -->|"static import"| shared
Loading

When route.js runs, main has already loaded shared.js. So shared.js does not need to keep the route entry bit.

Before reduction:

shared.js dependent entries = { main, route }
route.js  dependent entries = { route }

After reduction:

shared.js dependent entries = { main }
route.js  dependent entries = { route }

That lets normal chunk grouping put shared.js with main, while route.js imports the binding from the already-loaded chunk instead of forcing an extra shared chunk.

The pass works on temporary atoms, where each atom is a group of modules with the same dependent-entry bitset:

atom(shared.js) = {
  modules: [shared.js],
  dependentEntries: { main, route }
}

Then it computes the atoms loaded before each dynamic entry executes:

staticLoaded[entry] = atoms loaded by the entry's static dependency graph
alreadyLoaded[dynamicEntry] = intersection of loaded atoms from every importer entry

The key detail is the intersection. If route.js can be dynamically imported by both main1 and main2, an atom can only drop the route bit if it is already loaded by both importers.

Pseudo-code:

atoms = groupModulesByDependentEntryBits()
staticLoaded = computeStaticLoadedAtomsForEachEntry()

for each included dynamic import:
  dynamicImporters[dynamicEntry].add(importerEntry)

queue = all dynamic entries

while queue is not empty:
  dynamicEntry = queue.pop()

  nextAlreadyLoaded = intersection over importerEntry in dynamicImporters[dynamicEntry] of:
    staticLoaded[importerEntry] union alreadyLoaded[importerEntry]

  if nextAlreadyLoaded changed:
    alreadyLoaded[dynamicEntry] = nextAlreadyLoaded

    for childDynamicEntry imported by dynamicEntry:
      dynamicImporters[childDynamicEntry].add(dynamicEntry)
      queue.push(childDynamicEntry)

for each atom:
  for each dynamicEntry bit in atom.dependentEntries:
    if alreadyLoaded[dynamicEntry] contains atom:
      remove dynamicEntry bit from atom

The reduced bits are only accepted when chunk safety is preserved. The pass still bails out for TLA, keeps strict entry signatures intact, handles runtime-only atoms and dynamic-entry modules specially, and checks static import cycles in risky cases such as manual chunks or strict signatures.

Summary

  • Add dynamic already-loaded analysis for experimental.chunkOptimization.
  • Propagate already-loaded atoms across included dynamic imports with a fixed-point worklist.
  • Preserve safety checks for TLA, strict entry signatures, manual chunking, and static import cycles.
  • Add BitSet::all and BitSet::intersect helpers.
  • Add basic and multi-entry dynamic already-loaded fixtures.
  • Update affected snapshots and runtime/static-cycle assertions.
  • Document the new analysis in meta/design/code-splitting.md.

Tests

  • Added fixture coverage under function/chunk_optimization/dynamic_already_loaded_*.
  • Updated regression coverage for chunk merging/static import cycle cases affected by the new grouping.

@netlify

netlify Bot commented May 7, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 7e5855c
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a0163d3b7e17c0008c37020

@IWANABETHATGUY IWANABETHATGUY self-assigned this May 9, 2026
@IWANABETHATGUY IWANABETHATGUY force-pushed the chunk-already-loaded2 branch 2 times, most recently from 4c91ebb to 90ec99c Compare May 9, 2026 12:37

IWANABETHATGUY commented May 9, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq

codspeed-hq Bot commented May 9, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing chunk-already-loaded2 (401218c) with main (9c3c339)2

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.

  2. No successful run was found on main (70c4828) during the generation of this report, so 9c3c339 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@IWANABETHATGUY IWANABETHATGUY force-pushed the chunk-already-loaded2 branch 3 times, most recently from e06b0da to 97e9097 Compare May 9, 2026 13:43
@IWANABETHATGUY IWANABETHATGUY changed the base branch from main to graphite-base/9305 May 9, 2026 13:47
@IWANABETHATGUY IWANABETHATGUY force-pushed the chunk-already-loaded2 branch from 0aa8ccd to d0668d7 Compare May 9, 2026 13:47
@IWANABETHATGUY IWANABETHATGUY changed the base branch from graphite-base/9305 to refactor/extract-static-cycle-helper May 9, 2026 13:47
@graphite-app graphite-app Bot changed the base branch from refactor/extract-static-cycle-helper to graphite-base/9305 May 9, 2026 13:56
@IWANABETHATGUY IWANABETHATGUY marked this pull request as draft May 9, 2026 14:04
@IWANABETHATGUY IWANABETHATGUY changed the title Chunk already loaded2 feat(chunk-optimization): dedupe already-loaded dynamic deps May 9, 2026
@IWANABETHATGUY IWANABETHATGUY force-pushed the chunk-already-loaded2 branch from 047790c to b6e976c Compare May 9, 2026 15:00
@IWANABETHATGUY IWANABETHATGUY changed the base branch from graphite-base/9305 to main May 9, 2026 15:00
@IWANABETHATGUY IWANABETHATGUY force-pushed the chunk-already-loaded2 branch 2 times, most recently from a508d8d to 5b1ec0a Compare May 9, 2026 17:51
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review May 10, 2026 08:46
@hyf0

hyf0 commented May 10, 2026

Copy link
Copy Markdown
Member

Any context/issue about this PR?

@hyf0

This comment was marked as outdated.

@hyf0

hyf0 commented May 10, 2026

Copy link
Copy Markdown
Member

So shared.js does not need to keep the route entry bit.

Will this PR actually make shared.js's bitset not contain route.js or this PR is only a post optimization and it doesn't affect shared.js's bitset?

Comment thread crates/rolldown/tests/rolldown/issues/8989/artifacts.snap
@IWANABETHATGUY

Copy link
Copy Markdown
Member Author

Any context/issue about this PR?

#9305 (comment), It is an optimization pass port from rollup, and I have already illustrated the algorithm in the body of pr.

@IWANABETHATGUY IWANABETHATGUY force-pushed the chunk-already-loaded2 branch from 5b1ec0a to 401218c Compare May 10, 2026 16:00

@hyf0 hyf0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IWANABETHATGUY commented May 11, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • May 11, 5:05 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.
  • May 11, 5:05 AM UTC: IWANABETHATGUY added this pull request to the Graphite merge queue.
  • May 11, 5:10 AM UTC: Merged by the Graphite merge queue.

Adds a pre-materialization chunk optimization pass that removes dynamic-entry dependency bits for modules already guaranteed to be loaded by every importer of that dynamic entry. This lets shared modules stay grouped with the static entry path instead of being split into extra chunks or pulled into dynamic chunks unnecessarily.

### Already-Loaded Algorithm

Consider this module graph:

```mermaid
flowchart LR
  main["main entry"]
  shared["shared.js"]
  route["route.js dynamic entry"]

  main -->|"static import"| shared
  main -.->|"dynamic import()"| route
  route -->|"static import"| shared
```

When `route.js` runs, `main` has already loaded `shared.js`. So `shared.js` does not need to keep the `route` entry bit.

Before reduction:

```txt
shared.js dependent entries = { main, route }
route.js  dependent entries = { route }
```

After reduction:

```txt
shared.js dependent entries = { main }
route.js  dependent entries = { route }
```

That lets normal chunk grouping put `shared.js` with `main`, while `route.js` imports the binding from the already-loaded chunk instead of forcing an extra shared chunk.

The pass works on temporary **atoms**, where each atom is a group of modules with the same dependent-entry bitset:

```txt
atom(shared.js) = {
  modules: [shared.js],
  dependentEntries: { main, route }
}
```

Then it computes the atoms loaded before each dynamic entry executes:

```txt
staticLoaded[entry] = atoms loaded by the entry's static dependency graph
alreadyLoaded[dynamicEntry] = intersection of loaded atoms from every importer entry
```

The key detail is the **intersection**. If `route.js` can be dynamically imported by both `main1` and `main2`, an atom can only drop the `route` bit if it is already loaded by **both** importers.

Pseudo-code:

```txt
atoms = groupModulesByDependentEntryBits()
staticLoaded = computeStaticLoadedAtomsForEachEntry()

for each included dynamic import:
  dynamicImporters[dynamicEntry].add(importerEntry)

queue = all dynamic entries

while queue is not empty:
  dynamicEntry = queue.pop()

  nextAlreadyLoaded = intersection over importerEntry in dynamicImporters[dynamicEntry] of:
    staticLoaded[importerEntry] union alreadyLoaded[importerEntry]

  if nextAlreadyLoaded changed:
    alreadyLoaded[dynamicEntry] = nextAlreadyLoaded

    for childDynamicEntry imported by dynamicEntry:
      dynamicImporters[childDynamicEntry].add(dynamicEntry)
      queue.push(childDynamicEntry)

for each atom:
  for each dynamicEntry bit in atom.dependentEntries:
    if alreadyLoaded[dynamicEntry] contains atom:
      remove dynamicEntry bit from atom
```

The reduced bits are only accepted when chunk safety is preserved. The pass still bails out for TLA, keeps strict entry signatures intact, handles runtime-only atoms and dynamic-entry modules specially, and checks static import cycles in risky cases such as manual chunks or strict signatures.

### Summary

- Add dynamic already-loaded analysis for `experimental.chunkOptimization`.
- Propagate already-loaded atoms across included dynamic imports with a fixed-point worklist.
- Preserve safety checks for TLA, strict entry signatures, manual chunking, and static import cycles.
- Add `BitSet::all` and `BitSet::intersect` helpers.
- Add basic and multi-entry dynamic already-loaded fixtures.
- Update affected snapshots and runtime/static-cycle assertions.
- Document the new analysis in `meta/design/code-splitting.md`.

### Tests

- Added fixture coverage under `function/chunk_optimization/dynamic_already_loaded_*`.
- Updated regression coverage for chunk merging/static import cycle cases affected by the new grouping.
@graphite-app graphite-app Bot force-pushed the chunk-already-loaded2 branch from 401218c to 7e5855c Compare May 11, 2026 05:06
@graphite-app graphite-app Bot merged commit 7e5855c into main May 11, 2026
33 checks passed
@graphite-app graphite-app Bot deleted the chunk-already-loaded2 branch May 11, 2026 05:10
graphite-app Bot pushed a commit that referenced this pull request May 11, 2026
The issue is fixed in #9305
Adds a regression test (`crates/rolldown/tests/rolldown/optimization/chunk_merging/cjs_facade_reexport_merges_into_entry`) reproducing the scenario from #9331 — two CJS facade modules (`module.exports = require('./*-impl.js')`) consumed across a chain of nested dynamic imports.

Snapshots the expected 3-chunk output (user entry / dynamic route / dynamic child, with both facades and their impls merged into the user entry — no stray common chunk for the facade) so future changes to the chunk optimizer can't regress this case.

closed #9331
@rolldown-guard rolldown-guard Bot mentioned this pull request May 13, 2026
shulaoda added a commit that referenced this pull request May 13, 2026
## [1.0.1] - 2026-05-13

### 🚀 Features

- experimental/lazy-barrel: advice on oversized barrel modules (#9236) by @shulaoda
- rolldown: inline optional-chain enum access (#9379) by @Dunqing
- chunk-optimization: dedupe already-loaded dynamic deps (#9305) by @IWANABETHATGUY
- binding: call moduleParsed hook in ParallelJsPlugin (#9318) by @jaehafe

### 🐛 Bug Fixes

- transform: enable `enum_eval` for `transformSync` and vite TS transform (#9325) by @Dunqing
- error: remove severity prefix from diagnostic messages (#9262) by @Kyujenius
- deps: pin pnpm to 10.23.0 to work around catalog mismatch on Netlify (#9364) by @shulaoda
- ci: pin mimalloc-safe to 0.1.58 (#9361) by @shulaoda
- dev/lazy: fix exports of lazy requests in lazy chunks (#9249) by @h-a-n-a
- rolldown_plugin_vite_resolve: handle errors in `resolveSubpathImports` callback (#9355) by @sapphi-red
- rolldown_plugin_lazy_compilation: use loadExports for fetched proxy to preserve original export names (#9132) by @h-a-n-a
- common: include offending index in HybridIndexVec panic message (#9296) by @SAY-5

### 🚜 Refactor

- ecmascript: extract semantic_builder_for_transform helper (#9326) by @Dunqing
- test: extract reusable static-import-cycle helper (#9332) by @IWANABETHATGUY

### 📚 Documentation

- clarify scope of `topLevelVar` (#9380) by @IWANABETHATGUY
- meta/design: add ast-mutation design doc (#9338) by @hyf0
- feat: add ai policy in contribution guide (#9315) by @mdong1909

### ⚡ Performance

- binding: enable mimalloc v3 to reduce idle memory (#9349) by @shulaoda

### 🧪 Testing

- mcs: cover require() in `$initial` group (#9376) by @hyf0
- add regression for CJS facade chunk merge into entry (#9351) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- switch prepare-release to manual dispatch with version input (#9383) by @shulaoda
- migrate `@rolldown/pluginutils` to `rolldown/plugins` (#9317) by @shulaoda
- deps: pin libmimalloc-sys2 to 0.1.54 (#9372) by @shulaoda
- replace `igorskyflyer/action-readfile` with `cat` (#9369) by @sapphi-red
- deps: update test262 submodule for tests (#9371) by @rolldown-guard[bot]
- use app token for test dep update PRs (#9368) by @sapphi-red
- replace some actions with gh commands (#9367) by @sapphi-red
- replace action-semantic-pull-request with inline regex (#9366) by @sapphi-red
- remove pull_request_target workflows (#9188) by @Boshen
- deps: upgrade oxc to 0.130.0 (#9360) by @shulaoda
- deps: update github actions (major) (#9348) by @renovate[bot]
- deps: update github actions (#9341) by @renovate[bot]
- deps: update rust crates (#9344) by @renovate[bot]
- deps: update crate-ci/typos action to v1.46.1 (#9357) by @renovate[bot]
- deps: update npm packages (#9343) by @renovate[bot]
- deps: update pnpm to v10.33.4 (#9347) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to ^0.25.0 (#9346) by @renovate[bot]
- .claude: add rolldown-repl encoder, rename decode skill (#9352) by @IWANABETHATGUY
- deps: update crate-ci/typos action to v1.46.0 (#9345) by @renovate[bot]
- deps: update napi to v3.8.6 (#9342) by @renovate[bot]
- deps: update dependency vite-plus to v0.1.20 (#9340) by @renovate[bot]
- enable rollup chunking-form test (#9335) by @IWANABETHATGUY
- typo: fix typo in watcher options comment (#9324) by @thescripted

### ❤️ New Contributors

* @Kyujenius made their first contribution in [#9262](#9262)
* @SAY-5 made their first contribution in [#9296](#9296)
* @thescripted made their first contribution in [#9324](#9324)

Co-authored-by: shulaoda <[email protected]>
graphite-app Bot pushed a commit that referenced this pull request May 15, 2026
### Root cause

PR #9270 added a dynamic-dominator fallback for common chunk merging. That fallback treated dynamic-import reachability as proof that one dynamic entry was already loaded before another.

That is unsound for sibling dynamic imports. A user entry can expose both `load1() => import('./d1')` and `load2() => import('./d2')`, but calling only `load2()` must not execute `d1` side effects.

The test case introduced by #9270 is already covered by #9305, so this PR does not add duplicate coverage for that path. Instead, it covers the sibling dynamic-import case where dynamic reachability does not imply execution order.

I verified this against #9270 directly:

- Parent of #9270 (`2b235157c`) passes the sibling dynamic repro.
- #9270 merge commit (`0b257a924`) fails with `['main', 'shared', 'd1 main', 'd2']`.
- The generated `d2.js` imports `./d1.js`, causing the side-effect leak.

### Changes

- Remove the dynamic-only dominator fallback from common chunk merge target selection.
- Keep dynamic-only shared modules in a separate common chunk unless the existing static `find_merge_target` proof can find a safe target.
- Add an `issues/9350` regression fixture that enables only `mergeCommonChunks` and asserts `main.load2()` does not execute `d1`.
- Document why dynamic-import reachability alone is not a safe merge proof.

### Tests

```sh
just t-run crates/rolldown/tests/rolldown/issues/9350/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_chain/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_siblings_no_merge/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_with_exports_no_merge/_config.json
```

closed #9350
IWANABETHATGUY pushed a commit that referenced this pull request May 18, 2026
## [1.0.1] - 2026-05-13

### 🚀 Features

- experimental/lazy-barrel: advice on oversized barrel modules (#9236) by @shulaoda
- rolldown: inline optional-chain enum access (#9379) by @Dunqing
- chunk-optimization: dedupe already-loaded dynamic deps (#9305) by @IWANABETHATGUY
- binding: call moduleParsed hook in ParallelJsPlugin (#9318) by @jaehafe

### 🐛 Bug Fixes

- transform: enable `enum_eval` for `transformSync` and vite TS transform (#9325) by @Dunqing
- error: remove severity prefix from diagnostic messages (#9262) by @Kyujenius
- deps: pin pnpm to 10.23.0 to work around catalog mismatch on Netlify (#9364) by @shulaoda
- ci: pin mimalloc-safe to 0.1.58 (#9361) by @shulaoda
- dev/lazy: fix exports of lazy requests in lazy chunks (#9249) by @h-a-n-a
- rolldown_plugin_vite_resolve: handle errors in `resolveSubpathImports` callback (#9355) by @sapphi-red
- rolldown_plugin_lazy_compilation: use loadExports for fetched proxy to preserve original export names (#9132) by @h-a-n-a
- common: include offending index in HybridIndexVec panic message (#9296) by @SAY-5

### 🚜 Refactor

- ecmascript: extract semantic_builder_for_transform helper (#9326) by @Dunqing
- test: extract reusable static-import-cycle helper (#9332) by @IWANABETHATGUY

### 📚 Documentation

- clarify scope of `topLevelVar` (#9380) by @IWANABETHATGUY
- meta/design: add ast-mutation design doc (#9338) by @hyf0
- feat: add ai policy in contribution guide (#9315) by @mdong1909

### ⚡ Performance

- binding: enable mimalloc v3 to reduce idle memory (#9349) by @shulaoda

### 🧪 Testing

- mcs: cover require() in `$initial` group (#9376) by @hyf0
- add regression for CJS facade chunk merge into entry (#9351) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- switch prepare-release to manual dispatch with version input (#9383) by @shulaoda
- migrate `@rolldown/pluginutils` to `rolldown/plugins` (#9317) by @shulaoda
- deps: pin libmimalloc-sys2 to 0.1.54 (#9372) by @shulaoda
- replace `igorskyflyer/action-readfile` with `cat` (#9369) by @sapphi-red
- deps: update test262 submodule for tests (#9371) by @rolldown-guard[bot]
- use app token for test dep update PRs (#9368) by @sapphi-red
- replace some actions with gh commands (#9367) by @sapphi-red
- replace action-semantic-pull-request with inline regex (#9366) by @sapphi-red
- remove pull_request_target workflows (#9188) by @Boshen
- deps: upgrade oxc to 0.130.0 (#9360) by @shulaoda
- deps: update github actions (major) (#9348) by @renovate[bot]
- deps: update github actions (#9341) by @renovate[bot]
- deps: update rust crates (#9344) by @renovate[bot]
- deps: update crate-ci/typos action to v1.46.1 (#9357) by @renovate[bot]
- deps: update npm packages (#9343) by @renovate[bot]
- deps: update pnpm to v10.33.4 (#9347) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to ^0.25.0 (#9346) by @renovate[bot]
- .claude: add rolldown-repl encoder, rename decode skill (#9352) by @IWANABETHATGUY
- deps: update crate-ci/typos action to v1.46.0 (#9345) by @renovate[bot]
- deps: update napi to v3.8.6 (#9342) by @renovate[bot]
- deps: update dependency vite-plus to v0.1.20 (#9340) by @renovate[bot]
- enable rollup chunking-form test (#9335) by @IWANABETHATGUY
- typo: fix typo in watcher options comment (#9324) by @thescripted

### ❤️ New Contributors

* @Kyujenius made their first contribution in [#9262](#9262)
* @SAY-5 made their first contribution in [#9296](#9296)
* @thescripted made their first contribution in [#9324](#9324)

Co-authored-by: shulaoda <[email protected]>
IWANABETHATGUY added a commit that referenced this pull request May 18, 2026
### Root cause

PR #9270 added a dynamic-dominator fallback for common chunk merging. That fallback treated dynamic-import reachability as proof that one dynamic entry was already loaded before another.

That is unsound for sibling dynamic imports. A user entry can expose both `load1() => import('./d1')` and `load2() => import('./d2')`, but calling only `load2()` must not execute `d1` side effects.

The test case introduced by #9270 is already covered by #9305, so this PR does not add duplicate coverage for that path. Instead, it covers the sibling dynamic-import case where dynamic reachability does not imply execution order.

I verified this against #9270 directly:

- Parent of #9270 (`2b235157c`) passes the sibling dynamic repro.
- #9270 merge commit (`0b257a924`) fails with `['main', 'shared', 'd1 main', 'd2']`.
- The generated `d2.js` imports `./d1.js`, causing the side-effect leak.

### Changes

- Remove the dynamic-only dominator fallback from common chunk merge target selection.
- Keep dynamic-only shared modules in a separate common chunk unless the existing static `find_merge_target` proof can find a safe target.
- Add an `issues/9350` regression fixture that enables only `mergeCommonChunks` and asserts `main.load2()` does not execute `d1`.
- Document why dynamic-import reachability alone is not a safe merge proof.

### Tests

```sh
just t-run crates/rolldown/tests/rolldown/issues/9350/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_chain/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_siblings_no_merge/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_with_exports_no_merge/_config.json
```

closed #9350
kjanoudi added a commit to kjanoudi/rolldown that referenced this pull request May 19, 2026
…en dynamic siblings would back-import them

Both `dynamic_already_loaded` and `try_insert_common_module_to_exist_chunk`
will hoist a wrapped (CJS or wrapped-ESM) module's atom into a static
entry when the atom is shared between that entry and one or more
effective dynamic-entry chunks. Once hoisted, the sibling dynamic
chunks emit `import { require_X as t } from "./entry.js"` and call
`require_X()` at top level. Pure ESM postpones the dynamic edge so the
`var require_X = __commonJSMin(...)` slot is initialized in time, but
embedders that compose chunks via static imports (Cloudflare Workers
SSR + react-router static route manifests, etc.) evaluate the child
chunk's body before the entry's body completes and hit TDZ on
`require_X is not a function`.

Refuse the hoist in two places when an atom is wrapped:

1. In `can_use_reduced_dependent_entries`, gate the reduction on whether
   any removed dynamic entry is **effective** — its module body carries
   only its own entry bit, so it will emerge as a separate chunk that
   would back-import the wrapper. `INEFFECTIVE` dynamic entries (their
   modules co-located with the surviving parent, as in rolldown#8361 / #8361_2)
   are still allowed to reduce — preserving PR rolldown#9305's fix for that
   cycle pattern. The check peeks at `index_splitting_info` because the
   write happens only after every atom has been decided.

2. In `try_insert_into_existing_chunk`, refuse the merge when the
   candidate's modules contain any wrapped module **and** a dynamic
   entry chunk also depends on the candidate. Keeping the wrapper in a
   shared common chunk lets every consumer pick it up as a leaf import
   with no back-edge to the parent entry. Pure-ESM atoms are
   unaffected.

Closes rolldown#9441
kjanoudi added a commit to kjanoudi/rolldown that referenced this pull request May 19, 2026
…en dynamic siblings would back-import them

Both `dynamic_already_loaded` and `try_insert_common_module_to_exist_chunk`
will hoist a wrapped (CJS or wrapped-ESM) module's atom into a static
entry when the atom is shared between that entry and one or more
effective dynamic-entry chunks. Once hoisted, the sibling dynamic
chunks emit `import { require_X as t } from "./entry.js"` and call
`require_X()` at top level. Pure ESM postpones the dynamic edge so the
`var require_X = __commonJSMin(...)` slot is initialized in time, but
embedders that compose chunks via static imports (Cloudflare Workers
SSR + react-router static route manifests, etc.) evaluate the child
chunk's body before the entry's body completes and hit TDZ on
`require_X is not a function`.

Refuse the hoist in two places when an atom is wrapped:

1. In `can_use_reduced_dependent_entries`, gate the reduction on whether
   any removed dynamic entry is **effective** — its module body carries
   only its own entry bit, so it will emerge as a separate chunk that
   would back-import the wrapper. `INEFFECTIVE` dynamic entries (their
   modules co-located with the surviving parent, as in rolldown#8361 / #8361_2)
   are still allowed to reduce — preserving PR rolldown#9305's fix for that
   cycle pattern. The check peeks at `index_splitting_info` because the
   write happens only after every atom has been decided.

2. In `try_insert_into_existing_chunk`, refuse the merge when the
   candidate's modules contain any wrapped module **and** a dynamic
   entry chunk also depends on the candidate. Keeping the wrapper in a
   shared common chunk lets every consumer pick it up as a leaf import
   with no back-edge to the parent entry. Pure-ESM atoms are
   unaffected.

Closes rolldown#9441
kjanoudi added a commit to kjanoudi/rolldown that referenced this pull request May 19, 2026
…en dynamic siblings would back-import them

Both `dynamic_already_loaded` and `try_insert_common_module_to_exist_chunk`
will hoist a wrapped (CJS or wrapped-ESM) module's atom into a static
entry when the atom is shared between that entry and one or more
effective dynamic-entry chunks. Once hoisted, the sibling dynamic
chunks emit `import { require_X as t } from "./entry.js"` and call
`require_X()` at top level. Pure ESM postpones the dynamic edge so the
`var require_X = __commonJSMin(...)` slot is initialized in time, but
embedders that compose chunks via static imports (Cloudflare Workers
SSR + react-router static route manifests, etc.) evaluate the child
chunk's body before the entry's body completes and hit TDZ on
`require_X is not a function`.

Refuse the hoist in two places when an atom is wrapped:

1. In `can_use_reduced_dependent_entries`, gate the reduction on whether
   any removed dynamic entry is **effective** — its module body carries
   only its own entry bit, so it will emerge as a separate chunk that
   would back-import the wrapper. `INEFFECTIVE` dynamic entries (their
   modules co-located with the surviving parent, as in rolldown#8361 / #8361_2)
   are still allowed to reduce — preserving PR rolldown#9305's fix for that
   cycle pattern. The check peeks at `index_splitting_info` because the
   write happens only after every atom has been decided.

2. In `try_insert_into_existing_chunk`, refuse the merge when the
   candidate's modules contain any wrapped module **and** a dynamic
   entry chunk also depends on the candidate. Keeping the wrapper in a
   shared common chunk lets every consumer pick it up as a leaf import
   with no back-edge to the parent entry. Pure-ESM atoms are
   unaffected.

Closes rolldown#9441
V1OL3TF0X pushed a commit to V1OL3TF0X/rolldown that referenced this pull request May 25, 2026
)

### Root cause

PR rolldown#9270 added a dynamic-dominator fallback for common chunk merging. That fallback treated dynamic-import reachability as proof that one dynamic entry was already loaded before another.

That is unsound for sibling dynamic imports. A user entry can expose both `load1() => import('./d1')` and `load2() => import('./d2')`, but calling only `load2()` must not execute `d1` side effects.

The test case introduced by rolldown#9270 is already covered by rolldown#9305, so this PR does not add duplicate coverage for that path. Instead, it covers the sibling dynamic-import case where dynamic reachability does not imply execution order.

I verified this against rolldown#9270 directly:

- Parent of rolldown#9270 (`2b235157c`) passes the sibling dynamic repro.
- rolldown#9270 merge commit (`0b257a924`) fails with `['main', 'shared', 'd1 main', 'd2']`.
- The generated `d2.js` imports `./d1.js`, causing the side-effect leak.

### Changes

- Remove the dynamic-only dominator fallback from common chunk merge target selection.
- Keep dynamic-only shared modules in a separate common chunk unless the existing static `find_merge_target` proof can find a safe target.
- Add an `issues/9350` regression fixture that enables only `mergeCommonChunks` and asserts `main.load2()` does not execute `d1`.
- Document why dynamic-import reachability alone is not a safe merge proof.

### Tests

```sh
just t-run crates/rolldown/tests/rolldown/issues/9350/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_chain/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_siblings_no_merge/_config.json
just t-run crates/rolldown/tests/rolldown/optimization/chunk_merging/dynamic_dominator_with_exports_no_merge/_config.json
```

closed rolldown#9350
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.

2 participants