fix: gate sideEffects:false modules' side effects on body demand#10080
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
ff2be16 to
1b22ce3
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a tree-shaking inconsistency where sideEffects: false ESM modules could have side-effectful, binding-reading statements resurrected by wrap triggers (strictExecutionOrder, require() of ESM, inlined dynamic import under codeSplitting: false). The fix gates those statements so they are included only when the module’s body is demanded (own-export or namespace demand), preventing emitted chunks from referencing bindings whose import records were correctly deferred/omitted (notably with experimental.lazyBarrel).
Changes:
- Introduces “on-demand” inclusion edges for side-effectful statements that reference module-level symbols in
UserDefined(false)ESM modules, keyed off own-export / namespace demand. - Replays the same inclusion semantics during chunk optimization to keep link-stage and generate-stage behavior aligned.
- Adds regression fixtures/snapshots covering the affected issue family (#9691, #9806, #9961, #9964, #10013, #10048) plus a dead pure dynamic-entry case.
Reviewed changes
Copilot reviewed 57 out of 61 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rs | Implements on-demand gating + inclusion edges for sideEffects:false ESM statements |
| crates/rolldown/src/stages/link_stage/mod.rs | Re-exports compute helper for reuse in generate stage |
| crates/rolldown/src/stages/generate_stage/chunk_optimizer.rs | Replays on-demand inclusion semantics during chunk optimization |
| crates/rolldown/tests/rolldown/tree_shaking/dead_dynamic_entry_side_effect_free/_config.json | New regression config for dead pure dynamic import + strictExecutionOrder |
| crates/rolldown/tests/rolldown/tree_shaking/dead_dynamic_entry_side_effect_free/artifacts.snap | Snapshot asserting leaf body stays dropped |
| crates/rolldown/tests/rolldown/tree_shaking/dead_dynamic_entry_side_effect_free/entry-a.js | Fixture entry with pure dynamic import + live re-export usage |
| crates/rolldown/tests/rolldown/tree_shaking/dead_dynamic_entry_side_effect_free/entry-b.js | Fixture entry importing only the re-exported value |
| crates/rolldown/tests/rolldown/tree_shaking/dead_dynamic_entry_side_effect_free/helper.js | Re-export source used by entries |
| crates/rolldown/tests/rolldown/tree_shaking/dead_dynamic_entry_side_effect_free/leaf.js | Side-effect-free leaf with external import + unused own export |
| crates/rolldown/tests/rolldown/tree_shaking/dead_dynamic_entry_side_effect_free/node_modules/dep/index.js | Local external dep that must remain unreachable |
| crates/rolldown/tests/rolldown/tree_shaking/dead_dynamic_entry_side_effect_free/node_modules/dep/package.json | Declares local dep as ESM export |
| crates/rolldown/tests/rolldown/issues/9964/_config.json | Regression config for CJS->ESM require wrap + export* barrel |
| crates/rolldown/tests/rolldown/issues/9964/artifacts.snap | Snapshot asserting unused star-source is pruned under wrap |
| crates/rolldown/tests/rolldown/issues/9964/entry.cjs | CJS entry requiring ESM consumer (wrap trigger) |
| crates/rolldown/tests/rolldown/issues/9964/lib/account.js | sideEffects:false star-source with derived initializers reading imports |
| crates/rolldown/tests/rolldown/issues/9964/lib/eg.js | Defines binding that previously got pruned while still referenced |
| crates/rolldown/tests/rolldown/issues/9964/lib/index.js | Barrel with export * plus unrelated local export |
| crates/rolldown/tests/rolldown/issues/9964/package.json | Marks fixture package as ESM + sideEffects:false |
| crates/rolldown/tests/rolldown/issues/9964/story.js | ESM consumer importing only unrelated util |
| crates/rolldown/tests/rolldown/issues/9961/_config.json | Regression config for strictExecutionOrder + sideEffects:false top-level call |
| crates/rolldown/tests/rolldown/issues/9961/artifacts.snap | Snapshot asserting the problematic top-level call is not emitted |
| crates/rolldown/tests/rolldown/issues/9961/core/checkGlobals.js | Imported function that was previously dropped while call remained |
| crates/rolldown/tests/rolldown/issues/9961/core/http.js | Used export to keep barrel reachable |
| crates/rolldown/tests/rolldown/issues/9961/core/index.js | Barrel calling imported binding at module top level |
| crates/rolldown/tests/rolldown/issues/9961/core/package.json | Declares sideEffects:false for fixture package |
| crates/rolldown/tests/rolldown/issues/9961/core/setupWorker.js | Used export to keep barrel reachable |
| crates/rolldown/tests/rolldown/issues/9961/main.js | App entry using only selected exports from barrel |
| crates/rolldown/tests/rolldown/issues/9961/package.json | Root fixture package.json |
| crates/rolldown/tests/rolldown/issues/9806/_config.json | Regression config for lazyBarrel + derived initializer in sideEffects:false barrel |
| crates/rolldown/tests/rolldown/issues/9806/artifacts.snap | Snapshot asserting bundle executes and unused derived reads are not kept |
| crates/rolldown/tests/rolldown/issues/9806/main.js | Entry importing only unrelated re-export to trigger pruning |
| crates/rolldown/tests/rolldown/issues/9806/pkg-barrel/foo.cjs | CJS submodule used only by derived initializer |
| crates/rolldown/tests/rolldown/issues/9806/pkg-barrel/index.mjs | sideEffects:false barrel with derived property read |
| crates/rolldown/tests/rolldown/issues/9806/pkg-barrel/other.cjs | Unrelated re-export that stays used |
| crates/rolldown/tests/rolldown/issues/9806/pkg-barrel/package.json | Declares sideEffects:false for barrel package |
| crates/rolldown/tests/rolldown/issues/9691/_config.json | Regression config for codeSplitting:false dynamic wrap propagation through export* |
| crates/rolldown/tests/rolldown/issues/9691/artifacts.snap | Snapshot asserting dead star-source is pruned to avoid dangling bindings |
| crates/rolldown/tests/rolldown/issues/9691/lib/dep-a.js | Used re-export path (external builtin import) |
| crates/rolldown/tests/rolldown/issues/9691/lib/dep-b.js | Deferred import path that must not be referenced |
| crates/rolldown/tests/rolldown/issues/9691/lib/handler-a.js | Used star-source providing the requested export |
| crates/rolldown/tests/rolldown/issues/9691/lib/handler-b.js | Dead star-source with top-level binding read (previously kept) |
| crates/rolldown/tests/rolldown/issues/9691/lib/index.js | Barrel with export * from used and unused sources |
| crates/rolldown/tests/rolldown/issues/9691/lib/package.json | Declares sideEffects:false for barrel package |
| crates/rolldown/tests/rolldown/issues/9691/main.js | Entry that triggers wrap via inlined dynamic import |
| crates/rolldown/tests/rolldown/issues/9691/trigger.js | Dynamic import target that pulls barrel local export |
| crates/rolldown/tests/rolldown/issues/10048/_config.json | Regression config for missing helper bindings under strictExecutionOrder |
| crates/rolldown/tests/rolldown/issues/10048/_test.mjs | Runtime test asserting built entry import does not throw |
| crates/rolldown/tests/rolldown/issues/10048/artifacts.snap | Snapshot asserting helper-only module is pruned safely |
| crates/rolldown/tests/rolldown/issues/10048/data.js | Helper-independent passthrough export (themes) |
| crates/rolldown/tests/rolldown/issues/10048/entry.js | Entry using only passthrough export but keeping module reachable |
| crates/rolldown/tests/rolldown/issues/10048/helpers.js | Helper-only module defining __commonJS / __toESM exports |
| crates/rolldown/tests/rolldown/issues/10048/package.json | Declares sideEffects:false for fixture package |
| crates/rolldown/tests/rolldown/issues/10048/theming.js | Module calling helpers at top-level while re-exporting passthrough |
| crates/rolldown/tests/rolldown/issues/10013/_config.json | Regression config for external bindings referenced without import under strictExecutionOrder |
| crates/rolldown/tests/rolldown/issues/10013/artifacts.snap | Snapshot asserting leaf body (and external import) is fully dropped |
| crates/rolldown/tests/rolldown/issues/10013/entry-a.js | Entry importing only re-exported value |
| crates/rolldown/tests/rolldown/issues/10013/entry-b.js | Second entry importing only re-exported value |
| crates/rolldown/tests/rolldown/issues/10013/helper.js | Re-export source used by entries |
| crates/rolldown/tests/rolldown/issues/10013/leaf.js | Leaf with unused own export referencing external bindings |
| crates/rolldown/tests/rolldown/issues/10013/node_modules/dep/index.js | Local external dep that must remain unreachable |
| crates/rolldown/tests/rolldown/issues/10013/node_modules/dep/package.json | Declares local dep as ESM export |
Merging this PR will not alter performance
Comparing Footnotes
|
How to use the Graphite Merge QueueAdd 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. |
hyfdev
left a comment
There was a problem hiding this comment.
I found some issues but I'm not sure if it's correct to fix them in this direction. The treeshking part becomes too complicated, it's very hard to manually reason if the behavior is correct.
However, tests are passed, it's possitive to prevent future regressions after merging them.
Merge activity
|
) ### What this PR solves Any wrap trigger — `strictExecutionOrder`, `require()` of ESM, or an inlined dynamic import under `codeSplitting: false` — force-includes a `sideEffects: false` ESM module through its wrapper ref, retaining binding-reading side-effect statements that plain (unwrapped) tree-shaking provably drops. With `experimental.lazyBarrel`, those retained statements reference import records the loader correctly never loaded (it defers records by requested exports), so the emitted chunk contains free identifiers and throws `ReferenceError` at load. The tree-shaking inconsistency between `strictExecutionOrder: true` and `false` is the shared root cause of the family: fixes #9691, fixes #9806, fixes #9961, fixes #9964, fixes #10013, fixes #10048 ### How A binding-reading side-effect statement of a user-declared side-effect-free ESM module is now included **iff the module's body is demanded**: one of its own (non-re-export) exports or its namespace object becomes used. This mirrors the lazy-barrel loader's `local`/`All` classification exactly — the loader loads *every* plain import record precisely in those cases — so a kept statement can never reference an unloaded record, and a deferred record is only ever referenced by dropped statements. The loader itself needs no changes. Behavior deliberately kept as-is (pinned by existing tests): - statements with no module-symbol references (a bare `console.log()`) and import/re-export statements (they drive wrapper `init_*` chains and side-effect imports) keep the unconditional sweep — `package_json_side_effects_false_keep_*` esbuild fixtures; - bodies of modules whose own export is demanded (#7597's top-level asserts) or whose namespace is observed (`import * as ns`, `require(esm)`) are retained unchanged; - user-defined entries, `eval` modules, and CommonJS modules are exempt. Dynamic entries are **not** exempt: a live dynamic entry's body joins through namespace/own-export demand, while a dead pure dynamic import must not resurrect a body its removal already models as an empty namespace (covered by the new `tree_shaking/dead_dynamic_entry_side_effect_free` fixture). ### Alternatives explored - Loader-side fix (force-load records referenced by retained statements): rejected — the deferral strategy is correct; the retention is the bug. - Blanket statement-level stripping for `sideEffects: false` modules: rejected — breaks the esbuild-parity `keep_*` fixtures (namespace/require demand must keep the body). - Statement-granular demand edges (keep a statement once any symbol it references is used): rejected — still dropped #7597's asserts. ### Notes for reviewers - Inclusion changes only for `UserDefined(false)` ESM modules reached exclusively through re-exports or wrapper refs; every pre-existing snapshot is byte-identical (full integration suite: 1785 passed, 0 failed). - Each `issues/*` fixture executes its output with node; `issues/10013` and the dynamic-entry fixture additionally carry a local `node_modules/dep` that throws on evaluation, so re-emitting the dropped external import fails execution, not just the snapshot. - Residual and deferred (benign): wrapped modules whose body was pruned still emit empty `init_x` shells; a follow-up no-op-init cleanup can drop them.
9897950 to
fdb678e
Compare
Do you mean this issue #10085? |
No. |
… binding but keeps its property reads) (#10109) ## What Adds a Rust integration-test fixture at `crates/rolldown/tests/rolldown/issues/10099/` closes #10099. ## Why #10099: importing only `dayjs` from `element-plus` emitted a bundle that threw `ReferenceError: defaults_default is not defined`. The `sideEffects:false` barrel (`index.mjs`) re-exports an **external** module's default (`dayjs`) as a passthrough while reading `defaults_default.install`/`.version` at the top level. Importing only that passthrough force-includes the barrel; under `experimental.lazyBarrel` the tree-shaker dropped the `defaults_default` import **and** its definition but kept the property reads → dangling reference. Fixed by #10080 (gate `sideEffects:false` modules' side effects on body demand). This PR adds the regression test for the specific element-plus shape, which none of #10080's fixtures cover: here the inclusion driver is an **external-default passthrough** (the sibling fixtures use local passthroughs or dynamic-import wrapping). ## The fixture - `_config.json`: `external:["extdep"]`, `experimental.lazyBarrel:true`, `treeshake.moduleSideEffects:false` - barrel `index.js` default-imports `defaults.js`, reads `.install`/`.version`, and re-exports the external default as `extdep` - entry `main.js` imports only `{ extdep }` and asserts it resolves - `node_modules/extdep/` stub so the external passthrough resolves at runtime `experimental.lazyBarrel: true` is load-bearing: the bug is lazyBarrel-gated and lazyBarrel is default-off since #10071, so without it the test would pass on both the base and the fix. ## Verification - **Green** on current `main` (fix present): the harness executes the compiled entry (`expectExecuted` default) and the snapshot is clean — no `defaults_default`. - **Red** on the pre-fix base: the exact fixture source + config through published `[email protected]` throws `ReferenceError: defaults_default is not defined`. Test-only; no source changes.
## [1.1.5] - 2026-07-08 ### 🚀 Features - detect top-level import-binding reads as execution-order sensitive (#10180) by @hyf0 - sourcemap_filenames: add a sourcemapFileNames option (#9271) by @V1OL3TF0X - binding: record plugin hook result kind in tracing spans (#10154) by @IWANABETHATGUY - linking: skip side-effect-free modules in per-entry reachability (#10111) by @IWANABETHATGUY - improve error message for unresolved virtual imports (#10156) by @sapphi-red - add descriptive metadata to plugin API (#10106) by @sapphi-red - add `--configLoader=native` option (#10118) by @sapphi-red ### 🐛 Bug Fixes - improve invalid annotation warnings (#10185) by @hyf0 - keep deduplicated asset filenames stable once they can be observed (#10191) by @shulaoda - sourcemap_filenames: use public option name in pattern errors (#10188) by @IWANABETHATGUY - sourcemap_filenames: hash prepared sourcemap content (#10178) by @hyf0 - tree-shake unused circular declarators exported via export list (#10166) by @IWANABETHATGUY - dev: don't panic when an HMR rebuild hits an unresolved import (#10162) by @shulaoda - propagate errors from output.globals function (#9880) by @shulaoda - dev: revert cache mutations when a partial scan fails (#10110) by @shulaoda - dev: update importer relationships of cached modules in incremental build (#10107) by @shulaoda - hmr: fall back to full reload when a changed module is not registered as executed (#10132) by @shulaoda - chunk-optimizer: follow entry facade edges in runtime placement cycle check (#10101) by @hyf0 - dev: ignore watcher events after close (#10113) by @hyf0 - emit async wrapper for TLA modules under onDemandWrapping (#10086) by @IWANABETHATGUY - gate sideEffects:false modules' side effects on body demand (#10080) by @IWANABETHATGUY - rolldown_plugin_vite_resolve: return empty object for `browser: false` mapped modules (#10082) by @sapphi-red - reset the word-boundary state on newline in Hires::Boundary sourcemaps (#10025) by @shulaoda - trim an emptied chunk's outro/intro instead of skipping past it (#10029) by @shulaoda - test each edited chunk's own start against indent exclude ranges (#10026) by @shulaoda - preserve sourcemap mappings for indented lines when a CJS module shares the chunk (#10074) by @hyf0 ### 🚜 Refactor - separate tree-shaking side effects from execution order sensitivity (#10168) by @hyf0 - type construct_vite_preload_call to take an ObjectPattern (#10135) by @shulaoda - treeshake: single-source the own-export classification shared with the lazy-barrel loader (#10098) by @IWANABETHATGUY - dev: reuse Vite's bundledDev server (#10081) by @h-a-n-a - clippy: ban std HashMap/HashSet in favour of FxHashMap/FxHashSet (#10108) by @Boshen - treeshake: make body demand a second module bit instead of a stmt multimap (#10097) by @IWANABETHATGUY - seal used_symbol_refs by construction after its last writer (#10091) by @hyf0 - treeshake: replace inclusion mutual recursion with a worklist engine (#10096) by @IWANABETHATGUY - treeshake: split include_statements.rs into focused modules (#10095) by @IWANABETHATGUY - drop redundant is_user_defined filter on partitioned entries (#10050) by @shulaoda - project the retained export interface out of used_symbol_refs (#10089) by @hyf0 - track used external symbols separately from used_symbol_refs (#10088) by @hyf0 - make module namespace inclusion an explicit linking metadata field (#10087) by @hyf0 - rename statement evaluation metadata (#10078) by @hyf0 ### 📚 Documentation - virtual modules user-facing id convention (#10155) by @sapphi-red - cli: clarify disabling boolean/object flags like codeSplitting (#10153) by @IWANABETHATGUY - chore: remove Vite+ alpha banner (#10105) by @mdong1909 - write down the used_symbol_refs contract (#10090) by @hyf0 - dev/lazy: update design and implementation (#10079) by @h-a-n-a ### ⚡ Performance - ast_scanner: stop order-sensitivity checks once a module is flagged (#10190) by @IWANABETHATGUY - return impl ExactSizeIterator from slice-backed accessors (#10133) by @Boshen - binding: box dev and watcher napi futures (#10103) by @Boshen ### 🧪 Testing - move string_wizard replace unit tests to the JS magic-string suite (#10176) by @IWANABETHATGUY - dev: assert incremental scan state matches a fresh full build after each HMR step (#10115) by @shulaoda - dev: restore runtime assertions of delete_file_not_used_anymore (#10112) by @shulaoda - dev: fix flaky dev server tests in CI (#10152) by @h-a-n-a - add regression test for #10099 (lazyBarrel drops default-import binding but keeps its property reads) (#10109) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - deploy website to Void via GitHub OIDC (#10192) by @Boshen - deps: update oxc to 0.139.0 (#10161) by @shulaoda - deps: update test262 submodule for tests (#10160) by @rolldown-guard[bot] - rolldown_plugin_utils: remove dead asset-url and css scaffolding (#10131) by @shulaoda - deps: revert vite-plus to v0.2.1 (#10148) by @shulaoda - deps: update github actions (#10141) by @renovate[bot] - deps: update dependency rust to v1.96.1 (#10145) by @renovate[bot] - deps: update npm packages (#10142) by @renovate[bot] - deps: update rust crates (#10143) by @renovate[bot] - deps: update napi to v3.10.3 (#10121) by @renovate[bot] - rolldown_utils: remove unused time module (#10138) by @shulaoda - remove dead CopyModulePlugin::is_active method (#10129) by @shulaoda - remove dead LazyCompilationContext::is_lazy_module method (#10128) by @shulaoda - remove dead BuildDiagnostic::downcast_ref method (#10127) by @shulaoda - deps: update dependency vite-plus to v0.2.2 (#10084) by @renovate[bot] - deps: update rust crate oxc_sourcemap to v8.1.0 (#10122) by @renovate[bot] - deps: update crate-ci/typos action to v1.48.0 (#10124) by @renovate[bot] - enable more clippy restriction lints (#10114) by @Boshen - deps: update rust dependencies (#10100) by @Boshen - deps: update oxc resolver to v11.23.0 (#10083) by @renovate[bot] ###◀️ Revert - Revert "chore(deps): revert vite-plus to v0.2.1" (#10157) by @h-a-n-a - "fix(hmr): fall back to full reload when a changed module is not registered as executed (#10132)" (#10151) by @shulaoda ### ❤️ New Contributors * @V1OL3TF0X made their first contribution in [#9271](#9271) Co-authored-by: shulaoda <[email protected]>
…10351) ### Description Fixes #10337. Since 1.1.5, bundling a dynamically imported `sideEffects: false` package (three-mesh-bvh in the report) together with a `codeSplitting` group that matches the package crashes with: ``` Symbol "common_functions" in ".../three-mesh-bvh/src/webgl/glsl/common_functions.glsl.js" should belong to a chunk ``` ### Root cause Regression from #10080. When a manual code-splitting group leaves a dynamic entry's facade chunk empty, `optimize_facade_entry_chunks` eliminates the facade and re-includes the entry's namespace object (`SymbolIncludeReason::SimulatedFacadeChunk`) so the namespace becomes an export of the merge-target chunk. That include went through `drain_body_demand_stmts`, so it counted as *body demand* for the entry module and resurrected its deferred (gated) side-effect statements. In the reproduction, three-mesh-bvh's backwards-compatibility statement ```js export const shaderIntersectFunction = ` ... ${BVHShaderGLSL.common_functions} ... `; ``` comes back to life, and its member expression canonicalizes through the `export *` chain straight to the glsl leaf module. The leaf therefore becomes included **after** `split_chunks` already assigned every included module to a chunk, so it belongs to no chunk and `compute_cross_chunk_links` panics. ### Fix A simulated-facade include only materializes the eliminated facade's namespace object as a chunk export; it is not a semantic observation of the module, so it must not create body demand. `handle_include_symbol` now skips `drain_body_demand_stmts` for `SimulatedFacadeChunk` includes — mirroring the existing `WorkItem::Module` gate on `is_simulated_facade_chunk` a few lines below. This is safe because the namespace statement's symbol getters were already narrowed to link-time-used symbols before the replay, so the include cannot legitimately need anything that wasn't included at link time. This also restores the invariant that chunk-layout choices don't change tree-shaking results: with the fix, the grouped build emits exactly what the ungrouped build emits (the dead `${common_functions}` statement no longer appears). ### Alternatives considered Instead of (or in addition to) the semantic gate, the generate stage could detect modules newly included by the replay and assign them to the merge-target chunk. That converts the panic into working output, but it ships code the ungrouped build proves dead, and it papers over inclusion growing after chunk assignment instead of preventing it — so the semantic gate alone is preferred. ### Test `crates/rolldown/tests/rolldown/issues/10337` mirrors the three-mesh-bvh shape: dynamic `import()` → an index module with `export * as` plus the deferred template-literal statement → an `export *` barrel → the glsl leaf, with `treeshake.moduleSideEffects: false` and a `codeSplitting` group. The entry only touches `res.MeshBVH`, so the leaf is reachable exclusively through the facade-elimination replay. The fixture panics on `main` and passes with this fix; it also executes the output under node and asserts the namespace member exists.
…10351) ### Description Fixes #10337. Since 1.1.5, bundling a dynamically imported `sideEffects: false` package (three-mesh-bvh in the report) together with a `codeSplitting` group that matches the package crashes with: ``` Symbol "common_functions" in ".../three-mesh-bvh/src/webgl/glsl/common_functions.glsl.js" should belong to a chunk ``` ### Root cause Regression from #10080. When a manual code-splitting group leaves a dynamic entry's facade chunk empty, `optimize_facade_entry_chunks` eliminates the facade and re-includes the entry's namespace object (`SymbolIncludeReason::SimulatedFacadeChunk`) so the namespace becomes an export of the merge-target chunk. That include went through `drain_body_demand_stmts`, so it counted as *body demand* for the entry module and resurrected its deferred (gated) side-effect statements. In the reproduction, three-mesh-bvh's backwards-compatibility statement ```js export const shaderIntersectFunction = ` ... ${BVHShaderGLSL.common_functions} ... `; ``` comes back to life, and its member expression canonicalizes through the `export *` chain straight to the glsl leaf module. The leaf therefore becomes included **after** `split_chunks` already assigned every included module to a chunk, so it belongs to no chunk and `compute_cross_chunk_links` panics. ### Fix A simulated-facade include only materializes the eliminated facade's namespace object as a chunk export; it is not a semantic observation of the module, so it must not create body demand. `handle_include_symbol` now skips `drain_body_demand_stmts` for `SimulatedFacadeChunk` includes — mirroring the existing `WorkItem::Module` gate on `is_simulated_facade_chunk` a few lines below. This is safe because the namespace statement's symbol getters were already narrowed to link-time-used symbols before the replay, so the include cannot legitimately need anything that wasn't included at link time. This also restores the invariant that chunk-layout choices don't change tree-shaking results: with the fix, the grouped build emits exactly what the ungrouped build emits (the dead `${common_functions}` statement no longer appears). ### Alternatives considered Instead of (or in addition to) the semantic gate, the generate stage could detect modules newly included by the replay and assign them to the merge-target chunk. That converts the panic into working output, but it ships code the ungrouped build proves dead, and it papers over inclusion growing after chunk assignment instead of preventing it — so the semantic gate alone is preferred. ### Test `crates/rolldown/tests/rolldown/issues/10337` mirrors the three-mesh-bvh shape: dynamic `import()` → an index module with `export * as` plus the deferred template-literal statement → an `export *` barrel → the glsl leaf, with `treeshake.moduleSideEffects: false` and a `codeSplitting` group. The entry only touches `res.MeshBVH`, so the leaf is reachable exclusively through the facade-elimination replay. The fixture panics on `main` and passes with this fix; it also executes the output under node and asserts the namespace member exists.
…10351) ### Description Fixes #10337. Since 1.1.5, bundling a dynamically imported `sideEffects: false` package (three-mesh-bvh in the report) together with a `codeSplitting` group that matches the package crashes with: ``` Symbol "common_functions" in ".../three-mesh-bvh/src/webgl/glsl/common_functions.glsl.js" should belong to a chunk ``` ### Root cause Regression from #10080. When a manual code-splitting group leaves a dynamic entry's facade chunk empty, `optimize_facade_entry_chunks` eliminates the facade and re-includes the entry's namespace object (`SymbolIncludeReason::SimulatedFacadeChunk`) so the namespace becomes an export of the merge-target chunk. That include went through `drain_body_demand_stmts`, so it counted as *body demand* for the entry module and resurrected its deferred (gated) side-effect statements. In the reproduction, three-mesh-bvh's backwards-compatibility statement ```js export const shaderIntersectFunction = ` ... ${BVHShaderGLSL.common_functions} ... `; ``` comes back to life, and its member expression canonicalizes through the `export *` chain straight to the glsl leaf module. The leaf therefore becomes included **after** `split_chunks` already assigned every included module to a chunk, so it belongs to no chunk and `compute_cross_chunk_links` panics. ### Fix A simulated-facade include only materializes the eliminated facade's namespace object as a chunk export; it is not a semantic observation of the module, so it must not create body demand. `handle_include_symbol` now skips `drain_body_demand_stmts` for `SimulatedFacadeChunk` includes — mirroring the existing `WorkItem::Module` gate on `is_simulated_facade_chunk` a few lines below. This is safe because the namespace statement's symbol getters were already narrowed to link-time-used symbols before the replay, so the include cannot legitimately need anything that wasn't included at link time. This also restores the invariant that chunk-layout choices don't change tree-shaking results: with the fix, the grouped build emits exactly what the ungrouped build emits (the dead `${common_functions}` statement no longer appears). ### Alternatives considered Instead of (or in addition to) the semantic gate, the generate stage could detect modules newly included by the replay and assign them to the merge-target chunk. That converts the panic into working output, but it ships code the ungrouped build proves dead, and it papers over inclusion growing after chunk assignment instead of preventing it — so the semantic gate alone is preferred. ### Test `crates/rolldown/tests/rolldown/issues/10337` mirrors the three-mesh-bvh shape: dynamic `import()` → an index module with `export * as` plus the deferred template-literal statement → an `export *` barrel → the glsl leaf, with `treeshake.moduleSideEffects: false` and a `codeSplitting` group. The entry only touches `res.MeshBVH`, so the leaf is reachable exclusively through the facade-elimination replay. The fixture panics on `main` and passes with this fix; it also executes the output under node and asserts the namespace member exists.

