Skip to content

feat(linking): skip side-effect-free modules in per-entry reachability#10111

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/8920-skip-side-effect-free-barrels
Jul 7, 2026
Merged

feat(linking): skip side-effect-free modules in per-entry reachability#10111
graphite-app[bot] merged 1 commit into
mainfrom
fix/8920-skip-side-effect-free-barrels

Conversation

@IWANABETHATGUY

@IWANABETHATGUY IWANABETHATGUY commented Jul 3, 2026

Copy link
Copy Markdown
Member

Description

Closes #8920. Minimal alternative to #9155 — no per-entry inclusion tracking; only the code-splitting reachability edges change.

Root cause

Rolldown already agrees in two places on what forces a module to be loaded:

  • tree-shaking (include_side_effectful_dependencies) follows a static import-record edge only when the importee side_effects().has_side_effects(); used bindings pull in the module that canonically owns them (re-exports resolve through a barrel to the origin module), and
  • compute_cross_chunk_links emits a bare import "chunk.js" for a record edge only when the importee has side effects; all other cross-chunk imports are symbol-driven.

The code-splitting BFS (determine_reachable_modules_for_entry) was the one place without that filter: it walks meta.dependencies, the union of all static record targets plus the symbol-owner dependencies merged in by patch_module_dependencies. So entry-light → barrel (a pure record edge whose imported binding canonically lives in light.js) still marked the barrel — and transitively heavy.js — as reachable from the light entry, producing a shared chunk that neither the inclusion semantics nor the emitted imports actually load:

# before: 3 chunks, entry-light loads HeavyService through the shared chunk
barrel.js   (heavy.js + light.js + barrel.js, bits {light,heavy})
entry-heavy.js
entry-light.js

# after: 2 chunks, same as Rollup
entry-heavy.js (heavy.js + barrel.js + entry-heavy.js)
entry-light.js (light.js + entry-light.js)

Fix

patch_module_dependencies now additionally records meta.load_dependencies — the edges that actually force a module to be loaded at runtime:

  • modules owning the canonical symbols referenced by this module's included statements (incl. CJS namespace-alias owners, runtime-when-helpers, entry-chunk export owners), plus
  • import-record targets whose evaluation has side effects (the exact include_side_effectful_dependencies rule; with tree-shaking disabled the set equals dependencies, so nothing changes there).

determine_reachable_modules_for_entry walks load_dependencies. Every other meta.dependencies consumer (chunk optimizer, manual code splitting, on-demand wrapping, dynamic-already-loaded) keeps the broad set, so their behavior is untouched.

One deliberate exception: record edges into entry modules are never pruned. An entry's chunk exists regardless, and letting static importers participate in its bit pattern keeps shared code co-located with the entry chunk — Rollup reaches the same topology by collapsing code-less entry facades onto the chunk holding their exports. Without this, a statically imported re-export barrel that is also a dynamic entry pushes its re-export targets into a separate shared chunk and leaves the dynamic chunk an empty facade (caught by rollup's chunking-form/entry-without-code-dynamic in CI: 4 chunks instead of 3; mirrored here as the code_splitting/static_import_of_code_less_dynamic_entry fixture).

Every included module stays reachable: a module only becomes included via a side-effect-full record edge or via a used symbol, and both edge kinds are present in load_dependencies (the existing debug_assert at the chunk-assignment site checks exactly this invariant across the whole suite). The suite surfaced one previously masked asymmetry — an entry export resolving to a facade binding with a CJS namespace_alias (esbuild/importstar/export_other_nested_common_js) contributed no dependency edge from the entry-chunk loop, unlike the statement walk; the entry-chunk loop now mirrors the same namespace-alias handling.

Snapshot changes (all audited)

Beyond the new issues/8920 and code_splitting/static_import_of_code_less_dynamic_entry fixtures, 5 existing snapshots change, all in the same direction — a module that an entry never actually loads is no longer assigned to that entry's chunk group:

  • issues/rolldown_vite_593, chunk_merging/transitive_dynamic_dep_should_not_merge_into_side_effectful_entry, tree_shaking/no_side_effects_namespace_cross_chunk_cases: single-consumer modules move out of shared chunks into their only consumer; shared chunks keep only genuinely shared code.
  • strict_execution_order/strip_plain_chunk_imports: the common.js chunk (reachable only through plain record edges — what this fixture is about) colocates with its one real consumer; the executed node:assert checks still pass.
  • esbuild/splitting/splitting_cross_chunk_assignment_dependencies_recursive: the pure set* helper chain colocates into the c.js entry, letting the default dce-only pass eliminate the provably dead chain entirely (its a.js/b.js outputs were already empty on main).

Fixtures involving record edges into dynamic entry modules (issue_8809, dynamic_entry_shared_module_not_merged, 9320_star) keep today's output byte-for-byte thanks to the entry-edge exception above; slimming those dynamic-entry namespaces/facades is the existing proxy-chunk follow-up topic, not this PR.

Fixtures that execute their output all still pass; full integration suite is green (1756 passed, hmr/watcher skipped locally on Windows).

@netlify

netlify Bot commented Jul 3, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs ready!

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

QR Code

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

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

@IWANABETHATGUY IWANABETHATGUY changed the title feat(code-splitting): skip side-effect-free modules in per-entry reachability feat(linking): skip side-effect-free modules in per-entry reachability Jul 6, 2026
@IWANABETHATGUY
IWANABETHATGUY marked this pull request as ready for review July 6, 2026 03:34
@codspeed-hq

codspeed-hq Bot commented Jul 6, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/8920-skip-side-effect-free-barrels (258d455) with main (2d9f81c)

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.

