{{ message }}
Repository navigation
Fix parsing of arrow function params inside ternaries with Flow - #18240
Merged
Merged
Conversation
…l consequents
Disclaimer: This commit description was generated by an LLM (Claude).
With the `flow` plugin, the params of an arrow at the start of a conditional consequent are not converted to patterns when the arrow is parsed, so that `a ? (b): c => d` can be disambiguated afterwards in `parseConditional`. `getArrowLikeExpressions` then converts them, but it skipped arrows with a block body entirely. As a result:
- The params of a block-bodied arrow in that position stayed as expressions: `ObjectExpression`, `ArrayExpression`, `AssignmentExpression` and `TypeCastExpression`, and `SpreadElement` for async arrows. For example, in `a ? async ({ b }) => {} : c` the param is an `ObjectExpression`. The params were also never validated, so `a ? ({ b, b }) => {} : c` and `a ? async (...b = 1) => {} : c` parsed without errors, while `a ? ({ b, b }) => b : c` throws `Argument name clash`.
- A block-bodied arrow with a return type was never considered for re-parsing, so `a ? (b) : c => {};` threw `Unexpected token, expected ":"`, although `a ? (b) : c => d;` parses as `a ? b : (c => d)`.
This is a regression from babel#16425, first released in 7.24.5: it added the `node.body.type !== "BlockStatement"` condition to narrow `node.body` for `stack.push(node.body)`, which also stopped block-bodied arrows from being validated or collected. 7.24.4 handles all of the cases above.
Block-bodied arrows are now validated, or collected as possibly ambiguous, in the same way as arrows with an expression body. Only the traversal into the body is still limited to expression bodies, since a block body can't contain the conditional's `:`.
Found via react/metro#1725, where the `SpreadElement` param crashed a downstream Babel plugin.
Tests: added `flow/arrows-in-ternaries/block-body-params`, `flow/arrows-in-ternaries/block-body-invalid-params` and `flow/arrows-in-ternaries/block-body-return-type-like`. `yarn jest packages/babel-parser` and `yarn jest flow` pass, with and without `BABEL_8_BREAKING=true`.
Thanks for opening this pull request! 🎉We really appreciate you taking the time to contribute, @robhogan. A maintainer will take a look as soon as they can. In the meantime, please make sure that:
If anything needs adjusting we'll leave comments here. Thanks again! |
Collaborator
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/62133 |
|
commit: |
liuxingbaoyu
approved these changes
Sep 15, 2026
Member
|
Thanks! |
Contributor
Author
|
Thanks everyone. Is there a process / anything I can help with to pick this back to Babel 7? (We're hoping to upgrade to v8 but not quite there yet) |
Member
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.

The
flowplugin must disambiguate between a ternary:, and:specifying a type. One case of this is an arrow function within a ternary consequent (true branch) e.g:To do this, the params of an arrow at the start of a ternary consequent are not converted to patterns when the arrow is parsed, so that the result can be disambiguated afterwards in
parseConditional.getArrowLikeExpressionsshould handle that, but it currently skips arrows with a block body entirely - that was actually a regression introduced during type coverage improvements (#16425, released 7.24.5), probably because it looked like a necessary refinement:https://github.com/babel/babel/pull/16425/changes#diff-53c81cc57639357b0d9cca6420b37ecf42e39930e60c0bf6d5b16473bf78e129R2148
That bogus constraint had three symptoms:
The main one: params of block-bodied arrow in that position stayed as expressions:
ObjectExpression,ArrayExpression,AssignmentExpression, andSpreadElementfor async arrows, whereas they should beObjectPattern,ArrayPattern,AssignmentPatternandRestElement, respectively. For example, ina ? async ({ b }) => {} : cthe param is anObjectExpression, should beObjectPattern.The params were also never validated, so
a ? ({ b, b }) => {} : canda ? async (...b = 1) => {} : cparsed without errors, whilea ? ({ b, b }) => b : cthrowsArgument name clash.A block-bodied arrow with a return type was never considered for re-parsing, so
a ? (b) : c => {};threwUnexpected token, expected ":", althougha ? (b) : c => d;parses asa ? b : (c => d).With this PR, block-bodied arrows are now validated, or collected as possibly ambiguous, in the same way as arrows with an expression body. Only the traversal into the body (the
stack.push) is still limited to expression bodies, since a block body can't contain the conditional's:.Found via a report to Metro react/metro#1725, where investigation showed the
SpreadElementparam crashed a downstream Expo Babel plugin.