What this PR solves
Any wrap trigger —
strictExecutionOrder,require()of ESM, or an inlined dynamic import undercodeSplitting: false— force-includes asideEffects: falseESM module through its wrapper ref, retaining binding-reading side-effect statements that plain (unwrapped) tree-shaking provably drops. Withexperimental.lazyBarrel, those retained statements reference import records the loader correctly never loaded (it defers records by requested exports), so the emitted chunk contains free identifiers and throwsReferenceErrorat load. The tree-shaking inconsistency betweenstrictExecutionOrder: trueandfalseis the shared root cause of the family:fixes #9691, fixes #9806, fixes #9961, fixes #9964, fixes #10013, fixes #10048
How
A binding-reading side-effect statement of a user-declared side-effect-free ESM module is now included iff the module's body is demanded: one of its own (non-re-export) exports or its namespace object becomes used. This mirrors the lazy-barrel loader's
local/Allclassification exactly — the loader loads every plain import record precisely in those cases — so a kept statement can never reference an unloaded record, and a deferred record is only ever referenced by dropped statements. The loader itself needs no changes.Behavior deliberately kept as-is (pinned by existing tests):
console.log()) and import/re-export statements (they drive wrapperinit_*chains and side-effect imports) keep the unconditional sweep —package_json_side_effects_false_keep_*esbuild fixtures;import * as ns,require(esm)) are retained unchanged;evalmodules, and CommonJS modules are exempt.Dynamic entries are not exempt: a live dynamic entry's body joins through namespace/own-export demand, while a dead pure dynamic import must not resurrect a body its removal already models as an empty namespace (covered by the new
tree_shaking/dead_dynamic_entry_side_effect_freefixture).Alternatives explored
sideEffects: falsemodules: rejected — breaks the esbuild-paritykeep_*fixtures (namespace/require demand must keep the body).Notes for reviewers
UserDefined(false)ESM modules reached exclusively through re-exports or wrapper refs; every pre-existing snapshot is byte-identical (full integration suite: 1785 passed, 0 failed).issues/*fixture executes its output with node;issues/10013and the dynamic-entry fixture additionally carry a localnode_modules/depthat throws on evaluation, so re-emitting the dropped external import fails execution, not just the snapshot.init_xshells; a follow-up no-op-init cleanup can drop them.