add assertions signature for TypeScript#10543
Conversation
| visitor: ["parameterName", "typeAnnotation"], | ||
| visitor: ["parameterName", "typeAnnotation", "assertsModifier"], | ||
| fields: { | ||
| assertsModifier: validateOptionalType("TSAssertsKeyword"), |
There was a problem hiding this comment.
This should be a boolean, since the only needed information is if it is present or not.
There was a problem hiding this comment.
I was following the TypeScript AST.
But I understand Babel AST doesn't have to be the same as Typescript AST.
| const t: N.TsTypeAnnotation = this.startNode(); | ||
| this.expect(returnToken); | ||
|
|
||
| const assertsModifier = this.tsTryParse( |
There was a problem hiding this comment.
Note that this could be parser without using tryParse: it would be possible to parse the assert identifier and then convert it to a type annotation if the next token is not another identifier.
On the other hand, we use tryParse everywhere when parsing TypeScript. I think that it's better to be consistent here, and then the whole TS parser can be optimized on other PRs.
| "typeAnnotation": { | ||
| "type": "TSTypePredicate", | ||
| "start": 10, | ||
| "start": 8, |
|
The CircleCI failure is unrelated and already fixed on master. |
544d494 to
128cd3d
Compare
eventualbuddha
left a comment
There was a problem hiding this comment.
(I'm reviewing this because of this: https://babeljs.slack.com/archives/C062RC35M/p1571345121030900)
Having read the blog post on TypeScript 3.7 and played around with it in the local REPL, this seems sane to me.
|
Hi there! We on Could we look to align the AST representations? I got alerted to this via typescript-eslint/typescript-eslint#1158. |
|
I'm 100% ok with it. We are going to release a new Babel version soon; would you be able to prepare a quick PR? |
I'll do it |
|
@thorn0 thanks! |
Should I make changes to the docs manually, or wait for babel/website#2080?