Skip to content

feat: skip barrel files when all imports in the barrel files are marked as side effect free#9155

Closed
IWANABETHATGUY wants to merge 6 commits into
04-14-refactor_add_entry_idx_for_entrypointfrom
04-16-fix_8920-2
Closed

feat: skip barrel files when all imports in the barrel files are marked as side effect free#9155
IWANABETHATGUY wants to merge 6 commits into
04-14-refactor_add_entry_idx_for_entrypointfrom
04-16-fix_8920-2

Conversation

@IWANABETHATGUY

@IWANABETHATGUY IWANABETHATGUY commented Apr 19, 2026

Copy link
Copy Markdown
Member

Closed #8920

IWANABETHATGUY commented Apr 19, 2026

Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add 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.

@IWANABETHATGUY IWANABETHATGUY changed the title uu fix: 8920 Apr 19, 2026
@IWANABETHATGUY
IWANABETHATGUY marked this pull request as ready for review April 19, 2026 13:49
@IWANABETHATGUY IWANABETHATGUY changed the title fix: 8920 test-perf: 8920 Apr 19, 2026
@codspeed-hq

codspeed-hq Bot commented Apr 19, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing 04-16-fix_8920-2 (52bdc7e) with main (f554244)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 04-14-refactor_add_entry_idx_for_entrypoint (fd82afa) during the generation of this report, so main (f554244) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@IWANABETHATGUY
IWANABETHATGUY marked this pull request as draft April 19, 2026 13:57
@IWANABETHATGUY IWANABETHATGUY changed the title test-perf: 8920 feat: skip barrel files when all imports in the barrel files are marked as side effect free Apr 20, 2026
graphite-app Bot pushed a commit that referenced this pull request Jul 7, 2026
#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).
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

1 participant