Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript: Support conditional types syntax #7404

Merged
merged 1 commit into from Feb 24, 2018
Merged

TypeScript: Support conditional types syntax #7404

merged 1 commit into from Feb 24, 2018

Conversation

ghost
Copy link

@ghost ghost commented Feb 20, 2018

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

@JamesHenry This changes the AST format. CC @DanielRosenwasser for review.
Supports the syntax from microsoft/TypeScript#21316 and microsoft/TypeScript#21496. Now you can write type Element<T> = T extends (infer U)[] ? U : T;.

@babel-bot
Copy link
Collaborator

babel-bot commented Feb 20, 2018

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

@@ -1875,6 +1875,22 @@ Aliases: `TSTypeElement`

---

### tSConditionalType
Copy link

Choose a reason for hiding this comment

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

tsConditionalType?

Copy link
Member

Choose a reason for hiding this comment

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

It's generated, and should get fixed after we land: #6993

Copy link
Member

Choose a reason for hiding this comment

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

I'm cool with landing it, we can first make a change to https://github.com/babel/babel-upgrade for it pretty easily

@JamesHenry
Copy link
Member

Thanks for the heads up, @andy-ms! This is really cool

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

LGTM, but will wait for TS team to weigh in!

Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me.

Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me.

@DanielRosenwasser DanielRosenwasser mentioned this pull request Feb 24, 2018
3 tasks
@nicolo-ribaudo
Copy link
Member

Thank you!

@nicolo-ribaudo nicolo-ribaudo merged commit 6f6c8da into babel:master Feb 24, 2018
benjamn added a commit to benjamn/ast-types that referenced this pull request Feb 24, 2018
benjamn added a commit to benjamn/recast that referenced this pull request Feb 24, 2018
Although (infer T) must frequently be parenthesized to avoid ambiguity,
Recast does not have to encode that logic into FastPath#needsParens,
because Babylon wraps such types with TSParenthesizedType. Cool!

babel/babel#7404
benjamn/ast-types@da0367b
microsoft/TypeScript#21316
microsoft/TypeScript#21496
@benjamn
Copy link
Contributor

benjamn commented Feb 24, 2018

Working in ast-types@0.11.2 and recast@0.14.4!

@ghost ghost deleted the ts-conditional-types branch February 26, 2018 15:18
benjamn added a commit to benjamn/babel that referenced this pull request May 18, 2018
Follow-up to babel#7404

There is no TypeParameter type in the Babylon TypeScript AST hierarchy.
Flow does have a TypeParameter type, but it should not be confused with
the TypeScript TSTypeParameter, since Flow !== TypeScript, and the types
have totally different fields. Instead, the .typeParameter.type of a
Babylon-parsed TSInferType node should be TSTypeParameter.

It would probably be fine to leave the declared type of the TSInferType
.typeParameter field as TSType instead of TSTypeParameter, but the more
specific TSTypeParameter type should be safe/correct, since the TypeScript
parseInferType function always uses SyntaxKind.TypeParameter:
https://github.com/Microsoft/TypeScript/blob/66d6e5e6e007ce3ac70b7371c3e7ac46a99a0fcd/src/compiler/parser.ts#L3006

I noticed this typo because it has been causing ast-types test failures
lately, e.g. https://travis-ci.org/benjamn/ast-types/jobs/369634972
existentialism pushed a commit that referenced this pull request May 18, 2018
Follow-up to #7404

There is no TypeParameter type in the Babylon TypeScript AST hierarchy.
Flow does have a TypeParameter type, but it should not be confused with
the TypeScript TSTypeParameter, since Flow !== TypeScript, and the types
have totally different fields. Instead, the .typeParameter.type of a
Babylon-parsed TSInferType node should be TSTypeParameter.

It would probably be fine to leave the declared type of the TSInferType
.typeParameter field as TSType instead of TSTypeParameter, but the more
specific TSTypeParameter type should be safe/correct, since the TypeScript
parseInferType function always uses SyntaxKind.TypeParameter:
https://github.com/Microsoft/TypeScript/blob/66d6e5e6e007ce3ac70b7371c3e7ac46a99a0fcd/src/compiler/parser.ts#L3006

I noticed this typo because it has been causing ast-types test failures
lately, e.g. https://travis-ci.org/benjamn/ast-types/jobs/369634972
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants