Use jsdoc aliases if visible when printing types - #24153
Use jsdoc aliases if visible when printing types#24153Wesley Wigham (weswigham) merged 4 commits into
Conversation
Sorry, something went wrong.
Nathan Shively-Sanders (sandersn)
left a comment
There was a problem hiding this comment.
- This needs to handle @callback too.
- getEffectiveTypeParameterDeclarations needs a better implementation for jsdoc type aliases.
Sorry, something went wrong.
|
|
||
| function determineIfDeclarationIsVisible() { | ||
| switch (node.kind) { | ||
| case SyntaxKind.JSDocTypedefTag: |
There was a problem hiding this comment.
Need to handle SyntaxKind.CallbackTag too
Sorry, something went wrong.
There was a problem hiding this comment.
#23947 isn't merged yet
Sorry, something went wrong.
| node.kind === SyntaxKind.ClassExpression || node.kind === SyntaxKind.TypeAliasDeclaration) { | ||
| const declaration = <InterfaceDeclaration | TypeAliasDeclaration>node; | ||
| node.kind === SyntaxKind.ClassExpression || node.kind === SyntaxKind.TypeAliasDeclaration || node.kind === SyntaxKind.JSDocTypedefTag) { | ||
| const declaration = <InterfaceDeclaration | TypeAliasDeclaration | JSDocTypedefTag>node; |
There was a problem hiding this comment.
same. There is a predicate isJSDocTypeAlias that should maybe just be isTypeAlias and include SyntaxKind.TypeAliasDeclaration.
Sorry, something went wrong.
There was a problem hiding this comment.
#23947 isn't merged yet
Sorry, something went wrong.
| */ | ||
| export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters) { | ||
| return node.typeParameters || (isInJavaScriptFile(node) ? getJSDocTypeParameterDeclarations(node) : undefined); | ||
| export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters | JSDocTypedefTag) { |
There was a problem hiding this comment.
there are a few other places that we get type parameter declarations of typedef and callback tags. They explicitly do not use getEffectiveTypeParameterDeclarations because it incorrectly (1) finds the host and (2) looks for @template in any jsdoc comment. Jsdoc type aliases only look for @template in their containing comment.
That said, it's probably the Right Thing to make getEffectiveTypeParameterDeclarations handle jsdoc type aliases, and have everybody call it.
Sorry, something went wrong.
There was a problem hiding this comment.
That said, it's probably the Right Thing to make getEffectiveTypeParameterDeclarations handle jsdoc type aliases, and have everybody call it.
Do you not have such a fix in #23947 ?
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, along with the astonishing ability to forget what I did two weeks ago!
Sorry, something went wrong.
|
Nathan Shively-Sanders (@sandersn) Two of your comments don't seem to apply until your PR adding callback tag support is merged? How's this look as is? |
Sorry, something went wrong.
|
Well, good, except that 50% of the code will have to change after it's merged and will have lurking type parameter lookup bugs if it's not. If you merge this, can you open a bug either on me or on you explaining that @callback won't have this fix, once it's merged? |
Sorry, something went wrong.
|
I'm not particularly inclined to duplicate your work fixing jsdoc alias type parameters (which'd have issues merging anyway), nor can I implement the fixes for @callback until we actually support @callback; so notes on you it is! |
Sorry, something went wrong.
Fixes #24140
This avoids deeply structurally printing massive types which may have been defined in jsdoc. 😉