fix(treeshake): propagate pure annotation through compound exprs#9431
Merged
Conversation
The side-effect detector's catch-all collapses any compound expression
(object/array/sequence/conditional/logical/binary/unary/literal) into a
plain bool via `From<bool> for SideEffectDetail`, which can only encode
`Unknown` or `empty`. The `PureAnnotation` and `GlobalVarAccess`
metadata bits on a buried call are silently dropped.
`export default { foo: /* @__PURE__ */ (() => globalValue)() }`
therefore reaches statement level without the `PureAnnotation` flag,
the module fails to be marked `ExecutionOrderSensitive`, and
`wrap_modules` takes the `avoid_wrapping` branch. The module's
top-level `var` initializer is emitted at chunk top level — before
sibling modules like `setup.js` that the IIFE depends on.
Latent before oxc 0.131: oxc 0.130's DCE inlined the pure-annotated
IIFE down to a bare `globalValue` identifier, which the detector's
Identifier arm independently flagged via `GlobalVarAccess`. oxc 0.131
(oxc-project/oxc#22349) deliberately preserves the IIFE so downstream
tools see the annotation — surfacing this detector gap.
Add explicit arms for the transparent compound expression types
(`Object`/`Array`/`Sequence`/`Conditional`/`Logical`/`Binary`/`Unary`/
`TemplateLiteral`) so metadata propagates. Each arm gates via oxc's
`may_have_side_effects` for the spec-level boolean, then folds the
children through rolldown's own detector to harvest the flags,
stripping `Unknown` because oxc already certified the parent has no
side effect. Transparent wrappers (`(x)`, `x as T`, `x satisfies T`,
`x!`, `<T>x`, `x<T>`) are peeled at the top of `detect_side_effect_of_expr`
via `Expression::get_inner_expression`.
Closes #9425.
Contributor
Author
How to use the Graphite Merge QueueAdd 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. |
✅ Deploy Preview for rolldown-rs canceled.
|
Merging this PR will not alter performance
Comparing Footnotes
|
The trailing `- Unknown` in `fold_compound` is load-bearing: rolldown's detector can be more conservative than oxc for some sub-expressions (e.g. CJS-export assignment via `check_pure_cjs_export` returns `PureCjs` even when oxc considers the assignment side-effect-free). Once oxc has certified the parent at the gate, we strip any `Unknown` that a child contributed so only metadata bits propagate. Also adds a unit-test case for `TSInstantiationExpression` (`f<T>`), the fifth TS wrapper peeled by `get_inner_expression()` — the original `<T>x` form (TSTypeAssertion) is ambiguous with JSX under the test helper's `SourceType::tsx()` and isn't expressible here.
60081a6 to
67cbd30
Compare
shulaoda
approved these changes
May 18, 2026
Merged
shulaoda
added a commit
that referenced
this pull request
May 20, 2026
## [1.0.2] - 2026-05-20 ### 🚀 Features - devtools: emit package size in PackageGraphReady (#9434) by @IWANABETHATGUY - devtools: classify package dependency types (#9427) by @IWANABETHATGUY - devtools: map packages to modules and chunks (#9426) by @IWANABETHATGUY - devtools: mark used packages (#9423) by @IWANABETHATGUY - devtools: make duplicate packages discoverable (#9422) by @IWANABETHATGUY - devtools: emit package metadata (#9421) by @IWANABETHATGUY - update oxc to 0.132.0 (#9449) by @shulaoda - update oxc to 0.131.0 (#9424) by @shulaoda - allow checks.* to escalate emissions to hard errors (#9388) by @IWANABETHATGUY - dev: support watcher options `include` and `exclude` (#9395) by @h-a-n-a - emit warnings for invalid pure annotations (#9381) by @Kyujenius ### 🐛 Bug Fixes - hash: keep chunk file names stable when an unrelated entry is added (#9444) by @hyf0 - call `codeSplitting.groups[].name` in deterministic order (#9457) by @sapphi-red - dev/lazy: make `resolve_id` idempotent when the resolved id is already a lazy entry (#9439) by @h-a-n-a - chunk-optimization: publish absorbed dynamic-entry namespace cross-chunk (#9448) by @IWANABETHATGUY - treeshake: propagate pure annotation through compound exprs (#9431) by @Dunqing - finalizer: skip redundant init call when barrel executes in same chunk (#9354) by @IWANABETHATGUY - linking: initialize wrapped ESM re-export owners (#9353) by @IWANABETHATGUY - do not inherit __toESM across chunks for named-only external imports (#9333) (#9415) by @IWANABETHATGUY - watcher: don't write output or emit events after close() (#9328) by @situ2001 - chunk-optimization: avoid unsafe dynamic-only merges (#9398) by @IWANABETHATGUY - cjs: rename CJS-wrapped locals that would shadow chunk-scope names (#9392) by @hyf0 - dev/lazy: watch lazy modules added in rebuilds (#9391) by @h-a-n-a ### 🚜 Refactor - rolldown_dev: move dev example to break publish cycle (#9465) by @Boshen - binding: drop unsafe napi string helper, hoist transform ArcStr (#9456) by @hyf0 - ecmascript_utils: split rewrite_ident_reference off JsxExt trait (#9417) by @IWANABETHATGUY - use `ThreadsafeFunction::call_async_catch` (#9390) by @sapphi-red ### 📚 Documentation - devtools: document @rolldown/debug usage and package graph consumption (#9435) by @IWANABETHATGUY - replace `Inter` with system font stack in OG template SVG (#9240) by @yvbopeng - remove `output.comments` warning as all issues have been resolved (#9393) by @sapphi-red - in-depth: clarify @__PURE__ scope and document positions (#9389) by @Kyujenius - readme: remove release candidate notice (#9387) by @shulaoda ### ⚡ Performance - vite-resolve: cache importer existence checks (#9443) by @Brooooooklyn - binding: reduce plugin string clones (#9436) by @Brooooooklyn ### 🧪 Testing - enable `legal_comments_inline` test (#9394) by @sapphi-red ### ⚙️ Miscellaneous Tasks - bump pnpm to v11.1.2 (#9447) by @Boshen - deps: update rust crates (#9461) by @renovate[bot] - deps: update rollup submodule for tests to v4.60.4 (#9453) by @rolldown-guard[bot] - deps: update test262 submodule for tests (#9454) by @rolldown-guard[bot] - deps: update npm packages (#9430) by @renovate[bot] - deps: update github actions (#9429) by @renovate[bot] - deps: update dependency rolldown-plugin-dts to v0.25.1 (#9452) by @renovate[bot] - deps: update rust crates (#9428) by @renovate[bot] - revert allow checks.* to escalate emissions to hard errors (#9388) (#9438) by @IWANABETHATGUY - update mimalloc-safe to 0.1.61 (#9413) by @shulaoda - deny `todo`, `unimplemented`, and `print_stderr` clippy lints (#9412) by @Boshen - deps: update mimalloc-safe to 0.1.60 (#9410) by @shulaoda - remove `pip install setuptools` workaround for node-gyp on macOS (#9370) by @sapphi-red - renovate: disable automerge to require manual approval (#9386) by @shulaoda - deps: update napi (#9385) by @renovate[bot] ### ❤️ New Contributors * @yvbopeng made their first contribution in [#9240](#9240) Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
V1OL3TF0X
pushed a commit
to V1OL3TF0X/rolldown
that referenced
this pull request
May 25, 2026
…ldown#9431) ### Root cause `SideEffectDetector::detect_side_effect_of_expr`'s catch-all arm collapses any compound expression (`ObjectExpression`, `ArrayExpression`, `SequenceExpression`, `Conditional`/`Logical`/`Binary`/`Unary` expressions, `TemplateLiteral`) into a plain bool via `From<bool> for SideEffectDetail`: ```rust impl From<bool> for SideEffectDetail { fn from(value: bool) -> Self { if value { SideEffectDetail::Unknown } else { SideEffectDetail::empty() } } } ``` That conversion can only encode `Unknown` or `empty`. The `PureAnnotation` and `GlobalVarAccess` metadata bits on a buried call are silently dropped at the catch-all and never reach the statement-level intersection that drives `EcmaViewMeta::ExecutionOrderSensitive`. Concretely, `export default { foo: /* @__PURE__ */ (() => globalValue)() }` reaches statement level with `SideEffectDetail::empty()`. `wrap_modules` (`crates/rolldown/src/stages/link_stage/wrapping.rs`) takes the `avoid_wrapping` branch, `WrapKind::None` is emitted, and the module's top-level `var foo_inner_default = ...` runs at chunk top level — before sibling modules like `setup.js` that the IIFE depends on: ```js var foo_inner_default = { foo: /* @__PURE__ */ (() => globalValue)() }; // globalValue is undefined here __esmMin((() => { globalThis.globalValue = "foo"; // ... }))(); ``` ### Why this was latent before oxc 0.131 oxc 0.130's DCE pass unconditionally inlined `/* @__PURE__ */ (() => globalValue)()` down to a bare `globalValue` identifier. After inlining, the detector's `Identifier` arm independently flagged the unresolved global via `GlobalVarAccess` (a code path separate from `PureAnnotation`), the module was marked `ExecutionOrderSensitive`, and wrapping kicked in. Wrap-correctness was accidentally riding on oxc's IIFE inlining. oxc 0.131 (oxc-project/oxc#22349) intentionally **preserves** pure-annotated IIFEs in DCE-only mode so downstream tree-shakers can see the annotation. That is the right call on oxc's side — but it removes the inline-to-bare-global behavior rolldown was implicitly leaning on, exposing this detector gap. ### Changes - Add explicit arms in `detect_side_effect_of_expr` for the transparent compound expression types so metadata propagates: `ObjectExpression`, `ArrayExpression`, `SequenceExpression`, `ConditionalExpression`, `LogicalExpression`, `BinaryExpression`, `UnaryExpression`, `TemplateLiteral`. - Each arm goes through a single helper `fold_compound(expr, children)` that: 1. Asks oxc's `may_have_side_effects` for the spec-level boolean (handles ToPrimitive coercion, getter triggers in spread, Symbol checks in templates, etc.). If oxc says yes, return `Unknown` and stop. 2. Otherwise recurses through the children via rolldown's own detector to harvest `PureAnnotation` / `GlobalVarAccess` flags. 3. Strips `Unknown` from the harvested result since oxc already certified the parent has no side effect — only metadata bits should propagate. - Peel transparent syntactic wrappers (`(x)`, `x as T`, `x satisfies T`, `x!`, `<T>x`, `x<T>`) at the top of `detect_side_effect_of_expr` via `Expression::get_inner_expression()` — they add no runtime semantics, so a single peel covers all five wrapper variants (and any future ones oxc adds to that method). This mirrors the existing gate-and-harvest pattern already used by `detect_side_effect_of_call_expr` and the `NewExpression` arm. ### Tests New unit tests in `side_effect_detector::test`: - `test_pure_annotation_propagates_through_compound_expr` — covers `PureAnnotation` propagation through `Object`/`Array`/`Sequence`/`Conditional`/`Logical`/`Binary` (`===`) / `Unary` (`typeof`) / computed-key / nested compound, plus `GlobalVarAccess` propagation through compound, plus TS wrapper passthroughs (`as`, `satisfies`, `!`). - `test_pure_annotation_not_propagated_through_function_body` — verifies a pure annotation inside an arrow/function body does **not** propagate to the statement level (function bodies aren't evaluated at module init). - `test_compound_expr_side_effectful_operand_still_unknown` — verifies a real side effect anywhere in a compound expression still surfaces as `Unknown`; the new arms don't weaken side-effect detection. Existing integration suites kept green without snapshot changes: ```sh cargo test --package rolldown --lib ast_scanner::side_effect_detector::test:: cargo test --package rolldown --test integration -- strict_execution_order # 24 passed cargo test --package rolldown --test integration -- tree_shaking # 72 passed ``` The two pre-existing integration fixtures called out in the issue (`function/experimental/strict_execution_order/concatenate_wrapped_modules` and `.../issue_5303`) pass with their current snapshots — those snapshots already encode the wrapped behavior, so this PR restores conformance to them after the oxc 0.131 bump (rolldown#9424) would otherwise break them. Closes rolldown#9425. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: dalaoshu <165626830+shulaoda@users.noreply.github.com>
V1OL3TF0X
pushed a commit
to V1OL3TF0X/rolldown
that referenced
this pull request
May 25, 2026
## [1.0.2] - 2026-05-20 ### 🚀 Features - devtools: emit package size in PackageGraphReady (rolldown#9434) by @IWANABETHATGUY - devtools: classify package dependency types (rolldown#9427) by @IWANABETHATGUY - devtools: map packages to modules and chunks (rolldown#9426) by @IWANABETHATGUY - devtools: mark used packages (rolldown#9423) by @IWANABETHATGUY - devtools: make duplicate packages discoverable (rolldown#9422) by @IWANABETHATGUY - devtools: emit package metadata (rolldown#9421) by @IWANABETHATGUY - update oxc to 0.132.0 (rolldown#9449) by @shulaoda - update oxc to 0.131.0 (rolldown#9424) by @shulaoda - allow checks.* to escalate emissions to hard errors (rolldown#9388) by @IWANABETHATGUY - dev: support watcher options `include` and `exclude` (rolldown#9395) by @h-a-n-a - emit warnings for invalid pure annotations (rolldown#9381) by @Kyujenius ### 🐛 Bug Fixes - hash: keep chunk file names stable when an unrelated entry is added (rolldown#9444) by @hyf0 - call `codeSplitting.groups[].name` in deterministic order (rolldown#9457) by @sapphi-red - dev/lazy: make `resolve_id` idempotent when the resolved id is already a lazy entry (rolldown#9439) by @h-a-n-a - chunk-optimization: publish absorbed dynamic-entry namespace cross-chunk (rolldown#9448) by @IWANABETHATGUY - treeshake: propagate pure annotation through compound exprs (rolldown#9431) by @Dunqing - finalizer: skip redundant init call when barrel executes in same chunk (rolldown#9354) by @IWANABETHATGUY - linking: initialize wrapped ESM re-export owners (rolldown#9353) by @IWANABETHATGUY - do not inherit __toESM across chunks for named-only external imports (rolldown#9333) (rolldown#9415) by @IWANABETHATGUY - watcher: don't write output or emit events after close() (rolldown#9328) by @situ2001 - chunk-optimization: avoid unsafe dynamic-only merges (rolldown#9398) by @IWANABETHATGUY - cjs: rename CJS-wrapped locals that would shadow chunk-scope names (rolldown#9392) by @hyf0 - dev/lazy: watch lazy modules added in rebuilds (rolldown#9391) by @h-a-n-a ### 🚜 Refactor - rolldown_dev: move dev example to break publish cycle (rolldown#9465) by @Boshen - binding: drop unsafe napi string helper, hoist transform ArcStr (rolldown#9456) by @hyf0 - ecmascript_utils: split rewrite_ident_reference off JsxExt trait (rolldown#9417) by @IWANABETHATGUY - use `ThreadsafeFunction::call_async_catch` (rolldown#9390) by @sapphi-red ### 📚 Documentation - devtools: document @rolldown/debug usage and package graph consumption (rolldown#9435) by @IWANABETHATGUY - replace `Inter` with system font stack in OG template SVG (rolldown#9240) by @yvbopeng - remove `output.comments` warning as all issues have been resolved (rolldown#9393) by @sapphi-red - in-depth: clarify @__PURE__ scope and document positions (rolldown#9389) by @Kyujenius - readme: remove release candidate notice (rolldown#9387) by @shulaoda ### ⚡ Performance - vite-resolve: cache importer existence checks (rolldown#9443) by @Brooooooklyn - binding: reduce plugin string clones (rolldown#9436) by @Brooooooklyn ### 🧪 Testing - enable `legal_comments_inline` test (rolldown#9394) by @sapphi-red ### ⚙️ Miscellaneous Tasks - bump pnpm to v11.1.2 (rolldown#9447) by @Boshen - deps: update rust crates (rolldown#9461) by @renovate[bot] - deps: update rollup submodule for tests to v4.60.4 (rolldown#9453) by @rolldown-guard[bot] - deps: update test262 submodule for tests (rolldown#9454) by @rolldown-guard[bot] - deps: update npm packages (rolldown#9430) by @renovate[bot] - deps: update github actions (rolldown#9429) by @renovate[bot] - deps: update dependency rolldown-plugin-dts to v0.25.1 (rolldown#9452) by @renovate[bot] - deps: update rust crates (rolldown#9428) by @renovate[bot] - revert allow checks.* to escalate emissions to hard errors (rolldown#9388) (rolldown#9438) by @IWANABETHATGUY - update mimalloc-safe to 0.1.61 (rolldown#9413) by @shulaoda - deny `todo`, `unimplemented`, and `print_stderr` clippy lints (rolldown#9412) by @Boshen - deps: update mimalloc-safe to 0.1.60 (rolldown#9410) by @shulaoda - remove `pip install setuptools` workaround for node-gyp on macOS (rolldown#9370) by @sapphi-red - renovate: disable automerge to require manual approval (rolldown#9386) by @shulaoda - deps: update napi (rolldown#9385) by @renovate[bot] ### ❤️ New Contributors * @yvbopeng made their first contribution in [rolldown#9240](rolldown#9240) Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
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.

Root cause
SideEffectDetector::detect_side_effect_of_expr's catch-all arm collapses any compound expression (ObjectExpression,ArrayExpression,SequenceExpression,Conditional/Logical/Binary/Unaryexpressions,TemplateLiteral) into a plain bool viaFrom<bool> for SideEffectDetail:That conversion can only encode
Unknownorempty. ThePureAnnotationandGlobalVarAccessmetadata bits on a buried call are silently dropped at the catch-all and never reach the statement-level intersection that drivesEcmaViewMeta::ExecutionOrderSensitive.Concretely,
export default { foo: /* @__PURE__ */ (() => globalValue)() }reaches statement level withSideEffectDetail::empty().wrap_modules(crates/rolldown/src/stages/link_stage/wrapping.rs) takes theavoid_wrappingbranch,WrapKind::Noneis emitted, and the module's top-levelvar foo_inner_default = ...runs at chunk top level — before sibling modules likesetup.jsthat the IIFE depends on:Why this was latent before oxc 0.131
oxc 0.130's DCE pass unconditionally inlined
/* @__PURE__ */ (() => globalValue)()down to a bareglobalValueidentifier. After inlining, the detector'sIdentifierarm independently flagged the unresolved global viaGlobalVarAccess(a code path separate fromPureAnnotation), the module was markedExecutionOrderSensitive, and wrapping kicked in. Wrap-correctness was accidentally riding on oxc's IIFE inlining.oxc 0.131 (oxc-project/oxc#22349) intentionally preserves pure-annotated IIFEs in DCE-only mode so downstream tree-shakers can see the annotation. That is the right call on oxc's side — but it removes the inline-to-bare-global behavior rolldown was implicitly leaning on, exposing this detector gap.
Changes
detect_side_effect_of_exprfor the transparent compound expression types so metadata propagates:ObjectExpression,ArrayExpression,SequenceExpression,ConditionalExpression,LogicalExpression,BinaryExpression,UnaryExpression,TemplateLiteral.fold_compound(expr, children)that:may_have_side_effectsfor the spec-level boolean (handles ToPrimitive coercion, getter triggers in spread, Symbol checks in templates, etc.). If oxc says yes, returnUnknownand stop.PureAnnotation/GlobalVarAccessflags.Unknownfrom the harvested result since oxc already certified the parent has no side effect — only metadata bits should propagate.(x),x as T,x satisfies T,x!,<T>x,x<T>) at the top ofdetect_side_effect_of_exprviaExpression::get_inner_expression()— they add no runtime semantics, so a single peel covers all five wrapper variants (and any future ones oxc adds to that method).This mirrors the existing gate-and-harvest pattern already used by
detect_side_effect_of_call_exprand theNewExpressionarm.Tests
New unit tests in
side_effect_detector::test:test_pure_annotation_propagates_through_compound_expr— coversPureAnnotationpropagation throughObject/Array/Sequence/Conditional/Logical/Binary(===) /Unary(typeof) / computed-key / nested compound, plusGlobalVarAccesspropagation through compound, plus TS wrapper passthroughs (as,satisfies,!).test_pure_annotation_not_propagated_through_function_body— verifies a pure annotation inside an arrow/function body does not propagate to the statement level (function bodies aren't evaluated at module init).test_compound_expr_side_effectful_operand_still_unknown— verifies a real side effect anywhere in a compound expression still surfaces asUnknown; the new arms don't weaken side-effect detection.Existing integration suites kept green without snapshot changes:
The two pre-existing integration fixtures called out in the issue (
function/experimental/strict_execution_order/concatenate_wrapped_modulesand.../issue_5303) pass with their current snapshots — those snapshots already encode the wrapped behavior, so this PR restores conformance to them after the oxc 0.131 bump (#9424) would otherwise break them.Closes #9425.