Skip to content

Commit 736363b

Browse files
authored
Use other files when necessary to determine import style in JS (#40879)
* Use other files when necessary to determine import style in JS * Fix existing tests
1 parent dd84bc1 commit 736363b

7 files changed

+96
-41
lines changed

src/services/codefixes/importFixes.ts

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ namespace ts.codefix {
6464
const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker));
6565
const exportInfos = getAllReExportingModules(sourceFile, symbol, moduleSymbol, symbolName, host, program, useAutoImportProvider);
6666
const preferTypeOnlyImport = !!usageIsTypeOnly && compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error;
67-
const useRequire = shouldUseRequire(sourceFile, compilerOptions);
67+
const useRequire = shouldUseRequire(sourceFile, program);
6868
const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, /*position*/ undefined, preferTypeOnlyImport, useRequire, host, preferences);
6969
addImport({ fixes: [fix], symbolName });
7070
}
@@ -211,7 +211,7 @@ namespace ts.codefix {
211211
): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } {
212212
const compilerOptions = program.getCompilerOptions();
213213
const exportInfos = getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, host, program, /*useAutoImportProvider*/ true);
214-
const useRequire = shouldUseRequire(sourceFile, compilerOptions);
214+
const useRequire = shouldUseRequire(sourceFile, program);
215215
const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && !isSourceFileJS(sourceFile) && isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
216216
const moduleSpecifier = first(getNewImportInfos(program, sourceFile, position, preferTypeOnlyImport, useRequire, exportInfos, host, preferences)).moduleSpecifier;
217217
const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, preferTypeOnlyImport, useRequire, host, preferences);
@@ -362,10 +362,31 @@ namespace ts.codefix {
362362
});
363363
}
364364

365-
function shouldUseRequire(sourceFile: SourceFile, compilerOptions: CompilerOptions): boolean {
366-
return isSourceFileJS(sourceFile)
367-
&& !sourceFile.externalModuleIndicator
368-
&& (!!sourceFile.commonJsModuleIndicator || getEmitModuleKind(compilerOptions) < ModuleKind.ES2015);
365+
function shouldUseRequire(sourceFile: SourceFile, program: Program): boolean {
366+
// 1. TypeScript files don't use require variable declarations
367+
if (!isSourceFileJS(sourceFile)) {
368+
return false;
369+
}
370+
371+
// 2. If the current source file is unambiguously CJS or ESM, go with that
372+
if (sourceFile.commonJsModuleIndicator && !sourceFile.externalModuleIndicator) return true;
373+
if (sourceFile.externalModuleIndicator && !sourceFile.commonJsModuleIndicator) return false;
374+
375+
// 3. If there's a tsconfig/jsconfig, use its module setting
376+
const compilerOptions = program.getCompilerOptions();
377+
if (compilerOptions.configFile) {
378+
return getEmitModuleKind(compilerOptions) < ModuleKind.ES2015;
379+
}
380+
381+
// 4. Match the first other JS file in the program that's unambiguously CJS or ESM
382+
for (const otherFile of program.getSourceFiles()) {
383+
if (otherFile === sourceFile || !isSourceFileJS(otherFile) || program.isSourceFileFromExternalLibrary(otherFile)) continue;
384+
if (otherFile.commonJsModuleIndicator && !otherFile.externalModuleIndicator) return true;
385+
if (otherFile.externalModuleIndicator && !otherFile.commonJsModuleIndicator) return false;
386+
}
387+
388+
// 5. Literally nothing to go on
389+
return true;
369390
}
370391

371392
function getNewImportInfos(
@@ -445,7 +466,7 @@ namespace ts.codefix {
445466
const symbol = checker.getAliasedSymbol(umdSymbol);
446467
const symbolName = umdSymbol.name;
447468
const exportInfos: readonly SymbolExportInfo[] = [{ moduleSymbol: symbol, importKind: getUmdImportKind(sourceFile, program.getCompilerOptions()), exportedSymbolIsTypeOnly: false }];
448-
const useRequire = shouldUseRequire(sourceFile, program.getCompilerOptions());
469+
const useRequire = shouldUseRequire(sourceFile, program);
449470
const fixes = getFixForImport(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, /*preferTypeOnlyImport*/ false, useRequire, program, sourceFile, host, preferences);
450471
return { fixes, symbolName };
451472
}
@@ -497,7 +518,7 @@ namespace ts.codefix {
497518

498519
const compilerOptions = program.getCompilerOptions();
499520
const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && isValidTypeOnlyAliasUseSite(symbolToken);
500-
const useRequire = shouldUseRequire(sourceFile, compilerOptions);
521+
const useRequire = shouldUseRequire(sourceFile, program);
501522
const exportInfos = getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, program, useAutoImportProvider, host);
502523
const fixes = arrayFrom(flatMapIterator(exportInfos.entries(), ([_, exportInfos]) =>
503524
getFixForImport(exportInfos, symbolName, symbolToken.getStart(sourceFile), preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences)));

tests/cases/fourslash/importFixWithMultipleModuleExportAssignment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@
1717

1818
goTo.file("/c.js");
1919
verify.importFixAtPosition([
20-
`import { foo } from "./b";
20+
`const { foo } = require("./b");
2121
2222
foo`]);

tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts

Lines changed: 0 additions & 31 deletions
This file was deleted.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/// <reference path="fourslash.ts" />
2+
// @allowJs: true
3+
// @checkJs: true
4+
5+
// @Filename: types/dep.d.ts
6+
//// export declare class Dep {}
7+
8+
// @Filename: index.js
9+
//// Dep/**/
10+
11+
// @Filename: util.js
12+
//// import fs from 'fs';
13+
14+
15+
// When current file has no imports/requires, look at other JS files
16+
goTo.marker("");
17+
verify.importFixAtPosition([`import { Dep } from "./types/dep";
18+
19+
Dep`]);
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/// <reference path="fourslash.ts" />
2+
// @allowJs: true
3+
// @checkJs: true
4+
5+
// @Filename: types/dep.d.ts
6+
//// export declare class Dep {}
7+
8+
// @Filename: index.js
9+
//// Dep/**/
10+
11+
// @Filename: util1.ts
12+
//// import fs from 'fs';
13+
14+
// @Filename: util2.js
15+
//// const fs = require('fs');
16+
17+
18+
// TypeScript files don't count as indicators of import style for JS
19+
goTo.marker("");
20+
verify.importFixAtPosition([`const { Dep } = require("./types/dep");
21+
22+
Dep`]);
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path="fourslash.ts" />
2+
// @allowJs: true
3+
// @checkJs: true
4+
5+
// @Filename: types/dep.d.ts
6+
//// export declare class Dep {}
7+
8+
// @Filename: index.js
9+
//// import fs from 'fs';
10+
//// const path = require('path');
11+
////
12+
//// Dep/**/
13+
14+
// @Filename: util2.js
15+
//// export {};
16+
17+
18+
// When current file has both imports and requires, get import style from other files
19+
goTo.marker("");
20+
verify.importFixAtPosition([`import fs from 'fs';
21+
import { Dep } from './types/dep';
22+
const path = require('path');
23+
24+
Dep`]);

tests/cases/fourslash/importNameCodeFix_require_namedAndDefault.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// @allowJs: true
44
// @checkJs: true
55

6-
// @Filename: blah.js
6+
// @Filename: blah.ts
77
////export default class Blah {}
88
////export const Named1 = 0;
99
////export const Named2 = 1;

0 commit comments

Comments
 (0)