fix(transformer/typescript): inline optional-chain enum member access#21834
Merged
Merged
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
sapphi-red
approved these changes
May 7, 2026
Member
Author
Merge activity
|
e7d1adc to
89626a0
Compare
…#21834) ## Summary Closes #21813. Closes #21828. `c_num?.x` and `c_num?.['x']` were not handled by the enum-inlining pass in `enter_expression`, which only matched `Expression::StaticMemberExpression` and `Expression::ComputedMemberExpression`. When the es2020 optional-chain plugin lowered the chain first, the member access became: ```js c_num === null || c_num === void 0 ? void 0 : c_num.x ``` Then the inline pass saw the inner `c_num.x` and replaced it with the constant + deleted *that one* reference — but the two references in the test condition stayed live. With `optimizeConstEnums: true`, the const-enum declaration is then unconditionally removed, leaving dangling references to a binding that no longer exists. The same shape underlies the rolldown report ([rolldown/rolldown#9181](rolldown/rolldown#9181)) where rolldown's cross-module enum inlining drops the import that `c_num` was bound to. ## Fix Match `Expression::ChainExpression` whose top element is an optional `StaticMemberExpression` or `ComputedMemberExpression` on an enum identifier, and inline the entire chain to the constant **before** es2020 sees it. Reuses the existing `try_inline_enum_member` / `try_inline_computed_enum_member` helpers — they don't care about the `optional` flag. ## Fixtures - `optimize-enums/optional-chain-issue-21813/` — the bug case + an `exec.ts` that runs the transformed output to verify runtime semantics - `optimize-enums/optional-chain-value-kept/` — IIFE preserved when a bare `Foo` ref also exists - `optimize-enums/optional-chain-deeper-access/` — `Foo?.x.toString()` / `Bar?.['y']?.toString()` exercise nested chains where the inner inline still fires via the existing arms - `const-enum-optional-chain/` — const-enum path with `optimizeConstEnums: true` ## Known limitation Deeper chains under es2020 lowering — `c_num?.x.y` or `c_num?.x()` where the optional access is below the chain element — still produce dangling refs because the chain element is non-optional or a `CallExpression` and this fix's match arm doesn't trigger. Without es2020 lowering they work (the inner `?.x` gets visited as a regular `Expression::StaticMemberExpression`), so the conformance fixtures pass. A more general fix would walk the chain to find the leftmost optional-on-enum access; out of scope for this PR. ## Test plan - [x] `cargo run -p oxc_transform_conformance -- --filter optimize-enums/optional-chain` — all three pass - [x] `cargo run -p oxc_transform_conformance -- --filter const-enum-optional-chain` — passes - [x] `cargo run -p oxc_transform_conformance -- --exec --filter optimize-enums/optional-chain-issue-21813` — runtime check passes - [x] `cargo test -p oxc_transformer` — green - [x] `cargo clippy -p oxc_transformer -- -D warnings` — clean - [x] CI Conformance — green (snapshots updated for `oxc.snap.md` + `oxc_exec.snap.md`)
d50b964 to
0c7c01c
Compare
camc314
added a commit
that referenced
this pull request
May 11, 2026
### 🚀 Features - 66c9b01 transformer/typescript: Debug_assert that `enum_eval` ran in semantic (#22252) (Dunqing) - ffe6475 minifier: Fold `Array` constructor with safe spreads (#22215) (camc314) ### 🐛 Bug Fixes - d3d0b18 traverse: Handle `ChainElement::TSNonNullExpression` in `GatherNodeParts` (#22247) (leaysgur) - 4e880de transformer/object-rest-spread: Declare temp vars for computed keys (#22284) (camc314) - a7c3e22 semantic: Clear member write target for computed keys (#22302) (camc314) - 6a8852d codegen: Emit newline after legal-comment orphan flush (#22304) (Dunqing) - 5da9fda transformer/explicit-resource-management: Preserve class names (#22306) (Dunqing) - b5d970f transformer/explicit-resource-management: Preserve class names (#22290) (camc314) - bc54fd4 minifier: Keep function / class names if direct eval is present in the scope (#22241) (sapphi-red) - 7a810c0 minifier: Refresh direct eval flags after DCE (#21787) (Dunqing) - dd88726 transformer/legacy-decorator: Preserve accessor type annotation for emitDecoratorMetadata (#21966) (Dunqing) - 29a3cd7 codegen: Swap mapping/indent order for top-level decls (#22206) (Dunqing) - 73b4f40 minifier: Preserve catch binding with direct eval (#22221) (camc314) - 0e13d17 minifier: Preserve optional chain base side effects (#22219) (camc314) - 0c7c01c transformer/typescript: Inline optional-chain enum member access (#21834) (Dunqing) - a6aff7e codegen: Emit block/array/object end mapping at close char (#22200) (Dunqing) - a099b03 codegen: Emit call end mapping at `)` position, not past it (#22199) (Dunqing) - 5753774 minifier: Cap if-return ternary collapse for firefox (#21841) (Gurupungav Narayanan) - 2493bdd codegen: Correct sourcemap end mappings for closing delimiters (#22001) (Mark Dalgleish) - 3b385e2 minifier: Bail optimizing `Array` with unknown arg count (#22188) (camc314) - 9fa2122 parser: Parse array computed class keys (#22159) (camc314) ### 📚 Documentation - a4a6892 napi/parser: Correct code comment (#22278) (overlookmotel) - 9305373 oxc: Update README (#22178) (camc314) Co-authored-by: Cameron <[email protected]>
6 tasks
Dunqing
added a commit
to rolldown/rolldown
that referenced
this pull request
May 13, 2026
We already implemented single file optional chaining inline in Oxc oxc-project/oxc#21834 ## Summary Teaches the cross-module enum inliner to fold optional-chain access (`E?.x`, `E?.['x']`) to the same literal as `E.x` / `E['x']`. With that hole filled, the tree-shaker bypass added in #9229 no longer needs its `!prop.optional` guard. ## Why this is safe Enum bindings are always defined — the generated IIFE initializes them to `{}` (never null/undefined), and const enums are removed entirely after inlining. So `?.` cannot short-circuit on an enum object: `E?.x` is observationally equivalent to `E.x`. The TDZ assumption this relies on (an enum identifier reaching the finalizer is always bound to a real value) is the same one `E.x` inlining already makes — we just extend it to the optional form. ## What changed - `crates/rolldown/src/module_finalizers/mod.rs` — `try_inline_enum_access` now also matches `ChainExpression { StaticMemberExpression | ComputedMemberExpression }`. - `crates/rolldown/src/module_finalizers/impl_visit_mut.rs` — pre-match inline attempt for `ChainExpression`, done before the main `visit_expression` dispatch so `expr` can be passed to `try_inline_enum_access` without conflicting with the existing arm's mutable destructure of the inner chain. - `crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rs` — drops the `!prop.optional` guard from the enum-decl bypass. `!is_write` stays. - `crates/rolldown/tests/esbuild/ts/ts_enum_cross_module_inlining_access/` — moves the optional cases into the `inlined` group; snapshot now folds all 8 inlinable accesses to literals while keeping `e_num`/`e_str` (bare identifier references). ## Test plan - [x] `cargo test -p rolldown --test integration ts_enum_cross_module_inlining_access` - [x] `cargo test -p rolldown` (1703 passed, 0 failed) - [x] `cargo test -p rolldown --test integration enum` (24 passed) - [x] `cargo test -p rolldown --test integration chain` (25 passed) - [x] Spot-checked single-module `enum E { x = 1 }; console.log(E?.x)` via CLI — now folds to `console.log(1)` - [x] `just ued` — produced no additional snap-diff changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 <[email protected]>
IWANABETHATGUY
pushed a commit
to rolldown/rolldown
that referenced
this pull request
May 18, 2026
We already implemented single file optional chaining inline in Oxc oxc-project/oxc#21834 ## Summary Teaches the cross-module enum inliner to fold optional-chain access (`E?.x`, `E?.['x']`) to the same literal as `E.x` / `E['x']`. With that hole filled, the tree-shaker bypass added in #9229 no longer needs its `!prop.optional` guard. ## Why this is safe Enum bindings are always defined — the generated IIFE initializes them to `{}` (never null/undefined), and const enums are removed entirely after inlining. So `?.` cannot short-circuit on an enum object: `E?.x` is observationally equivalent to `E.x`. The TDZ assumption this relies on (an enum identifier reaching the finalizer is always bound to a real value) is the same one `E.x` inlining already makes — we just extend it to the optional form. ## What changed - `crates/rolldown/src/module_finalizers/mod.rs` — `try_inline_enum_access` now also matches `ChainExpression { StaticMemberExpression | ComputedMemberExpression }`. - `crates/rolldown/src/module_finalizers/impl_visit_mut.rs` — pre-match inline attempt for `ChainExpression`, done before the main `visit_expression` dispatch so `expr` can be passed to `try_inline_enum_access` without conflicting with the existing arm's mutable destructure of the inner chain. - `crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rs` — drops the `!prop.optional` guard from the enum-decl bypass. `!is_write` stays. - `crates/rolldown/tests/esbuild/ts/ts_enum_cross_module_inlining_access/` — moves the optional cases into the `inlined` group; snapshot now folds all 8 inlinable accesses to literals while keeping `e_num`/`e_str` (bare identifier references). ## Test plan - [x] `cargo test -p rolldown --test integration ts_enum_cross_module_inlining_access` - [x] `cargo test -p rolldown` (1703 passed, 0 failed) - [x] `cargo test -p rolldown --test integration enum` (24 passed) - [x] `cargo test -p rolldown --test integration chain` (25 passed) - [x] Spot-checked single-module `enum E { x = 1 }; console.log(E?.x)` via CLI — now folds to `console.log(1)` - [x] `just ued` — produced no additional snap-diff changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 <[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
Closes #21813.
Closes #21828.
c_num?.xandc_num?.['x']were not handled by the enum-inlining pass inenter_expression, which only matchedExpression::StaticMemberExpressionandExpression::ComputedMemberExpression. When the es2020 optional-chain plugin lowered the chain first, the member access became:Then the inline pass saw the inner
c_num.xand replaced it with the constant + deleted that one reference — but the two references in the test condition stayed live. WithoptimizeConstEnums: true, the const-enum declaration is then unconditionally removed, leaving dangling references to a binding that no longer exists.The same shape underlies the rolldown report (rolldown/rolldown#9181) where rolldown's cross-module enum inlining drops the import that
c_numwas bound to.Fix
Match
Expression::ChainExpressionwhose top element is an optionalStaticMemberExpressionorComputedMemberExpressionon an enum identifier, and inline the entire chain to the constant before es2020 sees it. Reuses the existingtry_inline_enum_member/try_inline_computed_enum_memberhelpers — they don't care about theoptionalflag.Fixtures
optimize-enums/optional-chain-issue-21813/— the bug case + anexec.tsthat runs the transformed output to verify runtime semanticsoptimize-enums/optional-chain-value-kept/— IIFE preserved when a bareFooref also existsoptimize-enums/optional-chain-deeper-access/—Foo?.x.toString()/Bar?.['y']?.toString()exercise nested chains where the inner inline still fires via the existing armsconst-enum-optional-chain/— const-enum path withoptimizeConstEnums: trueKnown limitation
Deeper chains under es2020 lowering —
c_num?.x.yorc_num?.x()where the optional access is below the chain element — still produce dangling refs because the chain element is non-optional or aCallExpressionand this fix's match arm doesn't trigger. Without es2020 lowering they work (the inner?.xgets visited as a regularExpression::StaticMemberExpression), so the conformance fixtures pass. A more general fix would walk the chain to find the leftmost optional-on-enum access; out of scope for this PR.Test plan
cargo run -p oxc_transform_conformance -- --filter optimize-enums/optional-chain— all three passcargo run -p oxc_transform_conformance -- --filter const-enum-optional-chain— passescargo run -p oxc_transform_conformance -- --exec --filter optimize-enums/optional-chain-issue-21813— runtime check passescargo test -p oxc_transformer— greencargo clippy -p oxc_transformer -- -D warnings— cleanoxc.snap.md+oxc_exec.snap.md)