Skip to content

tsserver: get candidates from <K extends keyof Foo> (fix #12536)#12543

Merged
mhegazy merged 3 commits into
microsoft:masterfrom
wonderful-panda:fix-12536
Dec 30, 2016
Merged

tsserver: get candidates from <K extends keyof Foo> (fix #12536)#12543
mhegazy merged 3 commits into
microsoft:masterfrom
wonderful-panda:fix-12536

Conversation

@wonderful-panda
Copy link
Copy Markdown
Contributor

@wonderful-panda wonderful-panda commented Nov 28, 2016

Fixes #12536

@msftclas
Copy link
Copy Markdown

Hi @wonderful-panda, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

}

function addStringLiteralCompletionsFromType(type: Type, result: CompletionEntry[]): void {
if (type && type.flags & TypeFlags.TypeParameter) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you get the type constraint and the type is a keyof type, where is that handled below?

Comment thread src/services/completions.ts Outdated

function addStringLiteralCompletionsFromType(type: Type, result: CompletionEntry[]): void {
if (type && type.flags & TypeFlags.TypeParameter) {
type = (<TypeParameter>type).constraint;
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.

you should use checker::getApparentType here instead. it does some more than just checking the constraint.

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.

to do that, you will need to expose this method on TypeChecker interface.

Copy link
Copy Markdown
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

comments listed erlier

@wonderful-panda
Copy link
Copy Markdown
Contributor Author

Thank you for the review. I've pushed new version.

@mhegazy mhegazy merged commit 6418c2f into microsoft:master Dec 30, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

4 participants