fix(chunk-optimizer): refuse asymmetric merge for cyclic dynamic entries (#9320)#9322
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Pushed a change so that the export-pollution guard uses the actual linked export surface instead of the syntactical This should fix cyclic dynamic entries that export only via |
5e3da2c to
aed96e3
Compare
|
Found a test case that is still broken. I will tweak the code a little. The rest looks good to me. |
59d3d6c to
fa33773
Compare
|
@IWANABETHATGUY, is there anything I can help with to land this? |
fa33773 to
40584d9
Compare
…ies (rolldown#9320) When two or more dynamic-import entries form a static-import cycle, `find_merge_target` finds multiple candidates that satisfy its "every other entry imports me" rule. Picking one is arbitrary and merges the others' modules into the chosen target's chunk file — which is what `import('./<target>.js')` resolves to at runtime — so the merged-in entries' named exports leak into the dynamic-import namespace observed by callers. Add a guard after `find_merge_target` that detects this cyclic shape and refuses the merge when a non-target pending entry module has observable exports (using linked export metadata so re-export-only entries are treated the same as direct named exports). The fallback then creates a separate common chunk so each dynamic entry keeps a facade exposing only its own exports. Co-Authored-By: Alexander Lichter <[email protected]> Co-Authored-By: IWANABETHATGUY <[email protected]>
40584d9 to
a98e151
Compare
> [!IMPORTANT] > **This is a minor release.** Two changes alter default behavior compared to `1.0.3`. Please read this section before upgrading. Everything else is additive (new features, fixes, deps). ##⚠️ Notable behavior changes ### 1. `experimental.lazyBarrel` is now enabled by default (#9632) **What changed.** `experimental.lazyBarrel` now defaults to `true`. When a barrel module is recognized as side-effect-free, Rolldown skips compiling the re-exported modules that are never actually used. **Impact.** For codebases with large barrel files (component libraries such as Ant Design, `@mui/icons-material`, etc.) this is a meaningful build-time speedup, and for the vast majority of projects the emitted output is unchanged. In rare cases where a barrel is *incorrectly* treated as side-effect-free, the optimization could drop a module that was being relied on for its side effects. **How to opt out (backward compatible).** ```js // rolldown.config.js export default { experimental: { lazyBarrel: false }, } ``` > Note: this opt-out flag is planned to be removed in a future release. If you have a case where you must turn it off, please open an issue so we can fix the underlying detection instead. --- ### 2. `tsconfig` project-reference resolution now aligns with TypeScript Upgrading `oxc_resolver` (`11.19.1` → `11.20.0` in #9549, then `→ 11.21.0` in #9634) changes how a *solution-style* `tsconfig.json` (one that only lists `references` and delegates the real settings to `tsconfig.app.json` / `tsconfig.node.json`, as Vite scaffolds) is resolved, bringing it **in line with how TypeScript (`tsc`) itself behaves**: - **Reference match priority** (oxc-resolver [#1151](oxc-project/oxc-resolver#1151)): when the root has `references`, a referenced project that includes the file now **takes precedence over the root**, instead of the root matching it first (this is what TypeScript already does). So that project's `compilerOptions.paths` now apply. - **`allowJs`** (oxc-resolver [#1198](oxc-project/oxc-resolver#1198)): whether a `.js`/`.jsx`/`.mjs`/`.cjs` file is included is now decided by **each referenced project's own** `allowJs`, not the root's (again matching TypeScript). So `tsconfig.app.json` with `allowJs: true` + `paths` now resolves aliases for `.js` files even when the root doesn't set `allowJs`. For most projects this is a fix (the standard Vite `paths` aliases now resolve, closes #8468), but it **is** a behavior change if you relied on the previous behavior, where the root's `paths` / `allowJs` took precedence. **If you relied on the old "root wins" behavior.** There is no exact toggle back, because the old behavior was the bug being fixed. The recommended path is to align your config with TypeScript: declare the `paths` / `allowJs` on the referenced project that actually owns the files. If you must keep the old precedence while still using `references`: a referenced project's match wins, and **the first matching `references` entry takes priority** (the root is only a fallback when no reference claims the file). So extract the old root settings into their own config and list it **first**: ```jsonc // tsconfig.json (solution root) { "files": [], "references": [ { "path": "./tsconfig.base.json" }, // old root paths/allowJs — listed first, so it wins { "path": "./tsconfig.app.json" }, { "path": "./tsconfig.node.json" } ] } ``` `tsconfig.base.json` should carry the `paths` you previously declared on the root, plus `allowJs: true` if it needs to claim `.js` files (the extension is checked against each config's own `allowJs`). With no `include`, it defaults to `**/*` under its directory and claims every file first. Alternatively, bypass reference resolution entirely by pointing the top-level `tsconfig` option at a single config: `export default { tsconfig: './tsconfig.app.json' }`. --- ## [1.1.0] - 2026-06-03 ### 🚀 Features - enable `experimental.lazyBarrel` by default (#9632) by @shulaoda - `import.meta.glob` support `caseSensitive` option (#9594) by @btea - add `SOURCEMAP_BROKEN` warning for renderChunk hook (#9601) by @sapphi-red - add `SOURCEMAP_BROKEN` warning for transform hook (#9600) by @sapphi-red - add `@__NO_SIDE_EFFECTS__` hint for invalid `@__PURE__` before function declarations (#9505) by @Copilot - code-splitting: support group-local `includeDependenciesRecursively` (#9587) by @hyf0 ### 🐛 Bug Fixes - report TSCONFIG_ERROR instead of UNHANDLEABLE_ERROR for a missing tsconfig file (#9633) by @shulaoda - browser: add missing exports and ensure consistency with `rolldown` package (#9629) by @sapphi-red - should build test-dev-server when test-node (#9610) by @situ2001 - chunk-optimizer: refuse asymmetric merge for cyclic dynamic entries (#9320) (#9322) by @aminpaks - dev: handle the remaining errors in dev (#9570) by @h-a-n-a - handle slash-normalized ids with preserveModulesRoot (#9595) by @IWANABETHATGUY - json: preserve .default access on JSON default imports (#9568) by @IWANABETHATGUY - testing: remove unintended trigger_full_build from test harness (#9573) by @hyf0 ### 🚜 Refactor - js-regex: use regress native replace/replace_all (#9607) by @IWANABETHATGUY - remove never-constructed `ImportStatus` variants (#9606) by @Boshen ### 📚 Documentation - clarify that `RolldownBuild::close` method should be called in most cases (#9619) by @sapphi-red ### ⚡ Performance - avoid unnecessary intermediate sourcemaps (#9599) by @sapphi-red ### 🧪 Testing - add unit test for collapsing module sourcemap (#9626) by @sapphi-red - cover vite-alias regex capture-group expansion (#9602) (#9608) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - deps: update oxc_resolver to 11.21.0 (#9634) by @shulaoda - update invalid option diagnostic link to point to Rolldown docs (#9631) by @sapphi-red - deps: update vite+ to v0.1.24 (#9628) by @renovate[bot] - deps: update oxc resolver to v11.20.0 (#9549) by @renovate[bot] - deps: update dependency vite-plus to v0.1.24 (#9470) by @renovate[bot] - deps: update npm packages (#9614) by @renovate[bot] - deps: upgrade oxc to 0.134.0 (#9625) by @shulaoda - deps: update crate-ci/typos action to v1.47.0 (#9620) by @renovate[bot] - deps: update rollup submodule for tests to v4.61.0 (#9623) by @rolldown-guard[bot] - deps: update github actions (#9613) by @renovate[bot] - deps: update pnpm to v11.4.0 (#9616) by @renovate[bot] - deps: update rust crates (#9615) by @renovate[bot] - deps: update test262 submodule for tests (#9624) by @rolldown-guard[bot] - deps: update dependency @napi-rs/cli to v3.7.0 (#9588) by @renovate[bot] - deps: update dependency rust to v1.96.0 (#9596) by @renovate[bot] - re-enable WASI testing with proper infrastructure (#9397) by @Boshen ### ❤️ New Contributors * @aminpaks made their first contribution in [#9322](#9322) Co-authored-by: shulaoda <[email protected]>
Summary
Fixes #9320. Two dynamic-import targets with a static-import cycle leak each other's named exports into their dynamic-import namespaces —
Object.keys(actionNs)returned['actionImpl', 'callFormFromAction', 'n', 't']instead of['actionImpl', 'callFormFromAction'].Root cause: when both modules form a cycle,
find_merge_target(inchunk_optimizer.rs) finds multiple candidates that satisfy its "every other entry imports me" rule. Picking one is arbitrary and merges the other entry's module into the chosen target's chunk file — which is whatimport('./<target>.js')resolves to at runtime — so the merged-in entry's named exports become observable on the wrong namespace.find_merge_targetthat detects the cyclic shape:>=2of the candidate dynamic-entry chunks' own entry modules with named exports sit in the pending common-chunk'sinfo.modules. When that holds, refuse the merge and letassign_modules_to_chunkfall back to a separate common chunk so each dynamic entry stays a facade exposing only its own exports.HashSetallocation (dynamic_chunk_entry_modulesis a smallVec, linearcontainsis faster).find_dynamic_dominator(covering the disjoint shared-module case) is unchanged.crates/rolldown/tests/rolldown/issues/9320/with_test.mjsasserting both namespaces'Object.keys.No other snapshots changed. Reverting the fix makes the new test fail with the exact bug output from the issue.