Conversation
Greptile SummaryFixed invariant error when optional chaining appears within try/catch blocks by adding explicit
Confidence Score: 5/5
Important Files Changed
|
There was a problem hiding this comment.
Additional Comments (1)
-
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-unexpected-terminal-in-optional.expect.md, line 22 (link)logic: test expectation file should be updated to reflect the new Todo error instead of the Invariant error
the PR description states the test was renamed from
error.bug-invariant-unexpected-terminal-in-optionaltoerror.todo-optional-chaining-within-try-catchand the expectation should now show a Todo error with message "Support value blocks (conditional, logical, optional chaining, etc) within a try/catch statement" instead of the invariant error shown here
120 files reviewed, 1 comment
| case 'maybe-throw': { | ||
| CompilerError.throwTodo({ | ||
| reason: `Support value blocks (conditional, logical, optional chaining, etc) within a try/catch statement`, | ||
| description: null, | ||
| loc: terminal.loc, | ||
| suggestions: null, | ||
| }); | ||
| } |
There was a problem hiding this comment.
style: missing break statement after throwTodo() - while this doesn't cause issues since throwTodo never returns, adding a break would follow switch case best practices and improve code clarity
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
Line: 586:593
Comment:
**style:** missing `break` statement after `throwTodo()` - while this doesn't cause issues since `throwTodo` never returns, adding a `break` would follow switch case best practices and improve code clarity
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| case 'maybe-throw': { | ||
| CompilerError.throwTodo({ | ||
| reason: `Support value blocks (conditional, logical, optional chaining, etc) within a try/catch statement`, | ||
| description: null, | ||
| loc: terminal.loc, | ||
| suggestions: null, | ||
| }); | ||
| } |
There was a problem hiding this comment.
style: missing break statement after throwTodo() - while this doesn't cause issues since throwTodo never returns, adding a break would follow switch case best practices and improve code clarity
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts
Line: 1019:1026
Comment:
**style:** missing `break` statement after `throwTodo()` - while this doesn't cause issues since `throwTodo` never returns, adding a `break` would follow switch case best practices and improve code clarity
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.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
Mirror of facebook/react#35573
Original author: josephsavona
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-throwterminals in their switch statements without a handler, falling through to the invariant case.Fixed by adding explicit
maybe-throwcases 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:
Closes #35570
Stack created with Sapling. Best reviewed with ReviewStack.