Conversation
Greptile SummaryFixed a longstanding bug in the React compiler where nested optional chaining within other value blocks (like ternaries, logical expressions, or nullish coalescing) would fail to compile. The issue occurred when constructing ReactiveFunction representations - the code was incorrectly using Key changes:
Confidence Score: 5/5
Important Files Changed
|
Much nicer workflow for working through errors in the compiler: * Run `yarn snap -w`, oops there are are errors * Hit 'p' to select a fixture => the suggestions populate with recent failures, sorted alphabetically. No need to copy/paste the name of the fixture you want to focus on! * tab/shift-tab to pick one, hit enter to select that one * ...Focus on fixing that test... * 'p' to re-enter the picker. Snap tracks the last state of each fixture and continues to show all tests that failed on their last run, so you can easily move on to the next one. The currently selected test is highlighted, making it easy to move to the next one. * 'a' at any time to run all tests * 'd' at any time to toggle debug output on/off (while focusing on a single test)
…opment Autogenerated summaries of each of the compiler passes which allow agents to get the key ideas of a compiler pass, including key input/output invariants, without having to reprocess the file each time. In the subsequent diff this seemed to help.
Optional chaining and other value blocks within try/catch blocks were throwing an Invariant error ("Unexpected terminal in optional") instead of the expected Todo error. This caused hard failures instead of graceful bailouts.
The issue occurred because DropManualMemoization and ValidateExhaustiveDependencies ran before BuildReactiveFunction, and encountered `maybe-throw` terminals in their switch statements without a handler, falling through to the invariant case.
Fixed by adding explicit `maybe-throw` cases in both files that throw the proper Todo error with the message "Support value blocks (conditional, logical, optional chaining, etc) within a try/catch statement".
Also renamed the existing bug test to reflect it's now correctly handled:
- error.bug-invariant-unexpected-terminal-in-optional → error.todo-optional-chaining-within-try-catch
Closes #35570
Fixes a longstanding issue where we didn't support code like `useFoo(value?.bar(), value?.bar()) ?? {}` - we would attempt to construct a ReactiveFunction, recursively processing the blocks, but the inner optional `value?.bar()` wouldn't match with what the outer optional was expecting to find. It's a one-line fix!
Note: memoization in the examples is not ideal, but i've confirmed that it is not strictly related to the optional issue.
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.
Mirror of facebook/react#35596
Original author: josephsavona
Fixes a longstanding issue where we didn't support code like
useFoo(value?.bar(), value?.bar()) ?? {}- we would attempt to construct a ReactiveFunction, recursively processing the blocks, but the inner optionalvalue?.bar()wouldn't match with what the outer optional was expecting to find. It's a one-line fix!Note: memoization in the examples is not ideal, but i've confirmed that it is not strictly related to the optional issue.
Stack created with Sapling. Best reviewed with ReviewStack.