From 337a2cbea368792804c9e798615073a3ebe54503 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 20 Jul 2021 13:53:37 -0700 Subject: [PATCH] Fix import statement completions for export= in JS files --- src/services/codefixes/importFixes.ts | 24 +++++++----- src/services/completions.ts | 4 +- .../importStatementCompletions_js.ts | 35 ++++++++++++++++++ .../importStatementCompletions_js2.ts | 37 +++++++++++++++++++ 4 files changed, 89 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/importStatementCompletions_js.ts create mode 100644 tests/cases/fourslash/importStatementCompletions_js2.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 0bf0dc06c786d..b8bfaaa23d5e9 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -564,17 +564,21 @@ namespace ts.codefix { : undefined; } - export function getImportKind(importingFile: SourceFile, exportKind: ExportKind, compilerOptions: CompilerOptions): ImportKind { + /** + * @param forceImportKeyword Indicates that the user has already typed `import`, so the result must start with `import`. + * (In other words, do not allow `const x = require("...")` for JS files.) + */ + export function getImportKind(importingFile: SourceFile, exportKind: ExportKind, compilerOptions: CompilerOptions, forceImportKeyword?: boolean): ImportKind { switch (exportKind) { case ExportKind.Named: return ImportKind.Named; case ExportKind.Default: return ImportKind.Default; - case ExportKind.ExportEquals: return getExportEqualsImportKind(importingFile, compilerOptions); - case ExportKind.UMD: return getUmdImportKind(importingFile, compilerOptions); + case ExportKind.ExportEquals: return getExportEqualsImportKind(importingFile, compilerOptions, !!forceImportKeyword); + case ExportKind.UMD: return getUmdImportKind(importingFile, compilerOptions, !!forceImportKeyword); default: return Debug.assertNever(exportKind); } } - function getUmdImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind { + function getUmdImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, forceImportKeyword: boolean): ImportKind { // Import a synthetic `default` if enabled. if (getAllowSyntheticDefaultImports(compilerOptions)) { return ImportKind.Default; @@ -587,7 +591,7 @@ namespace ts.codefix { case ModuleKind.CommonJS: case ModuleKind.UMD: if (isInJSFile(importingFile)) { - return isExternalModule(importingFile) ? ImportKind.Namespace : ImportKind.CommonJS; + return isExternalModule(importingFile) || forceImportKeyword ? ImportKind.Namespace : ImportKind.CommonJS; } return ImportKind.CommonJS; case ModuleKind.System: @@ -675,7 +679,7 @@ namespace ts.codefix { return originalSymbolToExportInfos; } - function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind { + function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, forceImportKeyword: boolean): ImportKind { const allowSyntheticDefaults = getAllowSyntheticDefaultImports(compilerOptions); const isJS = isInJSFile(importingFile); // 1. 'import =' will not work in es2015+ TS files, so the decision is between a default @@ -683,10 +687,12 @@ namespace ts.codefix { if (!isJS && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) { return allowSyntheticDefaults ? ImportKind.Default : ImportKind.Namespace; } - // 2. 'import =' will not work in JavaScript, so the decision is between a default - // and const/require. + // 2. 'import =' will not work in JavaScript, so the decision is between a default import, + // a namespace import, and const/require. if (isJS) { - return isExternalModule(importingFile) ? ImportKind.Default : ImportKind.CommonJS; + return isExternalModule(importingFile) || forceImportKeyword + ? allowSyntheticDefaults ? ImportKind.Default : ImportKind.Namespace + : ImportKind.CommonJS; } // 3. At this point the most correct choice is probably 'import =', but people // really hate that, so look to see if the importing file has any precedent diff --git a/src/services/completions.ts b/src/services/completions.ts index 6c0b8489d45cb..e16283c7f3e1d 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -711,12 +711,12 @@ namespace ts.Completions { origin.exportName === InternalSymbolName.ExportEquals ? ExportKind.ExportEquals : ExportKind.Named; const tabStop = preferences.includeCompletionsWithSnippetText ? "$1" : ""; - const importKind = codefix.getImportKind(sourceFile, exportKind, options); + const importKind = codefix.getImportKind(sourceFile, exportKind, options, /*forceImportKeyword*/ true); const suffix = useSemicolons ? ";" : ""; switch (importKind) { case ImportKind.CommonJS: return { replacementSpan, insertText: `import ${name}${tabStop} = require(${quotedModuleSpecifier})${suffix}` }; case ImportKind.Default: return { replacementSpan, insertText: `import ${name}${tabStop} from ${quotedModuleSpecifier}${suffix}` }; - case ImportKind.Namespace: return { replacementSpan, insertText: `import * as ${name}${tabStop} from ${quotedModuleSpecifier}${suffix}` }; + case ImportKind.Namespace: return { replacementSpan, insertText: `import * as ${name} from ${quotedModuleSpecifier}${suffix}` }; case ImportKind.Named: return { replacementSpan, insertText: `import { ${name}${tabStop} } from ${quotedModuleSpecifier}${suffix}` }; } } diff --git a/tests/cases/fourslash/importStatementCompletions_js.ts b/tests/cases/fourslash/importStatementCompletions_js.ts new file mode 100644 index 0000000000000..fe4e0babad584 --- /dev/null +++ b/tests/cases/fourslash/importStatementCompletions_js.ts @@ -0,0 +1,35 @@ +/// + +// These options resemble the defaults for inferred projects with JS + +// @allowJs: true +// @target: es2020 +// @checkJs: true +// @module: commonjs +// @noEmit: true +// @allowSyntheticDefaultImports: true + +// @Filename: /node_modules/react/index.d.ts +//// declare namespace React {} +//// export = React; + +// @Filename: /test.js +//// [|import R/**/|] + +verify.completions({ + marker: "", + isNewIdentifierLocation: true, + exact: [{ + name: "React", + source: "react", + insertText: `import React$1 from "react";`, + isSnippet: true, + replacementSpan: test.ranges()[0], + sourceDisplay: "react", + }], + preferences: { + includeCompletionsForImportStatements: true, + includeInsertTextCompletions: true, + includeCompletionsWithSnippetText: true, + } +}); diff --git a/tests/cases/fourslash/importStatementCompletions_js2.ts b/tests/cases/fourslash/importStatementCompletions_js2.ts new file mode 100644 index 0000000000000..431543ac58a3b --- /dev/null +++ b/tests/cases/fourslash/importStatementCompletions_js2.ts @@ -0,0 +1,37 @@ +/// + +// These options resemble the defaults for inferred projects with JS, +// except notably lacking --allowSyntheticDefaultImports. Probably nobody +// ever wants a configuration like this, but we maintain that this is +// correct and consistent behavior for these settings. + +// @allowJs: true +// @target: es2020 +// @checkJs: true +// @module: commonjs +// @noEmit: true + +// @Filename: /node_modules/react/index.d.ts +//// declare namespace React {} +//// export = React; + +// @Filename: /test.js +//// [|import R/**/|] + +verify.completions({ + marker: "", + isNewIdentifierLocation: true, + exact: [{ + name: "React", + source: "react", + insertText: `import * as React from "react";`, + isSnippet: true, + replacementSpan: test.ranges()[0], + sourceDisplay: "react", + }], + preferences: { + includeCompletionsForImportStatements: true, + includeInsertTextCompletions: true, + includeCompletionsWithSnippetText: true, + } +});