Skip to content

Enable member completions for import types - #23085

Merged
Wesley Wigham (weswigham) merged 6 commits into
microsoft:masterfrom
weswigham:import-types-completions
Apr 3, 2018
Merged

Enable member completions for import types#23085
Wesley Wigham (weswigham) merged 6 commits into
microsoft:masterfrom
weswigham:import-types-completions

Conversation

@weswigham

Copy link
Copy Markdown
Member

Fixes one of the points brought up in this comment.

Comment threadsrc/services/completions.ts Outdated
return typeChecker.isValidPropertyAccess(<PropertyAccessExpression>(node.parent), symbol.name);
}
else {
if (!(node as ImportTypeNode).isTypeOf) return false;

@ghostDeleted user (ghost)Apr 2, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use a && b instead of if (!a) return false; return b;

Comment threadsrc/services/completions.ts Outdated
return typeChecker.isValidPropertyAccess(<PropertyAccessExpression>(node.parent), symbol.name);
}
else {
return !(node as ImportTypeNode).isTypeOf && !!(symbol.flags & SymbolFlags.Value);

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.

Dont you still need typeChecker.isValidPropertyAccess especially checkPropertyAccessibility needs to be done to avoid not showing non exported stuff?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Non-exported symbols aren't returned by getExportsOfModule - except for class statics! I'll add a test.

@weswigham

Copy link
Copy Markdown
MemberAuthor

Sheetal Nandi (@sheetalkamat) Can I get a rereview?

@weswigham
Wesley Wigham (weswigham) merged commit a81a645 into microsoft:masterApr 3, 2018
@microsoftMicrosoft (microsoft) locked and limited conversation to collaborators Jul 25, 2018
Sign up for freeto 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

@weswigham@mhegazy@sheetalkamat