TypeScript: Fix TSInferType .typeParameter type. #7967
Merged
Conversation
This file contains 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
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
|
@andy-ms does this seem reasonable? |
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8015/ |
benjamn
added a commit
to benjamn/ast-types
that referenced
this pull request
May 18, 2018
That is, until babel/babel#7967 is released. This should fix the tests that have been failing on PRs lately.
ghost
approved these changes
May 18, 2018
existentialism
approved these changes
May 18, 2018
existentialism
added
the
PR: Bug Fix 🐛
bnjmnt4n
added a commit
to bnjmnt4n/ast-types
that referenced
this pull request
Feb 6, 2019
babel/babel#7967 has been released since @babel/parser@v7.0.0-beta.48. Ref. 9777ef7
benjamn
pushed a commit
to benjamn/ast-types
that referenced
this pull request
Feb 6, 2019
babel/babel#7967 has been released since @babel/parser@v7.0.0-beta.48. Ref. 9777ef7
lock
bot
added
the
outdated
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
ast-typestest failures, e.g. https://travis-ci.org/benjamn/ast-types/jobs/369634972TypeParameterFollow-up to #7404
There is no
TypeParametertype in the Babylon TypeScript AST hierarchy. Flow does have aTypeParametertype, but it should not be confused with the TypeScriptTSTypeParameter, since Flow !== TypeScript, and the types have totally different fields. Instead, the.typeParameter.typeof a Babylon-parsedTSInferTypenode should beTSTypeParameter.It would probably be fine to leave the declared type of the
TSInferType.typeParameterfield asTSTypeinstead ofTSTypeParameter, but the more specificTSTypeParametertype should be safe/correct, since the TypeScriptparseInferTypefunction always usesSyntaxKind.TypeParameter.I noticed this typo because it has been causing
ast-typestest failures lately, e.g. https://travis-ci.org/benjamn/ast-types/jobs/369634972