Skip to content

fix: reduce false positives in chunk optimizer circular dependency detection#9049

Merged
IWANABETHATGUY merged 12 commits intorolldown:mainfrom
AlonMiz:fix/chunk-optimizer-precise-cycle-detection
Apr 16, 2026
Merged

fix: reduce false positives in chunk optimizer circular dependency detection#9049
IWANABETHATGUY merged 12 commits intorolldown:mainfrom
AlonMiz:fix/chunk-optimizer-precise-cycle-detection

Conversation

@AlonMiz
Copy link
Copy Markdown
Contributor

@AlonMiz AlonMiz commented Apr 9, 2026

Summary

PR #8371 improved circular dependency detection in the chunk optimizer to fix #8361 (__commonJSMin is not a function). However, the fix introduced an over-approximation that blocks many legitimate chunk merges, causing a significant regression in bundle size and file count.

Fixes #9093

Problem

The current algorithm in would_create_circular_dependency does a BFS from source.deps ∪ target.deps and simulates post-merge edges for every chunk that depends on source. This causes false positives because:

  1. Starting BFS from target.deps means target can trivially "reach itself" through any transitive dependency that also depends on source
  2. The post-merge edge simulation amplifies this by redirecting any source-dependent chunk back to target

In a large application with ~2,200 entry points, this blocks 1,046 legitimate merges, producing +1,224 extra chunks and +8% bundle size.

Fix

Split the algorithm into two cases based on whether source has dependencies:

  • Source has deps: Only BFS from source.deps without post-merge simulation. This is sufficient because if target is reachable from source's dependency tree, merging would create a real cycle.

  • Source has no deps (e.g., runtime chunk): Keep the full algorithm with target.deps BFS and post-merge simulation. This correctly detects cycles like target → chunk_A → source(=target after merge), which is the pattern from [Bug]: __commonJSMin is not a function since rc4 #8361.

Results

Tested on a large application (~2,200 entry points):

Metric Before (rc.9) After (this fix) Improvement
JS files 3,212 2,418 -794 (-25%)
JS size 73.13 MB 63.99 MB -9.14 MB (-12.5%)

