Skip to content

fix: arrow/async in conditional expressions for TS#17572

Open
liuxingbaoyu wants to merge 2 commits intobabel:mainfrom
liuxingbaoyu:fix-ts-cond-arrow
Open

fix: arrow/async in conditional expressions for TS#17572
liuxingbaoyu wants to merge 2 commits intobabel:mainfrom
liuxingbaoyu:fix-ts-cond-arrow

Conversation

@liuxingbaoyu
Copy link
Copy Markdown
Member

Q                       A
Fixed Issues? Fixes #17547
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?
Documentation PR Link
Any Dependency Changes?
License MIT

I initially tried to port the TS implementation, but failed.
Maybe I should try again.

@liuxingbaoyu liuxingbaoyu changed the title fix: arrows/async in conditional expressions for TS fix: arrow/async in conditional expressions for TS Oct 25, 2025
@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Oct 25, 2025

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/60120

Comment on lines +3416 to +3426
if (!this.eat(tt.colon)) {
state.noArrowAt = this.state.noArrowAt;
if (state.noArrowAt.length <= 3) {
state.noArrowAt = [state.noArrowAt[0]];
} else {
state.noArrowAt = state.noArrowAt.slice(3);
}
this.state = state;
node.consequent = this.parseMaybeAssignAllowIn();
this.expect(tt.colon);
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was inspired by the Flow plugin, thanks to @nicolo-ribaudo.
The current implementation doesn't look reliable at all, but it passes all the tests.🤦‍♂️
If we try them one by one, it will be more reliable, but slower.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Oct 25, 2025

Open in StackBlitz

commit: 7414d5d

@@ -0,0 +1 @@
true ? async (): true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Although it is valid JavaScript, tsc does not support it either:

https://www.typescriptlang.org/play/?#code/C4JwrgpgBA-FCGBnAngOwMZQBQEoBcUokA3EA

Could we skip this test temporarily and enable it when we align to the tsc behaviour?

Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

I don;t love this, but it looks ok. It'd be great to see some benchmarks, but we should prioritize correctness over performance anyway.

veeceey added a commit to veeceey/babel that referenced this pull request Feb 16, 2026
…s from babel#17572

Since the `?` token is already verified at the start of parseConditional,
use `this.next()` to consume it instead of redundantly checking with
`this.expect()`. Also add additional conditional expression test fixtures
ported from babel#17572 to improve coverage of arrow/async ambiguity edge cases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
veeceey added a commit to veeceey/babel that referenced this pull request Feb 16, 2026
These test cases document known limitations of the current fix:
- arrow-ambiguity, arrow-like, arrow-param: Non-async arrow disambiguation
  in ternary expressions (out of scope for this PR)
- async-call, async-call-3: Edge cases where async call parsing interacts
  with nested arrow/paren contexts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
JLHwung pushed a commit that referenced this pull request Feb 23, 2026
…17799)

* Fix TypeScript parser failing on async calls inside ternary consequent

When the TypeScript plugin encounters `async(a)` followed by `:` inside
a ternary consequent, it incorrectly treats `:` as a return type annotation
for an async arrow function, rather than the ternary separator.

For example, `true ? async(a) : b => b` should parse as a ternary with
`async(a)` as the consequent (a call expression) and `b => b` as the
alternate (an arrow function). Instead, babel tried to parse
`async(a): b => b` as an async arrow with return type `b`.

This commit:
- Adds an `inConditionalConsequent` state flag set during ternary
  consequent parsing
- When the flag is active, `shouldParseAsyncArrow` no longer treats
  `:` as a return type annotation start
- Resets the flag inside parenthesized expressions, so
  `a ? (async(b): T => body) : c` still works correctly

Fixes #17547

* Address review: use this.next() instead of this.expect() and add tests from #17572

Since the `?` token is already verified at the start of parseConditional,
use `this.next()` to consume it instead of redundantly checking with
`this.expect()`. Also add additional conditional expression test fixtures
ported from #17572 to improve coverage of arrow/async ambiguity edge cases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add expected output files for test fixtures from #17572

These test cases document known limitations of the current fix:
- arrow-ambiguity, arrow-like, arrow-param: Non-async arrow disambiguation
  in ternary expressions (out of scope for this PR)
- async-call, async-call-3: Edge cases where async call parsing interacts
  with nested arrow/paren contexts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: replace throws expectations with output.json for async-call fixtures

The async-call and async-call-3 test fixtures had "throws" directives in
options.json expecting parse errors, but the parser fix now correctly
handles these cases. Replace with output.json files containing expected ASTs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[preset-typescript] Ternaries that call a function named async fail to parse

4 participants