IWANABETHATGUY commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jul 7, 3:41 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 7, 3:41 AM UTC: IWANABETHATGUY added this pull request to the Graphite merge queue.
  • Jul 7, 3:46 AM UTC: Merged by the Graphite merge queue.

#10111)

### Description

Closes #8920. Minimal alternative to #9155 — no per-entry inclusion tracking; only the code-splitting reachability edges change.

#### Root cause

Rolldown already agrees in two places on what forces a module to be **loaded**:

- tree-shaking (`include_side_effectful_dependencies`) follows a static import-record edge only when the importee `side_effects().has_side_effects()`; used bindings pull in the module that canonically owns them (re-exports resolve *through* a barrel to the origin module), and
- `compute_cross_chunk_links` emits a bare `import "chunk.js"` for a record edge only when the importee has side effects; all other cross-chunk imports are symbol-driven.

The code-splitting BFS (`determine_reachable_modules_for_entry`) was the one place without that filter: it walks `meta.dependencies`, the union of **all** static record targets plus the symbol-owner dependencies merged in by `patch_module_dependencies`. So `entry-light → barrel` (a pure record edge whose imported binding canonically lives in `light.js`) still marked the barrel — and transitively `heavy.js` — as reachable from the light entry, producing a shared chunk that neither the inclusion semantics nor the emitted imports actually load:

```
# before: 3 chunks, entry-light loads HeavyService through the shared chunk
barrel.js   (heavy.js + light.js + barrel.js, bits {light,heavy})
entry-heavy.js
entry-light.js

# after: 2 chunks, same as Rollup
entry-heavy.js (heavy.js + barrel.js + entry-heavy.js)
entry-light.js (light.js + entry-light.js)
```

#### Fix

`patch_module_dependencies` now additionally records `meta.load_dependencies` — the edges that actually force a module to be loaded at runtime:

- modules owning the canonical symbols referenced by this module's included statements (incl. CJS namespace-alias owners, runtime-when-helpers, entry-chunk export owners), plus
- import-record targets whose evaluation has side effects (the exact `include_side_effectful_dependencies` rule; with tree-shaking disabled the set equals `dependencies`, so nothing changes there).

`determine_reachable_modules_for_entry` walks `load_dependencies`. Every other `meta.dependencies` consumer (chunk optimizer, manual code splitting, on-demand wrapping, dynamic-already-loaded) keeps the broad set, so their behavior is untouched.

One deliberate exception: record edges **into entry modules** are never pruned. An entry's chunk exists regardless, and letting static importers participate in its bit pattern keeps shared code co-located with the entry chunk — Rollup reaches the same topology by collapsing code-less entry facades onto the chunk holding their exports. Without this, a statically imported re-export barrel that is also a dynamic entry pushes its re-export targets into a separate shared chunk and leaves the dynamic chunk an empty facade (caught by rollup's `chunking-form/entry-without-code-dynamic` in CI: 4 chunks instead of 3; mirrored here as the `code_splitting/static_import_of_code_less_dynamic_entry` fixture).

Every included module stays reachable: a module only becomes included via a side-effect-full record edge or via a used symbol, and both edge kinds are present in `load_dependencies` (the existing `debug_assert` at the chunk-assignment site checks exactly this invariant across the whole suite). The suite surfaced one previously masked asymmetry — an entry export resolving to a facade binding with a CJS `namespace_alias` (`esbuild/importstar/export_other_nested_common_js`) contributed no dependency edge from the entry-chunk loop, unlike the statement walk; the entry-chunk loop now mirrors the same namespace-alias handling.

#### Snapshot changes (all audited)

Beyond the new `issues/8920` and `code_splitting/static_import_of_code_less_dynamic_entry` fixtures, 5 existing snapshots change, all in the same direction — a module that an entry never actually loads is no longer assigned to that entry's chunk group:

- `issues/rolldown_vite_593`, `chunk_merging/transitive_dynamic_dep_should_not_merge_into_side_effectful_entry`, `tree_shaking/no_side_effects_namespace_cross_chunk_cases`: single-consumer modules move out of shared chunks into their only consumer; shared chunks keep only genuinely shared code.
- `strict_execution_order/strip_plain_chunk_imports`: the `common.js` chunk (reachable only through plain record edges — what this fixture is about) colocates with its one real consumer; the executed `node:assert` checks still pass.
- `esbuild/splitting/splitting_cross_chunk_assignment_dependencies_recursive`: the pure `set*` helper chain colocates into the `c.js` entry, letting the default `dce-only` pass eliminate the provably dead chain entirely (its `a.js`/`b.js` outputs were already empty on main).

Fixtures involving record edges into dynamic entry modules (`issue_8809`, `dynamic_entry_shared_module_not_merged`, `9320_star`) keep today's output byte-for-byte thanks to the entry-edge exception above; slimming those dynamic-entry namespaces/facades is the existing proxy-chunk follow-up topic, not this PR.

Fixtures that execute their output all still pass; full integration suite is green (1756 passed, hmr/watcher skipped locally on Windows).
@graphite-app
graphite-app Bot force-pushed the fix/8920-skip-side-effect-free-barrels branch from 258d455 to c2d817c Compare July 7, 2026 03:42
@graphite-app
graphite-app Bot merged commit c2d817c into main Jul 7, 2026
34 checks passed
@graphite-app
graphite-app Bot deleted the fix/8920-skip-side-effect-free-barrels branch July 7, 2026 03:46
@rolldown-guard rolldown-guard Bot mentioned this pull request Jul 8, 2026
shulaoda added a commit that referenced this pull request Jul 8, 2026
## [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]>
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.

[Feature Request]: skip barrel files when all imports in the barrel files are marked as side effect free

2 participants