feat(linking): skip side-effect-free modules in per-entry reachability#10111
Merged
Merged
Conversation
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
IWANABETHATGUY
marked this pull request as ready for review
July 6, 2026 03:34
Merging this PR will not alter performance
Comparing Footnotes
|
hyfdev
approved these changes
Jul 6, 2026
Member
Author
Merge activity
|
#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
Bot
force-pushed
the
fix/8920-skip-side-effect-free-barrels
branch
from
July 7, 2026 03:42
258d455 to
c2d817c
Compare
Merged
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]>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
include_side_effectful_dependencies) follows a static import-record edge only when the importeeside_effects().has_side_effects(); used bindings pull in the module that canonically owns them (re-exports resolve through a barrel to the origin module), andcompute_cross_chunk_linksemits a bareimport "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 walksmeta.dependencies, the union of all static record targets plus the symbol-owner dependencies merged in bypatch_module_dependencies. Soentry-light → barrel(a pure record edge whose imported binding canonically lives inlight.js) still marked the barrel — and transitivelyheavy.js— as reachable from the light entry, producing a shared chunk that neither the inclusion semantics nor the emitted imports actually load:Fix
patch_module_dependenciesnow additionally recordsmeta.load_dependencies— the edges that actually force a module to be loaded at runtime:include_side_effectful_dependenciesrule; with tree-shaking disabled the set equalsdependencies, so nothing changes there).determine_reachable_modules_for_entrywalksload_dependencies. Every othermeta.dependenciesconsumer (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-dynamicin CI: 4 chunks instead of 3; mirrored here as thecode_splitting/static_import_of_code_less_dynamic_entryfixture).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 existingdebug_assertat 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 CJSnamespace_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/8920andcode_splitting/static_import_of_code_less_dynamic_entryfixtures, 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: thecommon.jschunk (reachable only through plain record edges — what this fixture is about) colocates with its one real consumer; the executednode:assertchecks still pass.esbuild/splitting/splitting_cross_chunk_assignment_dependencies_recursive: the pureset*helper chain colocates into thec.jsentry, letting the defaultdce-onlypass eliminate the provably dead chain entirely (itsa.js/b.jsoutputs 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).