Skip to content

fix(transformer_plugins): unwrap ChainExpression after define replacement removes optional markers#20058

Merged
Dunqing merged 8 commits intooxc-project:mainfrom
IWANABETHATGUY:replace-global-define-panic
Mar 9, 2026
Merged

fix(transformer_plugins): unwrap ChainExpression after define replacement removes optional markers#20058
Dunqing merged 8 commits intooxc-project:mainfrom
IWANABETHATGUY:replace-global-define-panic

Conversation

@IWANABETHATGUY
Copy link
Contributor

@IWANABETHATGUY IWANABETHATGUY commented Mar 6, 2026

Summary

  • Fix panic when ReplaceGlobalDefines removes all optional markers from a ChainExpression, causing the optional chaining lowering transformer to hit unreachable!()
  • Fix incorrect test expectation for process?.env[0] with process.env -> {}

Root Cause

When ReplaceGlobalDefines replaces a member expression inside a ChainExpression (e.g., process?.env[0] with define process.env -> {}), the inner StaticMemberExpression that carried optional: true is replaced by {}. This leaves a ChainExpression wrapper with no optional elements. When the optional chaining transformer later tries to lower this (target < ES2020), it panics at transform_chain_expression_impl in crates/oxc_transformer/src/es2020/optional_chaining.rs because it expects at least one optional element in the chain.

This affects the playground pipeline where define runs before the transformer.

Fix

After walk_expression in visit_expression, check if the current expression is a ChainExpression with no remaining optional markers. If so, unwrap it to a plain expression (e.g., ChainExpression(ComputedMemberExpression(...)) becomes ComputedMemberExpression(...)), producing a valid AST.

Test plan

  • Added integration test define_then_transform_optional_chain that reproduces the exact panic (define first, then transformer with ES2019 target)

  • cargo test -p oxc_transformer_plugins — all 42 integration tests pass

  • just lint passes clean

Copilot AI review requested due to automatic review settings March 6, 2026 02:50
@github-actions github-actions bot added the C-bug Category - Bug label Mar 6, 2026
@IWANABETHATGUY
Copy link
Contributor Author

playground reproduction:

https://playground.oxc.rs/?options=%7B%22run%22%3A%7B%22lint%22%3Atrue%2C%22formatter%22%3Afalse%2C%22transform%22%3Atrue%2C%22isolatedDeclarations%22%3Afalse%2C%22whitespace%22%3Afalse%2C%22mangle%22%3Afalse%2C%22compress%22%3Afalse%2C%22scope%22%3Atrue%2C%22symbol%22%3Atrue%2C%22cfg%22%3Afalse%7D%2C%22parser%22%3A%7B%22extension%22%3A%22tsx%22%2C%22allowReturnOutsideFunction%22%3Atrue%2C%22preserveParens%22%3Atrue%2C%22allowV8Intrinsics%22%3Atrue%2C%22semanticErrors%22%3Atrue%7D%2C%22linter%22%3A%7B%7D%2C%22formatter%22%3A%7B%22useTabs%22%3Afalse%2C%22tabWidth%22%3A2%2C%22endOfLine%22%3A%22lf%22%2C%22printWidth%22%3A80%2C%22singleQuote%22%3Afalse%2C%22jsxSingleQuote%22%3Afalse%2C%22quoteProps%22%3A%22as-needed%22%2C%22trailingComma%22%3A%22all%22%2C%22semi%22%3Atrue%2C%22arrowParens%22%3A%22always%22%2C%22bracketSpacing%22%3Atrue%2C%22bracketSameLine%22%3Afalse%2C%22objectWrap%22%3A%22preserve%22%2C%22singleAttributePerLine%22%3Afalse%7D%2C%22transformer%22%3A%7B%22target%22%3A%22es2015%22%2C%22useDefineForClassFields%22%3Atrue%2C%22experimentalDecorators%22%3Atrue%2C%22emitDecoratorMetadata%22%3Atrue%7D%2C%22isolatedDeclarations%22%3A%7B%22stripInternal%22%3Afalse%7D%2C%22codegen%22%3A%7B%22normal%22%3Atrue%2C%22jsdoc%22%3Atrue%2C%22annotation%22%3Atrue%2C%22legal%22%3Atrue%7D%2C%22compress%22%3A%7B%7D%2C%22mangle%22%3A%7B%22topLevel%22%3Atrue%2C%22keepNames%22%3Afalse%7D%2C%22controlFlow%22%3A%7B%22verbose%22%3Afalse%7D%2C%22inject%22%3A%7B%22inject%22%3A%7B%7D%7D%2C%22define%22%3A%7B%22define%22%3A%7B%22process.env%22%3A%22%7B%7D%22%7D%7D%7D&code=console.log%28process%3F.env%5B0%5D%29%3B

Note: It can only be reproduced when ReplaceGlobalDefines are running before the transformer.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a panic in the transformer pipeline when ReplaceGlobalDefines rewrites an optional chain in a way that leaves a ChainExpression wrapper without any remaining optional markers (notably in playground-like “define first, then lower” flows).

Changes:

  • Unwrap Expression::ChainExpression nodes after define-walking when the chain no longer contains any optional: true markers.
  • Add an integration test that reproduces the define-then-transform optional-chaining lowering scenario (target < ES2020).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/oxc_transformer_plugins/src/replace_global_defines.rs Adds post-walk normalization to unwrap “empty” ChainExpressions and helper logic to detect remaining optional markers.
crates/oxc_transformer_plugins/tests/integrations/replace_global_defines.rs Adds an integration test that runs defines before transformer lowering to ensure the prior panic path is covered.

@Dunqing Dunqing self-assigned this Mar 9, 2026
@Dunqing Dunqing merged commit 291d867 into oxc-project:main Mar 9, 2026
20 checks passed
camc314 pushed a commit that referenced this pull request Mar 9, 2026
### 🚀 Features

- e8547cc parser: Report error for using declarations in ambient
contexts (#19934) (camc314)
- 8345318 allocator: Add methods for boxed slices `ArenaBox<[T]>`
(#19968) (overlookmotel)
- f83be30 allocator: Add `Vec::push_fast` method (#19959)
(overlookmotel)

### 🐛 Bug Fixes

- 291d867 transformer_plugins: Unwrap ChainExpression after define
replacement removes optional markers (#20058) (IWANABETHATGUY)
- 36b2e56 codegen: Print type for TSImportEqualsDeclaration (#20128)
(camc314)
- 5a246ec codegen: Print type arguments for JSXOpeningElement (#20127)
(camc314)
- a40870e codegen: Preserve parens for TSNonNullExpression (#20125)
(camc314)
- ae830b2 codegen: Print `declare` for `TSInterfaceDeclaration` (#20124)
(camc314)
- 92cfb14 linter/plugins: Fix types for `walkProgram` and
`walkProgramWithCfg` (#20081) (overlookmotel)
- ee0491e apps,napi: Explicitly specify libs in tsconfigs (#20071)
(camc314)
- 588009e codegen: Print `static` keyword for TSIndexSignature (#19755)
(Dunqing)
- 5a8799c codegen: Print `with_clause` for `ExportNamedDeclaration`
(#20002) (Dunqing)
- 7502afe parser: Correct capacity for tokens `Vec` (#19967)
(overlookmotel)

### ⚡ Performance

- 4ea8f9a napi: Remove `napi_build::setup()` from `oxc_napi` to avoid
redundant rebuilds (#20094) (Boshen)
- 2baa5fb napi: Unify build-test profile to coverage for cache sharing
(#20090) (Boshen)
- 8ba61dd parser: Make pushing tokens faster (#19960) (overlookmotel)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants