Skip to content

Commit 6277305

Browse files
authored
Fix import statement completions for export= in JS files (microsoft#45128)
1 parent c79ec7b commit 6277305

File tree

4 files changed

+89
-11
lines changed

4 files changed

+89
-11
lines changed

src/services/codefixes/importFixes.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -560,17 +560,21 @@ namespace ts.codefix {
560560
: undefined;
561561
}
562562

563-
export function getImportKind(importingFile: SourceFile, exportKind: ExportKind, compilerOptions: CompilerOptions): ImportKind {
563+
/**
564+
* @param forceImportKeyword Indicates that the user has already typed `import`, so the result must start with `import`.
565+
* (In other words, do not allow `const x = require("...")` for JS files.)
566+
*/
567+
export function getImportKind(importingFile: SourceFile, exportKind: ExportKind, compilerOptions: CompilerOptions, forceImportKeyword?: boolean): ImportKind {
564568
switch (exportKind) {
565569
case ExportKind.Named: return ImportKind.Named;
566570
case ExportKind.Default: return ImportKind.Default;
567-
case ExportKind.ExportEquals: return getExportEqualsImportKind(importingFile, compilerOptions);
568-
case ExportKind.UMD: return getUmdImportKind(importingFile, compilerOptions);
571+
case ExportKind.ExportEquals: return getExportEqualsImportKind(importingFile, compilerOptions, !!forceImportKeyword);
572+
case ExportKind.UMD: return getUmdImportKind(importingFile, compilerOptions, !!forceImportKeyword);
569573
default: return Debug.assertNever(exportKind);
570574
}
571575
}
572576

573-
function getUmdImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind {
577+
function getUmdImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, forceImportKeyword: boolean): ImportKind {
574578
// Import a synthetic `default` if enabled.
575579
if (getAllowSyntheticDefaultImports(compilerOptions)) {
576580
return ImportKind.Default;
@@ -583,7 +587,7 @@ namespace ts.codefix {
583587
case ModuleKind.CommonJS:
584588
case ModuleKind.UMD:
585589
if (isInJSFile(importingFile)) {
586-
return isExternalModule(importingFile) ? ImportKind.Namespace : ImportKind.CommonJS;
590+
return isExternalModule(importingFile) || forceImportKeyword ? ImportKind.Namespace : ImportKind.CommonJS;
587591
}
588592
return ImportKind.CommonJS;
589593
case ModuleKind.System:
@@ -671,18 +675,20 @@ namespace ts.codefix {
671675
return originalSymbolToExportInfos;
672676
}
673677

674-
function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind {
678+
function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, forceImportKeyword: boolean): ImportKind {
675679
const allowSyntheticDefaults = getAllowSyntheticDefaultImports(compilerOptions);
676680
const isJS = isInJSFile(importingFile);
677681
// 1. 'import =' will not work in es2015+ TS files, so the decision is between a default
678682
// and a namespace import, based on allowSyntheticDefaultImports/esModuleInterop.
679683
if (!isJS && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) {
680684
return allowSyntheticDefaults ? ImportKind.Default : ImportKind.Namespace;
681685
}
682-
// 2. 'import =' will not work in JavaScript, so the decision is between a default
683-
// and const/require.
686+
// 2. 'import =' will not work in JavaScript, so the decision is between a default import,
687+
// a namespace import, and const/require.
684688
if (isJS) {
685-
return isExternalModule(importingFile) ? ImportKind.Default : ImportKind.CommonJS;
689+
return isExternalModule(importingFile) || forceImportKeyword
690+
? allowSyntheticDefaults ? ImportKind.Default : ImportKind.Namespace
691+
: ImportKind.CommonJS;
686692
}
687693
// 3. At this point the most correct choice is probably 'import =', but people
688694
// really hate that, so look to see if the importing file has any precedent

src/services/completions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,12 +713,12 @@ namespace ts.Completions {
713713
origin.exportName === InternalSymbolName.ExportEquals ? ExportKind.ExportEquals :
714714
ExportKind.Named;
715715
const tabStop = preferences.includeCompletionsWithSnippetText ? "$1" : "";
716-
const importKind = codefix.getImportKind(sourceFile, exportKind, options);
716+
const importKind = codefix.getImportKind(sourceFile, exportKind, options, /*forceImportKeyword*/ true);
717717
const suffix = useSemicolons ? ";" : "";
718718
switch (importKind) {
719719
case ImportKind.CommonJS: return { replacementSpan, insertText: `import ${name}${tabStop} = require(${quotedModuleSpecifier})${suffix}` };
720720
case ImportKind.Default: return { replacementSpan, insertText: `import ${name}${tabStop} from ${quotedModuleSpecifier}${suffix}` };
721-
case ImportKind.Namespace: return { replacementSpan, insertText: `import * as ${name}${tabStop} from ${quotedModuleSpecifier}${suffix}` };
721+
case ImportKind.Namespace: return { replacementSpan, insertText: `import * as ${name} from ${quotedModuleSpecifier}${suffix}` };
722722
case ImportKind.Named: return { replacementSpan, insertText: `import { ${name}${tabStop} } from ${quotedModuleSpecifier}${suffix}` };
723723
}
724724
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// These options resemble the defaults for inferred projects with JS
4+
5+
// @allowJs: true
6+
// @target: es2020
7+
// @checkJs: true
8+
// @module: commonjs
9+
// @noEmit: true
10+
// @allowSyntheticDefaultImports: true
11+
12+
// @Filename: /node_modules/react/index.d.ts
13+
//// declare namespace React {}
14+
//// export = React;
15+
16+
// @Filename: /test.js
17+
//// [|import R/**/|]
18+
19+
verify.completions({
20+
marker: "",
21+
isNewIdentifierLocation: true,
22+
exact: [{
23+
name: "React",
24+
source: "react",
25+
insertText: `import React$1 from "react";`,
26+
isSnippet: true,
27+
replacementSpan: test.ranges()[0],
28+
sourceDisplay: "react",
29+
}],
30+
preferences: {
31+
includeCompletionsForImportStatements: true,
32+
includeInsertTextCompletions: true,
33+
includeCompletionsWithSnippetText: true,
34+
}
35+
});
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// These options resemble the defaults for inferred projects with JS,
4+
// except notably lacking --allowSyntheticDefaultImports. Probably nobody
5+
// ever wants a configuration like this, but we maintain that this is
6+
// correct and consistent behavior for these settings.
7+
8+
// @allowJs: true
9+
// @target: es2020
10+
// @checkJs: true
11+
// @module: commonjs
12+
// @noEmit: true
13+
14+
// @Filename: /node_modules/react/index.d.ts
15+
//// declare namespace React {}
16+
//// export = React;
17+
18+
// @Filename: /test.js
19+
//// [|import R/**/|]
20+
21+
verify.completions({
22+
marker: "",
23+
isNewIdentifierLocation: true,
24+
exact: [{
25+
name: "React",
26+
source: "react",
27+
insertText: `import * as React from "react";`,
28+
isSnippet: true,
29+
replacementSpan: test.ranges()[0],
30+
sourceDisplay: "react",
31+
}],
32+
preferences: {
33+
includeCompletionsForImportStatements: true,
34+
includeInsertTextCompletions: true,
35+
includeCompletionsWithSnippetText: true,
36+
}
37+
});

0 commit comments

Comments
 (0)