Skip to content

Jsdoc @template in scope as type parameter#16463

Merged
sandersn merged 5 commits into
masterfrom
jsdoc-@template-in-scope-as-type-parameter
Jun 12, 2017
Merged

Jsdoc @template in scope as type parameter#16463
sandersn merged 5 commits into
masterfrom
jsdoc-@template-in-scope-as-type-parameter

Conversation

@sandersn
Copy link
Copy Markdown
Member

The code to make sure that type parameters are in scope for instantiation previously ignored type parameters created by @template. Now it correctly says that they are in scope.

Fixes #15177, which showed that functions returned from a generic function were not instantiated. Turns out this was because the type parameters were not recognised as in scope, so the instantiation algorithm skipped them.

sandersn added 2 commits June 12, 2017 13:55
The test to make sure that type parameters are in scope for
instantiation previously ignored type parameters created by `@template`.
Now it correctly says that they are in scope.
Comment thread src/compiler/checker.ts Outdated
const declaration = node as DeclarationWithTypeParameters;
if (declaration.typeParameters) {
for (const d of declaration.typeParameters) {
const typeParameters = declaration.typeParameters || getTypeParametersFromJSDocTemplate(declaration);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rbuckton has been using a pattern to use of getEffectiveType* to get the TS nodes, or the ones from JSDoc. we can follow the same here and use getEffectiveTypeParameters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and use that in both places where you need the type parametes

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out that pattern. Done.

sandersn added 2 commits June 12, 2017 14:23
Create getEffectiveTypeParameterDeclarations in utilities.ts
Comment thread src/compiler/utilities.ts
/**
* Gets the effective return type annotation of a signature. If the node was parsed in a
* JavaScript file, gets the return type annotation from JSDoc.
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you see any perf impact of having this function here vs in checker.ts?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there normally a perf impact for moving functions to utilities? I didn't expect anything. But I can check to see whether anything shows up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are emitted to ts. instead of the direct function call.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there is no perf impact. Monaco gets 0.05s slower, TFS gets 0.04s faster on average over 20 iterations. But neither number is statistically significant.

@sandersn sandersn merged commit 3d8cf62 into master Jun 12, 2017
@sandersn sandersn deleted the jsdoc-@template-in-scope-as-type-parameter branch June 12, 2017 23:07
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants