fix(code-splitting): deconflict force-included runtime helper statements#10336
Merged
Merged
Conversation
hyfdev
force-pushed
the
codex/facade-gate-post-lowering-edges
branch
from
July 17, 2026 07:59
0688d2e to
1f73c9f
Compare
hyfdev
force-pushed
the
codex/deconflict-forced-runtime-stmts
branch
from
July 17, 2026 08:00
2bbe119 to
9734a3f
Compare
hyfdev
force-pushed
the
codex/facade-gate-post-lowering-edges
branch
from
July 17, 2026 08:18
1f73c9f to
4206c33
Compare
hyfdev
force-pushed
the
codex/deconflict-forced-runtime-stmts
branch
from
July 17, 2026 08:18
9734a3f to
d57826c
Compare
This was referenced Jul 17, 2026
hyfdev
force-pushed
the
codex/facade-gate-post-lowering-edges
branch
from
July 18, 2026 05:35
4206c33 to
0f463fb
Compare
hyfdev
force-pushed
the
codex/deconflict-forced-runtime-stmts
branch
from
July 18, 2026 05:35
d57826c to
2ea8341
Compare
hyfdev
force-pushed
the
codex/facade-gate-post-lowering-edges
branch
from
July 18, 2026 05:48
0f463fb to
ea9b969
Compare
hyfdev
force-pushed
the
codex/deconflict-forced-runtime-stmts
branch
2 times, most recently
from
July 20, 2026 12:37
d62f740 to
ba3ba38
Compare
hyfdev
changed the base branch from
codex/facade-gate-post-lowering-edges
to
main
July 20, 2026 12:37
hyfdev
marked this pull request as ready for review
July 20, 2026 13:01
Merging this PR will not alter performance
Comparing Footnotes
|
IWANABETHATGUY
approved these changes
Jul 20, 2026
Member
Author
Merge activity
|
…nts (#10336) ## Summary Chunk deconfliction filtered each module's statements on raw `stmt_info_included`, so a runtime helper statement that tree-shaking excluded but strict order lowering force-includes (e.g. `__esmMin`) was rendered and symbol-assigned yet never registered with the renamer. When a user module co-hosted with the runtime declares a top-level binding of the same name, the two share the chunk's root scope: the user's initializer overwrites the runtime helper and the next wrapper's `__esmMin(...)` call throws. This ORs the order overlay into deconfliction's inclusion test, matching the two other consumers. ## Verified mechanism - `compute_cross_chunk_links.rs:438` and `module_finalizers/mod.rs:1623` both OR `order_wrap_state.forces_runtime_stmt(...)` into their inclusion test; `deconflict_chunk_symbols.rs:137` filtered on `meta.stmt_info_included.has_bit(...)` alone. - A single-chunk strict build (`codeSplitting: false`) places the runtime next to user code (`order_wrapping.rs:451-457`). Pure ESM demands no helper at link time, so tree-shaking drops `__esmMin` and strict order lowering force-includes it — the exact statement the deconflict filter dropped. ## Fix `deconflict_chunk_symbols` already receives `order_wrap_state` and `link_output`, so the change is a one-line mirror of the existing two sites: `has_bit(idx) || forces_runtime_stmt(&link_output.runtime, module.idx, stmt_info)` (`forces_runtime_stmt` self-guards `module_idx == runtime.id()`, so it is inert for every non-runtime module). **Item 7 (maintainability).** I kept the inline `||` rather than extracting a shared "is this statement rendered?" accessor: `forces_runtime_stmt` is already the shared half, the other two sites use this exact inline form, and a shared method would have to take `meta` + `stmt_idx` + `stmt_info` (coupling `OrderWrapState` to `LinkingMetadata`) and rewrite two already-correct sites — a larger diff than the reviewer's "keep the diff small" guidance wants for a fix. The three sites now read identically. ## Behavior / snapshot impact - **Full suite: zero existing snapshots change.** No current fixture co-hosts a user binding named after a force-included runtime helper, so the added disjunct fires nowhere else. - **One new fixture** `function/experimental/strict_execution_order/deconflict_forced_runtime_helper` (strict, `codeSplitting: false`): `helper.js` declares a top-level `__esmMin` (hoisted to a root-scope `var __esmMin` by its order wrapper) co-hosted with the runtime; `late.js` is ordered after it. - **Broken output verified on the unfixed base:** two `var __esmMin` at root — the runtime helper and helper.js's binding. `helper`'s init reassigns the shared binding to the user string, and `init_late = __esmMin(...)` then throws `TypeError: __esmMin is not a function` (the fixture fails to execute). - **Fixed:** deconflict registers the runtime statement, so the renamer renames the runtime helper to `__esmMin$1`; every wrapper calls `__esmMin$1(...)` and the user's `__esmMin` keeps its name. Snapshot pins the rename; `_test.mjs` asserts the executed result `{ helper: 'H:USERVAL', late: 'LATE:z' }`. **Note on wrap-all vs on-demand, and the co-hosting mechanism.** The reproduction is a wrap-all strict build (`strictExecutionOrder` without `onDemandWrapping`) because it deterministically co-hosts the runtime with user code in one chunk. The collision needs the runtime co-hosted with a user module *at deconflict time*. `try_merge_runtime_chunk` can co-host the standalone runtime into a user consumer chunk even in multi-chunk builds, but under strict order wrapping `ensure_runtime_module_for_order_wraps` (`order_wrapping.rs:395-457`) relocates a co-hosted runtime (`modules.len() > 1`) back to a standalone chunk unless code splitting is disabled — so at deconflict time the runtime sits next to user code only in single-chunk (`codeSplitting: false`) builds. (This stack's own multi-chunk strict on-demand fixtures, e.g. `facade_gate_emergent_edge`, confirm it — they emit a standalone `rolldown-runtime.js` chunk.) In a single chunk on-demand finds no cross-chunk execution-order hazard, so it wraps nothing and demands no `__esmMin`; wrap-all is the frame that both co-hosts the runtime and force-includes the helper. The fix itself is mode-independent: `forces_runtime_stmt` keys on `required_runtime_helpers()`, covering on-demand and wrap-all and both the `__esmMin` and `__esm` (profiler_names) variants. ## Validation - `cargo test -p rolldown` — 1872 passed (+1 new fixture, executed under node); only the 5 known environmental failures (`cjs_module_lexer_compat` ×3 + `npm_packages/util_deprecate` need `pnpm install`; `test262_module_code` needs the submodule). No existing snapshot changed (the local `symbols_ns2` drift is pre-existing `node_modules`-resolution noise, reproducible with this change reverted, and is not committed). - `cargo fmt --all --check` — clean; `cargo clippy -p rolldown --all-targets -- -D warnings` — clean. --- Follow-up to #10104; addresses item 2 (and the item-7 maintainability note) of #10104 (comment)
graphite-app
Bot
force-pushed
the
codex/deconflict-forced-runtime-stmts
branch
from
July 20, 2026 13:46
ba3ba38 to
ae2ec2e
Compare
✅ Deploy Preview for rolldown-rs canceled.
|
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.
Summary
Chunk deconfliction filtered each module's statements on raw
stmt_info_included, so a runtime helper statement that tree-shaking excluded but strict order lowering force-includes (e.g.__esmMin) was rendered and symbol-assigned yet never registered with the renamer. When a user module co-hosted with the runtime declares a top-level binding of the same name, the two share the chunk's root scope: the user's initializer overwrites the runtime helper and the next wrapper's__esmMin(...)call throws. This ORs the order overlay into deconfliction's inclusion test, matching the two other consumers.Verified mechanism
compute_cross_chunk_links.rs:438andmodule_finalizers/mod.rs:1623both ORorder_wrap_state.forces_runtime_stmt(...)into their inclusion test;deconflict_chunk_symbols.rs:137filtered onmeta.stmt_info_included.has_bit(...)alone.codeSplitting: false) places the runtime next to user code (order_wrapping.rs:451-457). Pure ESM demands no helper at link time, so tree-shaking drops__esmMinand strict order lowering force-includes it — the exact statement the deconflict filter dropped.Fix
deconflict_chunk_symbolsalready receivesorder_wrap_stateandlink_output, so the change is a one-line mirror of the existing two sites:has_bit(idx) || forces_runtime_stmt(&link_output.runtime, module.idx, stmt_info)(forces_runtime_stmtself-guardsmodule_idx == runtime.id(), so it is inert for every non-runtime module).Item 7 (maintainability). I kept the inline
||rather than extracting a shared "is this statement rendered?" accessor:forces_runtime_stmtis already the shared half, the other two sites use this exact inline form, and a shared method would have to takemeta+stmt_idx+stmt_info(couplingOrderWrapStatetoLinkingMetadata) and rewrite two already-correct sites — a larger diff than the reviewer's "keep the diff small" guidance wants for a fix. The three sites now read identically.Behavior / snapshot impact
function/experimental/strict_execution_order/deconflict_forced_runtime_helper(strict,codeSplitting: false):helper.jsdeclares a top-level__esmMin(hoisted to a root-scopevar __esmMinby its order wrapper) co-hosted with the runtime;late.jsis ordered after it.var __esmMinat root — the runtime helper and helper.js's binding.helper's init reassigns the shared binding to the user string, andinit_late = __esmMin(...)then throwsTypeError: __esmMin is not a function(the fixture fails to execute).__esmMin$1; every wrapper calls__esmMin$1(...)and the user's__esmMinkeeps its name. Snapshot pins the rename;_test.mjsasserts the executed result{ helper: 'H:USERVAL', late: 'LATE:z' }.Note on wrap-all vs on-demand, and the co-hosting mechanism. The reproduction is a wrap-all strict build (
strictExecutionOrderwithoutonDemandWrapping) because it deterministically co-hosts the runtime with user code in one chunk. The collision needs the runtime co-hosted with a user module at deconflict time.try_merge_runtime_chunkcan co-host the standalone runtime into a user consumer chunk even in multi-chunk builds, but under strict order wrappingensure_runtime_module_for_order_wraps(order_wrapping.rs:395-457) relocates a co-hosted runtime (modules.len() > 1) back to a standalone chunk unless code splitting is disabled — so at deconflict time the runtime sits next to user code only in single-chunk (codeSplitting: false) builds. (This stack's own multi-chunk strict on-demand fixtures, e.g.facade_gate_emergent_edge, confirm it — they emit a standalonerolldown-runtime.jschunk.) In a single chunk on-demand finds no cross-chunk execution-order hazard, so it wraps nothing and demands no__esmMin; wrap-all is the frame that both co-hosts the runtime and force-includes the helper. The fix itself is mode-independent:forces_runtime_stmtkeys onrequired_runtime_helpers(), covering on-demand and wrap-all and both the__esmMinand__esm(profiler_names) variants.Validation
cargo test -p rolldown— 1872 passed (+1 new fixture, executed under node); only the 5 known environmental failures (cjs_module_lexer_compat×3 +npm_packages/util_deprecateneedpnpm install;test262_module_codeneeds the submodule). No existing snapshot changed (the localsymbols_ns2drift is pre-existingnode_modules-resolution noise, reproducible with this change reverted, and is not committed).cargo fmt --all --check— clean;cargo clippy -p rolldown --all-targets -- -D warnings— clean.Follow-up to #10104; addresses item 2 (and the item-7 maintainability note) of #10104 (comment)