Skip to content

Commit cf33f7b

Browse files
authored
Don’t use 'import =' for auto-import when esModuleInterop is on unless it’s already been done (#36475)
* Revisit esModuleInterop import kind priority yet again * Test was wholly redundant * Check for precedent is simpler now
1 parent 9968f14 commit cf33f7b

File tree

2 files changed

+21
-15
lines changed

2 files changed

+21
-15
lines changed

src/services/codefixes/importFixes.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -461,28 +461,32 @@ namespace ts.codefix {
461461
const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol);
462462
if (defaultExport) return { symbol: defaultExport, kind: ImportKind.Default };
463463
const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol);
464-
return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getExportEqualsImportKind(importingFile, compilerOptions, checker) };
464+
return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getExportEqualsImportKind(importingFile, compilerOptions) };
465465
}
466466

467-
function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker): ImportKind {
467+
function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind {
468+
const allowSyntheticDefaults = getAllowSyntheticDefaultImports(compilerOptions);
469+
// 1. 'import =' will not work in es2015+, so the decision is between a default
470+
// and a namespace import, based on allowSyntheticDefaultImports/esModuleInterop.
468471
if (getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) {
469-
return getAllowSyntheticDefaultImports(compilerOptions) ? ImportKind.Default : ImportKind.Namespace;
472+
return allowSyntheticDefaults ? ImportKind.Default : ImportKind.Namespace;
470473
}
474+
// 2. 'import =' will not work in JavaScript, so the decision is between a default
475+
// and const/require.
471476
if (isInJSFile(importingFile)) {
472477
return isExternalModule(importingFile) ? ImportKind.Default : ImportKind.ConstEquals;
473478
}
479+
// 3. At this point the most correct choice is probably 'import =', but people
480+
// really hate that, so look to see if the importing file has any precedent
481+
// on how to handle it.
474482
for (const statement of importingFile.statements) {
475483
if (isImportEqualsDeclaration(statement)) {
476484
return ImportKind.Equals;
477485
}
478-
if (isImportDeclaration(statement) && statement.importClause && statement.importClause.name) {
479-
const moduleSymbol = checker.getImmediateAliasedSymbol(statement.importClause.symbol);
480-
if (moduleSymbol && moduleSymbol.name !== InternalSymbolName.Default) {
481-
return ImportKind.Default;
482-
}
483-
}
484486
}
485-
return ImportKind.Equals;
487+
// 4. We have no precedent to go on, so just use a default import if
488+
// allowSyntheticDefaultImports/esModuleInterop is enabled.
489+
return allowSyntheticDefaults ? ImportKind.Default : ImportKind.Equals;
486490
}
487491

488492
function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined {

tests/cases/fourslash/importNameCodeFixNewImportExportEqualsCommonJSInteropOn.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
////}
1919

2020
// @Filename: /a.ts
21-
////import bar from "bar";
21+
////import bar = require("bar");
2222
////
2323
////foo
2424

@@ -27,25 +27,27 @@
2727

2828
// @Filename: /c.ts
2929
////import es from "es";
30+
////import bar = require("bar");
3031
////
3132
////foo
3233

3334
// 1. Should match existing imports of 'export ='
3435
goTo.file('/a.ts');
35-
verify.importFixAtPosition([`import bar from "bar";
36-
import foo from "foo";
36+
verify.importFixAtPosition([`import bar = require("bar");
37+
import foo = require("foo");
3738
3839
foo`]);
3940

40-
// 2. Should default to ImportEquals
41+
// 2. Should default to default import
4142
goTo.file('/b.ts');
42-
verify.importFixAtPosition([`import foo = require("foo");
43+
verify.importFixAtPosition([`import foo from "foo";
4344
4445
foo`]);
4546

4647
// 3. Importing an 'export default' doesn’t count toward the usage heursitic
4748
goTo.file('/c.ts');
4849
verify.importFixAtPosition([`import es from "es";
50+
import bar = require("bar");
4951
import foo = require("foo");
5052
5153
foo`]);

0 commit comments

Comments
 (0)