Skip to content

fix(code-splitting): treat class definition-time global reads as order-sensitive#10322

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/strict-class-def-order-sensitivity
Jul 21, 2026
Merged

fix(code-splitting): treat class definition-time global reads as order-sensitive#10322
graphite-app[bot] merged 1 commit into
mainfrom
codex/strict-class-def-order-sensitivity

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

Fixes the class order-sensitivity gap found while reviewing #10104. A class can be side-effect-free for tree shaking while its definition still reads mutable global state from an extends expression, computed key, static initializer, or static block. Reordering that class across a module that patches the global can change behavior.

When Oxc certifies the whole class as side-effect-free, Rolldown now runs an independent definition-time visitor that collects only GlobalVarAccess and PureAnnotation reasons. It never invokes or short-circuits on tree-shaking analysis. Therefore a PureCJS target write remains tree-shaking-only, while its RHS and later siblings are still visited.

The visitor covers evaluated binding defaults, computed keys, assignment RHS, direct IIFEs, and nested classes. It skips deferred function and method bodies and instance initializers.

The tree-shaking channel is structurally unchanged: the original whole-class Oxc boolean remains its only input, and the new visitor can only add order-sensitive reasons.

Regression coverage

  • PureCJS-only and literal binding-default cases remain order-insensitive.
  • PureCJS followed by a global read or pure-annotated call is order-sensitive in either operand order.
  • Static-block binding defaults, computed member keys, assignment RHS, direct IIFEs, heritage expressions, and nested class-definition work are covered.
  • Deferred arrow/function bodies, method bodies, and instance initializers remain order-insensitive.

Validation

  • cargo test -p rolldown --lib — 87 passed.
  • cargo test -p rolldown — 87 unit and 49 integration tests passed; the only failure was the existing missing local test262 submodule.
  • cargo clippy -p rolldown --lib --tests -- -D warnings — clean.
  • cargo fmt --all -- --check — clean.

Follow-up to #10104; addresses Major 1 from #10104 (comment).

Draft, rebased directly onto main.

@hyfdev
hyfdev force-pushed the codex/strict-scc-final-topology-assert branch from ede4fd5 to bc71bc1 Compare July 16, 2026 15:14
@hyfdev
hyfdev force-pushed the codex/strict-class-def-order-sensitivity branch from 4709a6b to 4f804f3 Compare July 16, 2026 15:14
@hyfdev
hyfdev force-pushed the codex/strict-class-def-order-sensitivity branch from 4f804f3 to ab7808a Compare July 20, 2026 12:45
@hyfdev
hyfdev changed the base branch from codex/strict-scc-final-topology-assert to main July 20, 2026 12:45
@hyfdev
hyfdev force-pushed the codex/strict-class-def-order-sensitivity branch from ab7808a to b8df48d Compare July 20, 2026 13:55
@netlify

netlify Bot commented Jul 20, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit ff32edb
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a5ed05a4e378f000809cb9e

@hyfdev
hyfdev force-pushed the codex/strict-class-def-order-sensitivity branch from b8df48d to 3b7f8b7 Compare July 20, 2026 14:02
@hyfdev
hyfdev marked this pull request as ready for review July 20, 2026 14:11
Copilot AI review requested due to automatic review settings July 20, 2026 14:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codspeed-hq

codspeed-hq Bot commented Jul 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing codex/strict-class-def-order-sensitivity (3b7f8b7) with main (ae2ec2e)

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.

hyfdev commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jul 21, 1:49 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 21, 1:49 AM UTC: hyfdev added this pull request to the Graphite merge queue.
  • Jul 21, 1:55 AM UTC: Merged by the Graphite merge queue.

…r-sensitive (#10322)

## Summary

Fixes the class order-sensitivity gap found while reviewing #10104. A class can be side-effect-free for tree shaking while its definition still reads mutable global state from an extends expression, computed key, static initializer, or static block. Reordering that class across a module that patches the global can change behavior.

When Oxc certifies the whole class as side-effect-free, Rolldown now runs an independent definition-time visitor that collects only GlobalVarAccess and PureAnnotation reasons. It never invokes or short-circuits on tree-shaking analysis. Therefore a PureCJS target write remains tree-shaking-only, while its RHS and later siblings are still visited.

The visitor covers evaluated binding defaults, computed keys, assignment RHS, direct IIFEs, and nested classes. It skips deferred function and method bodies and instance initializers.

The tree-shaking channel is structurally unchanged: the original whole-class Oxc boolean remains its only input, and the new visitor can only add order-sensitive reasons.

## Regression coverage

- PureCJS-only and literal binding-default cases remain order-insensitive.
- PureCJS followed by a global read or pure-annotated call is order-sensitive in either operand order.
- Static-block binding defaults, computed member keys, assignment RHS, direct IIFEs, heritage expressions, and nested class-definition work are covered.
- Deferred arrow/function bodies, method bodies, and instance initializers remain order-insensitive.

## Validation

- cargo test -p rolldown --lib — 87 passed.
- cargo test -p rolldown — 87 unit and 49 integration tests passed; the only failure was the existing missing local test262 submodule.
- cargo clippy -p rolldown --lib --tests -- -D warnings — clean.
- cargo fmt --all -- --check — clean.

Follow-up to #10104; addresses Major 1 from #10104 (comment).

Draft, rebased directly onto main.
@graphite-app
graphite-app Bot force-pushed the codex/strict-class-def-order-sensitivity branch from 3b7f8b7 to ff32edb Compare July 21, 2026 01:50
@graphite-app
graphite-app Bot merged commit ff32edb into main Jul 21, 2026
34 checks passed
@graphite-app
graphite-app Bot deleted the codex/strict-class-def-order-sensitivity branch July 21, 2026 01:55
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.

3 participants