From 7fb1e7092b512ae770ed6b3d942d1869e1e5d9cb Mon Sep 17 00:00:00 2001 From: Amin Pakseresht Date: Wed, 20 Jan 2021 21:26:40 -0500 Subject: [PATCH 1/6] Add AutoImportSuggestions for UMD module export declarations instead of global keywords --- src/services/completions.ts | 16 ++++++++- ...pletionsImport_umdModules1_globalAccess.ts | 30 ++++++++++++++++ ...letionsImport_umdModules2_moduleExports.ts | 34 +++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/completionsImport_umdModules1_globalAccess.ts create mode 100644 tests/cases/fourslash/completionsImport_umdModules2_moduleExports.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index f0ad156956d82..8a0fa7fe0eeed 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -855,6 +855,7 @@ namespace ts.Completions { host: LanguageServiceHost ): CompletionData | Request | undefined { const typeChecker = program.getTypeChecker(); + const compilerOptions = program.getCompilerOptions(); let start = timestamp(); let currentToken = getTokenAtPosition(sourceFile, position); // TODO: GH#15853 @@ -1519,7 +1520,20 @@ namespace ts.Completions { return false; } - symbol = skipAlias(symbol, typeChecker); + // External modules can have global export declarations that will be + // available as global keywords in all scopes. But if the external module + // already has an explicit export and user only wants to user explicit + // module imports then the global keywords will be filtered out so auto + // import suggestions will win in the completion + const symbolOrigin = skipAlias(symbol, typeChecker); + // We only want to filter out the global keywords + if (symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords + && symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions + && !compilerOptions.allowUmdGlobalAccess) { + return false; + } + // Continue with origin symbol + symbol = symbolOrigin; // import m = /**/ <-- It can only access namespace (if typing import = x. this would get member symbols and not namespace) if (isInRightSideOfInternalImportEqualsDeclaration(location)) { diff --git a/tests/cases/fourslash/completionsImport_umdModules1_globalAccess.ts b/tests/cases/fourslash/completionsImport_umdModules1_globalAccess.ts new file mode 100644 index 0000000000000..6ac4e394556ee --- /dev/null +++ b/tests/cases/fourslash/completionsImport_umdModules1_globalAccess.ts @@ -0,0 +1,30 @@ +/// + +// @filename: /package.json +//// { "dependencies": { "@types/classnames": "*" } } + +// @filename: /tsconfig.json +//// { "compilerOptions": { "allowUmdGlobalAccess": true } } + +// @filename: /node_modules/@types/classnames/package.json +//// { "name": "@types/classnames", "types": "index.d.ts" } + +// @filename: /node_modules/@types/classnames/index.d.ts +//// declare const classNames: () => string; +//// export = classNames; +//// export as namespace classNames; + +// @filename: /SomeReactComponent.tsx +//// import * as React from 'react'; +//// +//// const el1 =
foo
; + +goTo.marker("1"); + +verify.completions({ + includes: [{ + name: "classNames", + hasAction: undefined, // Asserts to have no actions + sortText: completion.SortText.GlobalsOrKeywords, + }], +}); diff --git a/tests/cases/fourslash/completionsImport_umdModules2_moduleExports.ts b/tests/cases/fourslash/completionsImport_umdModules2_moduleExports.ts new file mode 100644 index 0000000000000..56db8733cbc49 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_umdModules2_moduleExports.ts @@ -0,0 +1,34 @@ +/// + +// @filename: /package.json +//// { "dependencies": { "@types/classnames": "*" } } + +// @filename: /tsconfig.json +//// {} + +// @filename: /node_modules/@types/classnames/package.json +//// { "name": "@types/classnames", "types": "index.d.ts" } + +// @filename: /node_modules/@types/classnames/index.d.ts +//// declare const classNames: () => string; +//// export = classNames; +//// export as namespace classNames; + +// @filename: /SomeReactComponent.tsx +//// import * as React from 'react'; +//// +//// const el1 =
foo
; + +goTo.marker("1"); + +verify.completions({ + includes: [{ + name: "classNames", + hasAction: true, + source: "/node_modules/@types/classnames/index", + sortText: completion.SortText.AutoImportSuggestions, + }], + preferences: { + includeCompletionsForModuleExports: true, + } +}); From 560b6fcbd58ebd2ef9eda0d2083acc81b72b56dc Mon Sep 17 00:00:00 2001 From: Amin Pakseresht Date: Thu, 21 Jan 2021 21:05:46 -0500 Subject: [PATCH 2/6] Add test for scripts --- ...pletionsImport_umdModules1_globalAccess.ts | 3 ++ .../completionsImport_umdModules3_script.ts | 32 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/cases/fourslash/completionsImport_umdModules3_script.ts diff --git a/tests/cases/fourslash/completionsImport_umdModules1_globalAccess.ts b/tests/cases/fourslash/completionsImport_umdModules1_globalAccess.ts index 6ac4e394556ee..3e2225ec8c12a 100644 --- a/tests/cases/fourslash/completionsImport_umdModules1_globalAccess.ts +++ b/tests/cases/fourslash/completionsImport_umdModules1_globalAccess.ts @@ -27,4 +27,7 @@ verify.completions({ hasAction: undefined, // Asserts to have no actions sortText: completion.SortText.GlobalsOrKeywords, }], + preferences: { + includeCompletionsForModuleExports: true, + } }); diff --git a/tests/cases/fourslash/completionsImport_umdModules3_script.ts b/tests/cases/fourslash/completionsImport_umdModules3_script.ts new file mode 100644 index 0000000000000..ea40cd57ce853 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_umdModules3_script.ts @@ -0,0 +1,32 @@ +/// + +// @filename: /package.json +//// { "dependencies": { "@types/classnames": "*" } } + +// @filename: /tsconfig.json +//// {} + +// @filename: /node_modules/@types/classnames/package.json +//// { "name": "@types/classnames", "types": "index.d.ts" } + +// @filename: /node_modules/@types/classnames/index.d.ts +//// declare const classNames: () => string; +//// export = classNames; +//// export as namespace classNames; + +// @filename: /SomeReactComponent.tsx +//// +//// const el1 =
foo
+ +goTo.marker("1"); + +verify.completions({ + includes: [{ + name: "classNames", + hasAction: undefined, // Asserts to have no actions + sortText: completion.SortText.GlobalsOrKeywords, + }], + preferences: { + includeCompletionsForModuleExports: true, + } +}); From c562092d4b70027daaf5175155e4ecbe96c45713 Mon Sep 17 00:00:00 2001 From: Amin Pakseresht Date: Thu, 21 Jan 2021 21:08:26 -0500 Subject: [PATCH 3/6] Add more comments --- src/services/completions.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/completions.ts b/src/services/completions.ts index 8a0fa7fe0eeed..4a599b0bd38a5 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1527,6 +1527,7 @@ namespace ts.Completions { // import suggestions will win in the completion const symbolOrigin = skipAlias(symbol, typeChecker); // We only want to filter out the global keywords + // Auto Imports are not available for scripts so this conditional is always false if (symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords && symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions && !compilerOptions.allowUmdGlobalAccess) { From 63c89ad83738b58dad11e51fe4fc10007c6853a2 Mon Sep 17 00:00:00 2001 From: Amin Pakseresht Date: Fri, 22 Jan 2021 17:25:18 -0500 Subject: [PATCH 4/6] Provide auto import suggestion only for modules and not scripts --- src/services/completions.ts | 4 +++- tests/cases/fourslash/completionsImport_umdModules3_script.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 4a599b0bd38a5..e92cb3539b9c5 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1506,6 +1506,7 @@ namespace ts.Completions { } const variableDeclaration = getVariableDeclaration(location); + const isFileSourceAModule = !!location.getSourceFile().externalModuleIndicator; filterMutate(symbols, symbol => { if (!isSourceFile(location)) { @@ -1528,7 +1529,8 @@ namespace ts.Completions { const symbolOrigin = skipAlias(symbol, typeChecker); // We only want to filter out the global keywords // Auto Imports are not available for scripts so this conditional is always false - if (symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords + if (isFileSourceAModule + && symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords && symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions && !compilerOptions.allowUmdGlobalAccess) { return false; diff --git a/tests/cases/fourslash/completionsImport_umdModules3_script.ts b/tests/cases/fourslash/completionsImport_umdModules3_script.ts index ea40cd57ce853..cdcf6da2287ea 100644 --- a/tests/cases/fourslash/completionsImport_umdModules3_script.ts +++ b/tests/cases/fourslash/completionsImport_umdModules3_script.ts @@ -4,7 +4,7 @@ //// { "dependencies": { "@types/classnames": "*" } } // @filename: /tsconfig.json -//// {} +//// { "compilerOptions": { "module": "es2015" }} // @filename: /node_modules/@types/classnames/package.json //// { "name": "@types/classnames", "types": "index.d.ts" } From 12e0c607b87552fbfa28f8a980a0273c9c825eba Mon Sep 17 00:00:00 2001 From: Amin Pakseresht Date: Fri, 22 Jan 2021 18:13:54 -0500 Subject: [PATCH 5/6] PR review #1 --- src/services/completions.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index e92cb3539b9c5..5e08e98c2df34 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1506,7 +1506,7 @@ namespace ts.Completions { } const variableDeclaration = getVariableDeclaration(location); - const isFileSourceAModule = !!location.getSourceFile().externalModuleIndicator; + const isFileSourceAModule = !!sourceFile.externalModuleIndicator; filterMutate(symbols, symbol => { if (!isSourceFile(location)) { @@ -1530,9 +1530,9 @@ namespace ts.Completions { // We only want to filter out the global keywords // Auto Imports are not available for scripts so this conditional is always false if (isFileSourceAModule + && !compilerOptions.allowUmdGlobalAccess && symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords - && symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions - && !compilerOptions.allowUmdGlobalAccess) { + && symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions) { return false; } // Continue with origin symbol From ab4dab1c3c5970d3cb6ae3e96babd5b42e1c55de Mon Sep 17 00:00:00 2001 From: Amin Pakseresht Date: Fri, 22 Jan 2021 18:14:41 -0500 Subject: [PATCH 6/6] PR review #1 --- src/services/completions.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 5e08e98c2df34..e7b1a8e86bdd4 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1506,7 +1506,6 @@ namespace ts.Completions { } const variableDeclaration = getVariableDeclaration(location); - const isFileSourceAModule = !!sourceFile.externalModuleIndicator; filterMutate(symbols, symbol => { if (!isSourceFile(location)) { @@ -1529,7 +1528,7 @@ namespace ts.Completions { const symbolOrigin = skipAlias(symbol, typeChecker); // We only want to filter out the global keywords // Auto Imports are not available for scripts so this conditional is always false - if (isFileSourceAModule + if (!!sourceFile.externalModuleIndicator && !compilerOptions.allowUmdGlobalAccess && symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords && symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions) {