-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Allow JSDoc casts of parenthesized expressions in JS #17211
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, although I have one question and two nits before you merge.
@@ -17731,6 +17735,20 @@ namespace ts { | |||
return type; | |||
} | |||
|
|||
function checkParenthesizedExpression(node: ParenthesizedExpression, checkMode?: CheckMode): Type { | |||
if (isInJavaScriptFile(node)) { | |||
if (node.jsDoc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not combine these predicates?
function checkParenthesizedExpression(node: ParenthesizedExpression, checkMode?: CheckMode): Type { | ||
if (isInJavaScriptFile(node)) { | ||
if (node.jsDoc) { | ||
const typecasts = flatten<JSDocTag>(map(node.jsDoc, doc => filter(doc.tags, tag => tag.kind === SyntaxKind.JSDocTypeTag))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use flatMap
instead? filter
should always return an array so you don't need the if (isArray ...
check in flatten
.
|
||
class SomeBase { | ||
constructor() { | ||
this.p = 42; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests cases don't have type annotations. But based on your follow-up comment in the associated bug, the syntax
/** @type {number} */
this.p = 42;
should also work. I just want to confirm that this is not addressed in this PR.
Parse jsdoc with normal TS type parser
* Minor cleanups in builder * Use enumerateInsertsAndDeletes
…the LHS of a VariableDeclaration (#17205)
… ones (#17179) * Reuse stored types for any[] and Promise<any> instead of creating new ones * Don't store anyPromiseType
`readFile` may return undefined
* Issue template: Recommand to run `tsc` directly * List some common build tools
9a29ac4
to
b5ec445
Compare
I'll reopen this against master (with feedback integrated) shortly. |
* Allow jsdoc casts of parenthesized expressions * Feedback from #17211
Fixes #5158
Based on #17176, since that changes some of the machinery that could have affected this.