Use jsdoc aliases if visible when printing types#24153
Use jsdoc aliases if visible when printing types#24153weswigham merged 4 commits intomicrosoft:masterfrom
Conversation
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
- This needs to handle
@callbacktoo. - getEffectiveTypeParameterDeclarations needs a better implementation for jsdoc type aliases.
|
|
||
| function determineIfDeclarationIsVisible() { | ||
| switch (node.kind) { | ||
| case SyntaxKind.JSDocTypedefTag: |
There was a problem hiding this comment.
Need to handle SyntaxKind.CallbackTag too
| 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.
| */ | ||
| 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.
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 ?
There was a problem hiding this comment.
Yes, along with the astonishing ability to forget what I did two weeks ago!
|
@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? |
|
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 |
|
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 |
2 participants
Fixes #24140
This avoids deeply structurally printing massive types which may have been defined in jsdoc. 😉