fix(code-splitting): collect eager order reasons for on-demand wrapping#10387
Merged
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
@rolldown/browser
@rolldown/debug
rolldown
@rolldown/binding-android-arm64
@rolldown/binding-darwin-arm64
@rolldown/binding-darwin-x64
@rolldown/binding-freebsd-x64
@rolldown/binding-linux-arm-gnueabihf
@rolldown/binding-linux-arm64-gnu
@rolldown/binding-linux-arm64-musl
@rolldown/binding-linux-ppc64-gnu
@rolldown/binding-linux-s390x-gnu
@rolldown/binding-linux-x64-gnu
@rolldown/binding-linux-x64-musl
@rolldown/binding-openharmony-arm64
@rolldown/binding-wasm32-wasi
@rolldown/binding-win32-arm64-msvc
@rolldown/binding-win32-x64-msvc
commit: |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Member
|
The collector still misses three eager-execution forms. They're all reachable through the same route this PR already models — the manual-pure member-root computed key from 1.
function make() {}
const v = make[new class { x = /* @__PURE__ */ globalThis.__orderRead?.() ?? 5 }().k];I'd add an 2. Directly invoked function-literal tags aren't entered Evaluating self.visit_immediately_invoked_function(&tagged.tag);No gating needed here since the function/arrow arms are ungated legacy behavior. 3. Assignment-expression callees
None of these are blockers — they're in the same likelihood class as the conditional/logical arms this PR adds, so treating them as follow-ups would be consistent. But each fix is one small arm (or one line) plus one witness test in the style you already established, so folding them in here would be cheap. Either way they should land in the order-fuzzer matrix: the preview build passing the complete 60-form matrix tells me the matrix currently lacks all three forms as well. A few smaller things:
Approving with the above as suggested follow-ups. |
IWANABETHATGUY
approved these changes
Jul 23, 2026
Member
Author
Merge activity
|
…ng (#10387) ## Summary Strict on-demand wrapping relies on per-statement `StmtEvalAnalyzer` facts to mark modules execution-order-sensitive. Oxc can validly certify an outer IIFE, member operation, binding pattern, assignment, or manually pure invocation as side-effect-free and stop there, but that certification can hide a nested global read from the separate scheduling decision. The reader module then remains eager and can run before a patch module after code splitting. This adds an independent eager-evaluation order-reason walk, run only for strict on-demand builds and only while the module and the current statement are not already known to be sensitive. It collects scheduling metadata without changing `tree_shaking_flags`. Ordinary builds, wrap-all builds, and cross-module tree-shaking re-analysis retain their existing paths, and the class-definition collector keeps its legacy behavior outside the new strict on-demand statement pass. The walk covers immediately invoked function literals, member children, binding defaults and computed keys, assignment RHS, PureCJS RHS, and call/new/tagged-template expressions covered by `manualPureFunctions`. Deferred function and method bodies and unconstructed instance fields remain deferred, and a getter behind a `propertyReadSideEffects: false` read stays invisible, matching the documented `TopLevelImportReadDetector` contract. ## Tests - New execution fixture `strict_execution_order/eager_read_nested_in_pure_expression`: the reader's IIFE-wrapped nested global read is split into its own chunk and the output is executed under node. At the merge base (`adcebe1`) it fails with `reader evaluated before patch: value = 5` — the unwrapped reader chunk is hoisted above the patch assignment — and with this change it passes in both wrapping modes. - The analyzer table tests keep one witness per collector branch (IIFE callee arms, member children, binding positions, assignment RHS, PureCJS RHS, manual-pure call/new/tagged-template) plus deferred-boundary, getter-contract, and default/wrap-all non-leak controls. The full 60-form syntax matrix stays in the external order-fuzzer release gate; spellings that exercise an already-witnessed branch are deliberately not duplicated in-repo. - The conditional/logical IIFE-callee arms decide the outcome only when a manual-pure member root certifies the enclosing read; bare conditional/logical-callee IIFEs are already order-sensitive through `UnknownSideEffect` because Oxc never certifies such callees. `test_conditional_and_logical_iife_callees_behind_manual_pure_member` pins both facts and fails if the arms are removed. - Follow-up from review: the walk now also covers the remaining eager-execution forms reachable behind a certified manual-pure member root — the construction-time positions of a directly `new`ed class literal (constructor parameters/body, instance field and accessor initializers), directly invoked literal template tags, and assignment-expression callees including logical assignments. Each arm has a decider witness in `test_eager_walk_covers_constructed_classes_literal_tags_and_assignment_callees`, verified by per-arm ablation. The literal-tag entry is gated to strict on-demand, unlike the suggested ungated form, so the legacy class-mode collector never starts entering tag bodies in ordinary builds — same isolation rule as the other extended arms. - TODO (order-fuzzer side, tracked outside this repo): add the three forms above to the eager-evaluation release matrix; the current 60-form matrix predates them. ## Regression bracket The order-fuzzer release-gap audit maintains a 60-form eager-evaluation matrix: 49 catch cases across IIFE traversal, member children, binding patterns, assignment RHS, computed object keys, and manual-pure call/new/tagged-template forms, plus 11 boundary controls (rolldown-order-fuzzer@77f196b). For the same inputs, wrap-all and disabled code splitting pass throughout, and the preview build of this change passes the complete matrix. Three separate `manualPureFunctions` child-effect deletion failures are intentionally not addressed here because they are ordinary tree-shaking correctness bugs, not on-demand wrapping decisions. ## Validation - `cargo test --workspace --exclude rolldown_binding` — full Rust suite green at the current head with no snapshot drift (the preview CI run skipped Cargo Test under the `trigger: preview` label) - `cargo clippy -p rolldown --lib --tests -- -D warnings`, `cargo fmt -p rolldown` - New fixture verified red at merge base `adcebe1` and green at head, in both on-demand and wrap-all modes
graphite-app
Bot
force-pushed
the
fix/strict-order-eager-reasons
branch
from
July 23, 2026 08:53
a2ac5f7 to
4de88b1
Compare
IWANABETHATGUY
added a commit
that referenced
this pull request
Jul 24, 2026
…re chains (#10427) Fixes #10397, at both drop paths named in the issue's Cause section. One note for review. Two unit tests from #10387 encoded the old behavior as their premise, their comments said so explicitly, and they now expect conservative retention in both analysis dimensions. --------- Co-authored-by: IWANABETHATGUY <[email protected]>
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
Strict on-demand wrapping relies on per-statement
StmtEvalAnalyzerfacts to mark modules execution-order-sensitive. Oxc can validly certify an outer IIFE, member operation, binding pattern, assignment, or manually pure invocation as side-effect-free and stop there, but that certification can hide a nested global read from the separate scheduling decision. The reader module then remains eager and can run before a patch module after code splitting.This adds an independent eager-evaluation order-reason walk, run only for strict on-demand builds and only while the module and the current statement are not already known to be sensitive. It collects scheduling metadata without changing
tree_shaking_flags. Ordinary builds, wrap-all builds, and cross-module tree-shaking re-analysis retain their existing paths, and the class-definition collector keeps its legacy behavior outside the new strict on-demand statement pass.The walk covers immediately invoked function literals, member children, binding defaults and computed keys, assignment RHS, PureCJS RHS, and call/new/tagged-template expressions covered by
manualPureFunctions. Deferred function and method bodies and unconstructed instance fields remain deferred, and a getter behind apropertyReadSideEffects: falseread stays invisible, matching the documentedTopLevelImportReadDetectorcontract.Tests
New execution fixture
strict_execution_order/eager_read_nested_in_pure_expression: the reader's IIFE-wrapped nested global read is split into its own chunk and the output is executed under node. At the merge base (adcebe1) it fails withreader evaluated before patch: value = 5— the unwrapped reader chunk is hoisted above the patch assignment — and with this change it passes in both wrapping modes.The analyzer table tests keep one witness per collector branch (IIFE callee arms, member children, binding positions, assignment RHS, PureCJS RHS, manual-pure call/new/tagged-template) plus deferred-boundary, getter-contract, and default/wrap-all non-leak controls. The full 60-form syntax matrix stays in the external order-fuzzer release gate; spellings that exercise an already-witnessed branch are deliberately not duplicated in-repo.
The conditional/logical IIFE-callee arms decide the outcome only when a manual-pure member root certifies the enclosing read; bare conditional/logical-callee IIFEs are already order-sensitive through
UnknownSideEffectbecause Oxc never certifies such callees.test_conditional_and_logical_iife_callees_behind_manual_pure_memberpins both facts and fails if the arms are removed.Follow-up from review: the walk now also covers the remaining eager-execution forms reachable behind a certified manual-pure member root — the construction-time positions of a directly
newed class literal (constructor parameters/body, instance field and accessor initializers), directly invoked literal template tags, and assignment-expression callees including logical assignments. Each arm has a decider witness intest_eager_walk_covers_constructed_classes_literal_tags_and_assignment_callees, verified by per-arm ablation. The literal-tag entry is gated to strict on-demand, unlike the suggested ungated form, so the legacy class-mode collector never starts entering tag bodies in ordinary builds — same isolation rule as the other extended arms.TODO (order-fuzzer side, tracked outside this repo): add the three forms above to the eager-evaluation release matrix; the current 60-form matrix predates them.
Regression bracket
The order-fuzzer release-gap audit maintains a 60-form eager-evaluation matrix: 49 catch cases across IIFE traversal, member children, binding patterns, assignment RHS, computed object keys, and manual-pure call/new/tagged-template forms, plus 11 boundary controls (rolldown-order-fuzzer@77f196b). For the same inputs, wrap-all and disabled code splitting pass throughout, and the preview build of this change passes the complete matrix. Three separate
manualPureFunctionschild-effect deletion failures are intentionally not addressed here because they are ordinary tree-shaking correctness bugs, not on-demand wrapping decisions.Validation
cargo test --workspace --exclude rolldown_binding— full Rust suite green at the current head with no snapshot drift (the preview CI run skipped Cargo Test under thetrigger: previewlabel)cargo clippy -p rolldown --lib --tests -- -D warnings,cargo fmt -p rolldownadcebe1and green at head, in both on-demand and wrap-all modes