Skip to content

installTypesForPackage refactor: Trigger even if resolved to a ".js" file#20353

Merged
2 commits merged into
masterfrom
refactorInstallTypesForPackage_js
Nov 30, 2017
Merged

installTypesForPackage refactor: Trigger even if resolved to a ".js" file#20353
2 commits merged into
masterfrom
refactorInstallTypesForPackage_js

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Nov 29, 2017

Fixes #20087

@ghost ghost requested a review from amcasey November 29, 2017 23:42
// Still offer to install types if it resolved to e.g. a ".js" file.
// `tryGetCodeActionForInstallPackageTypes` will verify that we're looking for a valid package name,
// so the fix won't trigger for imports of ".js" files that couldn't be better replaced by typings.
if (resolvedTo && resolvedTo.extension === Extension.Dts) {
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.

Can it resolve to a ts file and, if so, do we want to suppress the code action in that case?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It shouldn't matter because tryGetCodeActionForInstallPackageTypes will verify that we're looking for a valid package name, which don't contain .ts files when installed, but I've changed this to use extensionIsTypeScript anyway since it's just as easy.

@ghost ghost merged commit 56a046d into master Nov 30, 2017
@ghost ghost deleted the refactorInstallTypesForPackage_js branch November 30, 2017 16:41
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
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.

1 participant