Skip to content

fix(transformer/typescript): inline optional-chain enum member access#21834

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/transformer-enum-optional-chain-21813
May 7, 2026
Merged

fix(transformer/typescript): inline optional-chain enum member access#21834
graphite-app[bot] merged 1 commit into
mainfrom
fix/transformer-enum-optional-chain-21813

Conversation

@Dunqing

@Dunqing Dunqing commented Apr 27, 2026

Copy link
Copy Markdown
Member

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:

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) 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

  • cargo run -p oxc_transform_conformance -- --filter optimize-enums/optional-chain — all three pass
  • cargo run -p oxc_transform_conformance -- --filter const-enum-optional-chain — passes
  • cargo run -p oxc_transform_conformance -- --exec --filter optimize-enums/optional-chain-issue-21813 — runtime check passes
  • cargo test -p oxc_transformer — green
  • cargo clippy -p oxc_transformer -- -D warnings — clean
  • CI Conformance — green (snapshots updated for oxc.snap.md + oxc_exec.snap.md)

@Dunqing Dunqing requested a review from overlookmotel as a code owner April 27, 2026 08:52
@codspeed-hq

codspeed-hq Bot commented Apr 27, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing fix/transformer-enum-optional-chain-21813 (c86a10c) with main (3612db1)2

Open in CodSpeed

Footnotes

  1. 7 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.

  2. No successful run was found on main (15ded13) during the generation of this report, so 3612db1 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Dunqing Dunqing requested a review from sapphi-red April 29, 2026 15:19
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label May 7, 2026

Dunqing commented May 7, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • May 7, 8:44 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 7, 8:44 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • May 7, 8:44 AM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..
  • May 7, 8:44 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 7, 8:48 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • May 7, 8:48 AM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..
  • May 7, 8:48 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 7, 9:03 AM UTC: Dunqing added this pull request to the Graphite merge queue.
  • May 7, 9:08 AM UTC: Merged by the Graphite merge queue.

@Dunqing Dunqing force-pushed the fix/transformer-enum-optional-chain-21813 branch 2 times, most recently from e7d1adc to 89626a0 Compare May 7, 2026 08:51
@github-actions github-actions Bot added the A-transformer Area - Transformer / Transpiler label May 7, 2026
…#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`)
@graphite-app graphite-app Bot force-pushed the fix/transformer-enum-optional-chain-21813 branch from d50b964 to 0c7c01c Compare May 7, 2026 09:04
@graphite-app graphite-app Bot merged commit 0c7c01c into main May 7, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 7, 2026
@graphite-app graphite-app Bot deleted the fix/transformer-enum-optional-chain-21813 branch May 7, 2026 09:08
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]>
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-transformer Area - Transformer / Transpiler

Projects

None yet

2 participants