Skip to content

[compiler] Handle sequential optionals#451

Open
everettbu wants to merge 1 commit into
mainfrom
nested-sequential-optionals-rebased
Open

[compiler] Handle sequential optionals#451
everettbu wants to merge 1 commit into
mainfrom
nested-sequential-optionals-rebased

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35673
Original author: josephsavona


The previous PR (#35606) handled maybe-throw terminals within value blocks, and the changes there also made BuildReactiveFunction able to handle sequential optionals. However, CollectOptionalChainDependencies doesn't yet handle sequential value blocks, like foo(a?.b, b?.c) ?? {}. This PR fixes the remaining case, which means we should no longer have arbitrary limitations around value blocks.

The previous PR (#35606) handled maybe-throw terminals within value blocks, and the changes there also made BuildReactiveFunction able to handle sequential optionals. However, CollectOptionalChainDependencies doesn't yet handle sequential value blocks, like `foo(a?.b, b?.c) ?? {}`. This PR fixes the remaining case, which means we should no longer have arbitrary limitations around value blocks.
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 2, 2026
@greptile-apps

greptile-apps Bot commented Feb 2, 2026

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

This PR extends the React compiler's ability to handle sequential optional chains in value blocks. Previously, CollectOptionalChainDependencies could not handle cases where multiple optional chains or other value blocks (logical, ternary) appeared as function arguments, such as foo(a?.b, b?.c) ?? {}.

The implementation adds two new helper functions:

  • findOptionalTestBlock: Traverses through intermediate value blocks and nested terminals to find the test block for an optional chain
  • traverseValueBlock: Recursively processes value blocks to collect nested optional chains within logical, ternary, or sequence expressions

The fix resolves a previous TODO error ("Unexpected terminal kind optional for optional fallthrough block") that occurred when the compiler encountered sequential optionals. The PR includes comprehensive test coverage for various scenarios including multiple optional arguments, nested calls, logical operators, ternaries, and combinations thereof.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-structured with clear recursive helpers that follow existing patterns in the codebase. The fix resolves a known limitation (previously marked as TODO), includes extensive test coverage for edge cases, and maintains backward compatibility.
  • No files require special attention

Important Files Changed

Filename Overview
compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts Added findOptionalTestBlock and traverseValueBlock helper functions to handle sequential optional chains in value blocks like foo(a?.b, b?.c) ?? {}
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/error.todo-optional-call-chain-in-optional.expect.md Deleted error test - this case now compiles successfully after the fix
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/optional-call-chain-in-optional.expect.md New passing test for createArray(value?.x, value?.y)?.join(', ') pattern that previously threw a TODO error
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/optional-call-with-multiple-optional-args.expect.md New test validating compilation of foo(a?.value, b?.value)?.result with multiple optional arguments

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants