Fixed a crash on JSX emit trying to emit unexpectedly recovered BinaryExpression in a JSX attribute - #64470
Conversation
…xpressions parseJsxAttributeValue called parseJsxElementOrSelfClosingElementOrFragment with mustBeUnary=false, so the "JSX expressions must have one parent element" recovery could return a comma BinaryExpression as a JsxAttribute initializer, violating the JsxAttributeValue contract and crashing the react/react-jsx transforms. Fixes microsoft#64458
typescript-automation
Bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
| return p.parseJsxElementOrSelfClosingElementOrFragment(true /*inExpressionContext*/, -1, nil, false) | ||
| // An attribute value must be a single JsxAttributeValue, so don't allow the sibling-element | ||
| // recovery to wrap it in a synthetic binary expression. | ||
| return p.parseJsxElementOrSelfClosingElementOrFragment(true /*inExpressionContext*/, -1 /*topInvalidNodePosition*/, nil /*openingTag*/, true /*mustBeUnary*/) |
There was a problem hiding this comment.
I think it's best to handle this here - where the bad data was created. All of the other code just doesn't expect BinaryExpression here given:
type JsxAttributeValue = StringLiteral | JsxExpression | JsxElement | JsxSelfClosingElement | JsxFragmentThere was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The targeted parser correction preserves AST invariants and includes relevant regression baselines.
Review effort: Balanced
Findings: None
What changed in this PR
Prevents JSX emit crashes by disallowing synthetic binary-expression recovery within JSX attribute values.
Changes:
- Enforces the
JsxAttributeValueAST invariant during parser recovery. - Adds regression coverage for classic and automatic JSX emit.
- Updates affected diagnostic and emit baselines.
| File | Description |
|---|---|
tsc/internal/parser/parser.go |
Restricts sibling-element recovery in attribute values. |
tsc/testdata/tests/cases/compiler/jsxAttributeInitializerMultipleElementsNoCrash.tsx |
Adds the crash regression case. |
tsc/testdata/baselines/reference/compiler/jsxAttributeValueBinaryExpression.js |
Updates existing emit expectations. |
tsc/testdata/baselines/reference/compiler/jsxAttributeValueBinaryExpression.errors.txt |
Updates existing diagnostics. |
tsc/testdata/baselines/reference/compiler/jsxAttributeInitializerMultipleElementsNoCrash(jsx=react).js |
Records classic JSX emit. |
tsc/testdata/baselines/reference/compiler/jsxAttributeInitializerMultipleElementsNoCrash(jsx=react).errors.txt |
Records classic JSX diagnostics. |
tsc/testdata/baselines/reference/compiler/jsxAttributeInitializerMultipleElementsNoCrash(jsx=react-jsx).js |
Records automatic JSX emit. |
tsc/testdata/baselines/reference/compiler/jsxAttributeInitializerMultipleElementsNoCrash(jsx=react-jsx).errors.txt |
Records automatic JSX diagnostics. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
TypeScript Bot (@typescript-bot) user test this |
|
Jake Bailey (@jakebailey) Here are the results of running the user tests with tsc comparing Everything looks good! |
|
Jake Bailey (@jakebailey) Here are the results of running the top 1000 repos with tsc comparing Everything looks good! |
fixes a crash reported here #64458 (comment)