Testing

  • #8361 and #8361_2 integration tests pass (the bug this algorithm was designed to fix)
  • ✅ Full integration test suite: 1,652 passed, 0 failed (1 skipped due to missing test262 submodule)
  • ✅ New regression test added (#9049)
  • ✅ Verified on large production application

Fixes vitejs/vite#22007

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 9, 2026

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing AlonMiz:fix/chunk-optimizer-precise-cycle-detection (99b615b) with main (d9be119)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 (52650e7) during the generation of this report, so d9be119 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 9, 2026

Open in StackBlitz

@rolldown/browser

npm i https://pkg.pr.new/@rolldown/browser@9049

@rolldown/debug

npm i https://pkg.pr.new/@rolldown/debug@9049

@rolldown/pluginutils

npm i https://pkg.pr.new/@rolldown/pluginutils@9049

rolldown

npm i https://pkg.pr.new/rolldown@9049

@rolldown/binding-android-arm64

npm i https://pkg.pr.new/@rolldown/binding-android-arm64@9049

@rolldown/binding-darwin-arm64

npm i https://pkg.pr.new/@rolldown/binding-darwin-arm64@9049

@rolldown/binding-darwin-x64

npm i https://pkg.pr.new/@rolldown/binding-darwin-x64@9049

@rolldown/binding-freebsd-x64

npm i https://pkg.pr.new/@rolldown/binding-freebsd-x64@9049

@rolldown/binding-linux-arm-gnueabihf

npm i https://pkg.pr.new/@rolldown/binding-linux-arm-gnueabihf@9049

@rolldown/binding-linux-arm64-gnu

npm i https://pkg.pr.new/@rolldown/binding-linux-arm64-gnu@9049

@rolldown/binding-linux-arm64-musl

npm i https://pkg.pr.new/@rolldown/binding-linux-arm64-musl@9049

@rolldown/binding-linux-ppc64-gnu

npm i https://pkg.pr.new/@rolldown/binding-linux-ppc64-gnu@9049

@rolldown/binding-linux-s390x-gnu

npm i https://pkg.pr.new/@rolldown/binding-linux-s390x-gnu@9049

@rolldown/binding-linux-x64-gnu

npm i https://pkg.pr.new/@rolldown/binding-linux-x64-gnu@9049

@rolldown/binding-linux-x64-musl

npm i https://pkg.pr.new/@rolldown/binding-linux-x64-musl@9049

@rolldown/binding-openharmony-arm64

npm i https://pkg.pr.new/@rolldown/binding-openharmony-arm64@9049

@rolldown/binding-wasm32-wasi

npm i https://pkg.pr.new/@rolldown/binding-wasm32-wasi@9049

@rolldown/binding-win32-arm64-msvc

npm i https://pkg.pr.new/@rolldown/binding-win32-arm64-msvc@9049

@rolldown/binding-win32-x64-msvc

npm i https://pkg.pr.new/@rolldown/binding-win32-x64-msvc@9049

commit: ff7d744

@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 9, 2026

trying to add some tests... will update soon 🙏

@IWANABETHATGUY IWANABETHATGUY marked this pull request as draft April 9, 2026 12:32
AlonMiz pushed a commit to AlonMiz/rolldown that referenced this pull request Apr 12, 2026
…wn#9049)

Adds a multi-entry test case that verifies the chunk optimizer does not
create circular static imports when merging common chunks with
dependencies. This complements the rolldown#8361 test which covers the runtime
chunk (empty deps) case.
@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 12, 2026

I've added a tests to show the reproduction case 🙏

@IWANABETHATGUY
Copy link
Copy Markdown
Member

I am sorry, but the main branch produces identical results to this branch. We at least need a reproduction to verify the effectiveness of the issue-fixing PR.

@AlonMiz AlonMiz force-pushed the fix/chunk-optimizer-precise-cycle-detection branch from 66cd2ec to a951454 Compare April 14, 2026 06:17
@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 14, 2026

@IWANABETHATGUY Thanks for looking at this! I've added 5 unit tests that directly demonstrate the false positive in would_create_circular_dependency.

The key test is test_no_false_positive_when_source_has_deps:

Chunk graph:
  0 (target/entry) -> deps: {1, 2}
  1 (common/source) -> deps: {2}
  2 (common/lib) -> deps: {}

Merging chunk 1 into chunk 0 is safe — no cycle would be created.

Old algorithm (BFS from source.deps ∪ target.deps with simulation):

  1. BFS seeds: {2} ∪ {1, 2} = {1, 2}
  2. Visit chunk 1: 1.deps.contains(source=1)? → YES → queue target(0)
  3. Visit chunk 0: found target → FALSE POSITIVE

New algorithm (source has deps → BFS from source.deps only):

  1. BFS seeds: {2}
  2. Visit chunk 2: deps={}. Not target → no cycle → CORRECT

The integration test (issues/9049/) doesn't show a difference because the chunk optimizer's try_insert_into_existing_chunk only merges under specific conditions (single-entry reference, entry chunk kind, etc.) that are hard to trigger with a small test. But the unit tests prove the algorithm difference directly.

You can run them with:

cargo test -p rolldown --lib -- chunk_optimizer::tests

@IWANABETHATGUY
Copy link
Copy Markdown
Member

@IWANABETHATGUY Thanks for looking at this! I've added 5 unit tests that directly demonstrate the false positive in would_create_circular_dependency.

The key test is test_no_false_positive_when_source_has_deps:

Chunk graph:
  0 (target/entry) -> deps: {1, 2}
  1 (common/source) -> deps: {2}
  2 (common/lib) -> deps: {}

Merging chunk 1 into chunk 0 is safe — no cycle would be created.

Old algorithm (BFS from source.deps ∪ target.deps with simulation):

  1. BFS seeds: {2} ∪ {1, 2} = {1, 2}
  2. Visit chunk 1: 1.deps.contains(source=1)? → YES → queue target(0)
  3. Visit chunk 0: found target → FALSE POSITIVE

New algorithm (source has deps → BFS from source.deps only):

  1. BFS seeds: {2}
  2. Visit chunk 2: deps={}. Not target → no cycle → CORRECT

The integration test (issues/9049/) doesn't show a difference because the chunk optimizer's try_insert_into_existing_chunk only merges under specific conditions (single-entry reference, entry chunk kind, etc.) that are hard to trigger with a small test. But the unit tests prove the algorithm difference directly.

You can run them with:

cargo test -p rolldown --lib -- chunk_optimizer::tests

That did not make much sense. I also let Claude generate a counter-example with a unit test. We need an integration test. I will try another project and create a minimal reproduction first.

@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 14, 2026

That did not make much sense. I also let Claude generate a counter-example with a unit test. We need an integration test. I will try another project and create a minimal reproduction first.

im working on a reproduction as well 🙏 hope to update soon

@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 14, 2026

@IWANABETHATGUY I've been trying to create a minimal bundler-level reproduction, but the false positive only triggers in Angular applications built with Vite. Simple JS module graphs don't create the specific chunk dependency topology needed.

The reason: Angular's compiled output creates complex implicit dependency chains through dependency injection, decorators, and component metadata. These create a dense module dependency graph where calc_chunk_dependencies produces chunk-level dependencies that trigger the false positive in would_create_circular_dependency.

The unit tests I added directly prove the algorithm bugtest_no_false_positive_when_source_has_deps shows a chunk graph where the old algorithm returns true (false positive) but should return false.

I'm working on creating an Angular + Vite reproduction project with lazy routes that demonstrates the chunk count difference. Will update soon.

@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 14, 2026

Debug Output from Production App

I've added debug logging to the original algorithm and built against our production Angular app. Here's the actual data:

[BLOCKED] source=1232 (deps=[1229]), target=0 (deps=[2215, 1485, 1334, ...300+ deps])
[BLOCKED] source=1231 (deps=[1242, 1249, 1234, 1230, ...10 deps]), target=0 (deps=[...300+ deps])
[CHUNK_OPT] total blocked: 1046

Key observations:

  • ALL 1,046 blocked merges target chunk 0 (the main entry chunk)
  • Chunk 0 has 300+ dependencies (it's the Angular app's main entry)
  • Source chunks have small dep lists (1-10 deps)
  • The false positive occurs because BFS from target.deps (300+ chunks) inevitably reaches a chunk that depends on source, triggering the simulation

Why simple reproductions don't work:
The false positive requires the target chunk to have hundreds of dependencies. This only happens in large applications where the main entry transitively depends on many modules. In simple test projects, the main entry has few dependencies, so the BFS from target.deps doesn't reach enough chunks to trigger the false positive.

The false positive is also cascading — early merges via merge_chunk_dependencies add more deps to the target chunk, making subsequent merges more likely to trigger false positives.

The unit tests directly prove the algorithm bug with a crafted chunk graph that has the same topology:

  • Source with deps {A}
  • Target with deps {source, A}
  • Old algorithm: BFS from {A, source, A}, visits source, source.deps.contains(source) → YES → queue target → FOUND → false positive
  • New algorithm: source has deps → BFS from source.deps only {A}, visits A, A has no deps → no cycle → correct

I understand you need a bundler-level reproduction, but the false positive only manifests in apps with 2000+ entry points where the main entry accumulates 300+ chunk dependencies. I've tried 10+ different reproduction approaches (simple JS, Angular + Vite, virtual modules, 500 entries) and none trigger it because the chunk optimizer merges everything before the dependency graph becomes complex enough.

Would you be open to reviewing the unit tests as the proof, or would you prefer I provide access to our production build logs?

@IWANABETHATGUY
Copy link
Copy Markdown
Member

Debug Output from Production App

I've added debug logging to the original algorithm and built against our production Angular app. Here's the actual data:

[BLOCKED] source=1232 (deps=[1229]), target=0 (deps=[2215, 1485, 1334, ...300+ deps])
[BLOCKED] source=1231 (deps=[1242, 1249, 1234, 1230, ...10 deps]), target=0 (deps=[...300+ deps])
[CHUNK_OPT] total blocked: 1046

Key observations:

  • ALL 1,046 blocked merges target chunk 0 (the main entry chunk)
  • Chunk 0 has 300+ dependencies (it's the Angular app's main entry)
  • Source chunks have small dep lists (1-10 deps)
  • The false positive occurs because BFS from target.deps (300+ chunks) inevitably reaches a chunk that depends on source, triggering the simulation

Why simple reproductions don't work: The false positive requires the target chunk to have hundreds of dependencies. This only happens in large applications where the main entry transitively depends on many modules. In simple test projects, the main entry has few dependencies, so the BFS from target.deps doesn't reach enough chunks to trigger the false positive.

The false positive is also cascading — early merges via merge_chunk_dependencies add more deps to the target chunk, making subsequent merges more likely to trigger false positives.

The unit tests directly prove the algorithm bug with a crafted chunk graph that has the same topology:

  • Source with deps {A}
  • Target with deps {source, A}
  • Old algorithm: BFS from {A, source, A}, visits source, source.deps.contains(source) → YES → queue target → FOUND → false positive
  • New algorithm: source has deps → BFS from source.deps only {A}, visits A, A has no deps → no cycle → correct

I understand you need a bundler-level reproduction, but the false positive only manifests in apps with 2000+ entry points where the main entry accumulates 300+ chunk dependencies. I've tried 10+ different reproduction approaches (simple JS, Angular + Vite, virtual modules, 500 entries) and none trigger it because the chunk optimizer merges everything before the dependency graph becomes complex enough.

Would you be open to reviewing the unit tests as the proof, or would you prefer I provide access to our production build logs?

I will do it by myself.

@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 14, 2026

Reproduction

I created a minimal reproduction: https://github.com/AlonMiz/rolldown-9049-repro

Setup

git clone https://github.com/AlonMiz/rolldown-9049-repro.git
cd rolldown-9049-repro
npm install rolldown@latest
node generate.mjs
node build.mjs

Results

Version Total Chunks Common Chunks Regression
rc.4 (before #8371) 236 35
rc.5 (after #8371) 256 55 +20 extra
rc.15 (latest) 271 70 +35 extra

The project has 1 main entry that dynamically imports 200 route modules. Routes share 50 services and 20 utilities with overlapping dependencies. The chunk optimizer should merge shared modules into existing entry chunks, but #8371's overly-conservative circular dependency check blocks legitimate merges.

Run bash compare.sh to reproduce the version comparison automatically.

@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 14, 2026

cc @brandonroberts — this regression from #8371 likely affects Analog and other Angular/Vite projects with many lazy-loaded routes. The overly-conservative cycle detection in the chunk optimizer blocks legitimate merges, creating extra common chunks. Would be great to get your input on this.

…tection

When source has dependencies, only BFS from source's deps instead of
source.deps ∪ target.deps. Including target's deps causes false positives
because the simulation finds that target can trivially reach itself through
any transitive dependency that also depends on source.

When source has no dependencies (e.g., runtime chunk), BFS from target's
deps with post-merge simulation to detect real cycles like rolldown#8361.

Closes rolldown#9049
@AlonMiz AlonMiz force-pushed the fix/chunk-optimizer-precise-cycle-detection branch from d1fffe2 to 761a555 Compare April 15, 2026 07:51
@AlonMiz
Copy link
Copy Markdown
Contributor Author

AlonMiz commented Apr 15, 2026

Done! I've cleaned the PR:

  • Rebased on latest main
  • Squashed into a single commit
  • Replaced the integration test with your minimal reproduction (main + 2 routes + 2 services + 2 utils with dynamic imports)
  • Test asserts 3 chunks (rc.4 behavior) — without the fix, it produces 4 chunks (extra common chunk)

Verified locally:

  • 5 unit tests pass ✅
  • has_side_effects field added to test helper (new field from upstream)
  • Reproduction confirmed: rc.4 → 3 chunks, rc.15 → 4 chunks

autofix-ci Bot and others added 2 commits April 15, 2026 07:52
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 15, 2026

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 99b615b
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69e05c6cb6cb0b00084e71e7

IWANABETHATGUY and others added 4 commits April 15, 2026 17:03
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tection

When source has dependencies, only BFS from source's deps instead of
source.deps ∪ target.deps. Including target's deps causes false positives
because the simulation finds that target can trivially reach itself through
any transitive dependency that also depends on source.

When source has no dependencies (e.g., runtime chunk), BFS from target's
deps with post-merge simulation to detect real cycles like rolldown#8361.

Closes rolldown#9049
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@IWANABETHATGUY IWANABETHATGUY force-pushed the fix/chunk-optimizer-precise-cycle-detection branch from 386bb45 to 71bfaae Compare April 15, 2026 09:04
IWANABETHATGUY and others added 3 commits April 15, 2026 17:10
…com:AlonMiz/rolldown into fix/chunk-optimizer-precise-cycle-detection
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review April 15, 2026 09:38
@IWANABETHATGUY IWANABETHATGUY enabled auto-merge (squash) April 15, 2026 11:01
@brandonroberts
Copy link
Copy Markdown

Thanks for looking into this @AlonMiz. We'll try to add some bundle size regression checks in the Analog repo to alert us about this. The Angular CLI has an option to use Rolldown for chunk optimization, so its worth them checking the impact there also.

cc: @alan-agius4

@alan-agius4
Copy link
Copy Markdown

Thanks @brandonroberts for ping. Let me loop in @clydin.

@shulaoda shulaoda disabled auto-merge April 16, 2026 08:00
@IWANABETHATGUY IWANABETHATGUY merged commit f4e60c9 into rolldown:main Apr 16, 2026
32 checks passed
@github-actions github-actions Bot mentioned this pull request Apr 22, 2026
This was referenced Apr 22, 2026
shulaoda added a commit that referenced this pull request Apr 22, 2026
## [1.0.0-rc.17] - 2026-04-22

### 🐛 Bug Fixes

- link: error on missing export between TS modules (#9197) by @IWANABETHATGUY
- rolldown_plugin_vite_import_glob: import path should not be affected by absolute base option (#9145) by @kermanx
- `this.resolve()` returns null for bare relative paths without importer (#9142) by @Copilot
- collect destructured bindings in HMR module exports (#9146) by @h-a-n-a
- esbuild-tests: handle 0.28.0 test cases (#9149) by @sapphi-red
- plugin/copy-module: honor external resolutions from other plugins (#9139) by @TheAlexLichter
- allow undefined in sourcesContent type (#9136) by @jurijzahn8019
- reduce false positives in chunk optimizer circular dependency detection (#9049) by @AlonMiz

### 🚜 Refactor

- chunk-optimizer: extract runtime-module placement into rehome_runtime_module (#9163) by @IWANABETHATGUY

### 📚 Documentation

- add design doc for sort_modules execution ordering (#9169) by @IWANABETHATGUY
- add document for `RenderedModule` (#9147) by @sapphi-red

### ⚡ Performance

- rolldown_plugin_vite_import_glob: skip self-import earlier using raw path comparison (#9193) by @shulaoda

### 🧪 Testing

- lazy: add `playground/lazy-compilation` (#7974) by @hyf0

### ⚙️ Miscellaneous Tasks

- use app token for release PR (#9198) by @Boshen
- upgrade oxc to 0.127.0 (#9194) by @Dunqing
- use oxc security action (#9196) by @Boshen
- esbuild-tests: remove some tests from ignored list as enum inline is now supported (#9184) by @sapphi-red
- deps: update dependency vite-plus to v0.1.19 (#9183) by @renovate[bot]
- use vp instead of pnpm in check-wasi-binding-deps (#9182) by @shulaoda
- verify wasm32-wasi binding deps match @rolldown/browser before publish (#9162) by @shulaoda
- deps: update esbuild for tests to 0.28.0 (#9172) by @sapphi-red
- deps: update rollup submodule for tests to v4.60.2 (#9173) by @sapphi-red
- deps: update test262 submodule for tests (#9174) by @sapphi-red
- sort_modules: fix stale async-entry sort key comment (#9170) by @IWANABETHATGUY
- deps: update npm packages (#9157) by @renovate[bot]
- deps: update dependency diff to v9 (#9158) by @renovate[bot]
- deps: update rust crates (#9156) by @renovate[bot]
- run Windows CI on PRs labeled with `ci: windows` (#9153) by @hyf0
- update-test-dependencies: run setup-rust before file changes (#9151) by @sapphi-red
- deps: update dependency rust to v1.95.0 (#9140) by @renovate[bot]

### ❤️ New Contributors

* @jurijzahn8019 made their first contribution in [#9136](#9136)
* @AlonMiz made their first contribution in [#9049](#9049)

---------

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
IWANABETHATGUY added a commit that referenced this pull request Apr 25, 2026
## Summary

Fixes a regression introduced by #9049: when the chunk optimizer merges
a shared module into an entry chunk, it can produce a static import
cycle between that entry chunk and a manual-split chunk that also
depends on the shared module.

## Problem

`would_create_circular_dependency` decides whether merging `source` (the
temp chunk holding shared modules) into `target` (an existing entry
chunk) is safe. The merge is unsafe iff, post-merge, `target` would
reach itself in the dependency graph.

#9049 split the algorithm into two cases based on whether `source` has
dependencies, in order to reduce false positives that were blocking
legitimate merges (the PR reported ~1,046 blocked merges and +12.5%
bundle size on a 2,200-entry application). However, the `source has
deps` branch only does a forward BFS from `source.deps` and drops the
post-merge alias step that the original algorithm used to model new
back-edges introduced by the merge:

```rust
// pre-#9049 step (removed in the source-has-deps branch)
if self.chunks[chunk_idx].dependencies.contains(&source_chunk_idx) {
  queue.push_back(target_chunk_idx);
}
```

Without that step, the BFS cannot see cycles where the closing edge is
one of the back-edges created by retargeting `source`'s importers onto
`target`.

## Concrete repro shape (issue #9225)

- `main.js` (entry) statically imports `api.js` and `env.js`, plus
dynamic imports `lazy.js`, `env-user.js`, `dep-user.js`.
- `api.js` is captured by a manual-split group `{ name: "api", test:
"api\\.js" }`.
- `api.js` statically imports `env.js`; `env-user.js` statically imports
`env.js`.
- `env.js` is a shared module not captured by any group
(`includeDependenciesRecursively: false`).

The optimizer selects `main` as the static dominator for `env.js`'s
consumers and merges `env.js` into it. After the merge:

- `api.js` and `env-user.js` rewrite their `./env.js` imports to
`./main.js`.
- `main.js` still statically imports `./api.js` (pre-existing edge).
- → static cycle `main.js ⇄ api.js` in the output.

In the `source has deps` branch, the BFS starts from `source.deps = {
dep.js's chunk }`. `dep.js` has no further deps and does not reach
`main`, so the BFS returns `false` and the merge proceeds. The dropped
alias step would have queued `target` when the BFS visited `api`'s chunk
(whose `dependencies` contain `source`), correctly detecting the cycle.

## Fix

Revert `would_create_circular_dependency` to the pre-#9049 unified
algorithm:

- BFS from `source.deps ∪ target.deps`.
- For each visited chunk `c`, when `source ∈ c.dependencies`, also
enqueue `target` (modeling the post-merge edge `c → target`).
- If `target` is dequeued, the merge would create a cycle.

This is sound: it catches both the forward path (`source.deps → target`)
and the back-edge path (`target → … → c → source-becomes-target`).

## Why reverting #9049 is safe: the `issues/9049` fixture is already
covered by #9085

The existing `crates/rolldown/tests/rolldown/issues/9049/` fixture
continues to pass after this revert, because **#9049's
`would_create_circular_dependency` change was never load-bearing for
that fixture**. Bisecting between rc.15 and rc.16 (rc.15 produces 4
chunks, rc.16 produces 3) pinpoints the actual fix as **#9085** — `fix:
relax overly conservative side-effect leak check in chunk optimizer`
(commit `5ff1a7265`). #9085 narrows the side-effect leak guard in
`try_insert_into_existing_chunk`: the merge is now blocked only when a
*different* user-defined entry can also reach the dynamic entries
depending on the common chunk, instead of unconditionally blocking on
any such dependency. That relaxation is what allows
`services/svc{0,1}.js` to merge into the `route0/route1` chunks for this
fixture.

The fixture's name (`issues/9049`) is therefore historically misleading:
it tests behavior fixed by #9085, not #9049. The false-positive
regressions #9049 was addressing in real-world bundles remain a concern
but are not exercised by any in-tree fixture; a follow-up can re-tighten
the check (e.g. by tracking dependent aliases more precisely) without
losing the regression coverage this PR adds.

## Regression test

`crates/rolldown/tests/rolldown/issues/9225/` reproduces the cycle that
#9049 was masking.

- `_config.json` sets up `manualCodeSplitting.groups = [{ name: "api",
test: "api\\.js" }]` with `includeDependenciesRecursively: false`.
- `_test.mjs` walks `dist/*.js`, parses the static-import edges, and
asserts the graph has no cycle. It additionally executes the bundled
entry, checks `globalThis.__rolldown_issue_7449_value === 300000`, and
verifies that the side-effect module `dep.js` runs exactly once even
when the dynamic chunks (which all transitively depend on `dep.js`)
load.

Without this fix, `_test.mjs` fails on the cycle assertion before any
runtime check runs.

## Testing

- `cargo test -p rolldown --test integration` — **1692 passed, 0 failed,
70 ignored**.
- `just test-node` (Rolldown vitest `test:main` + `test:watcher` +
Rollup compatibility suite) — **722 + 31 + 916 passed, 0 failed**.
- New `issues/9225` fixture passes both the cycle assertion and the
runtime checks.

Closes #9225.
`

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

6 participants