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
This commit is contained in:
Andrew Branch 2020-10-05 11:39:20 -07:00 committed by GitHub
parent dd84bc1dc9
commit 736363b427
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 96 additions and 41 deletions

View File

@ -64,7 +64,7 @@ namespace ts.codefix {
const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker));
const exportInfos = getAllReExportingModules(sourceFile, symbol, moduleSymbol, symbolName, host, program, useAutoImportProvider);
const preferTypeOnlyImport = !!usageIsTypeOnly && compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error;
const useRequire = shouldUseRequire(sourceFile, compilerOptions);
const useRequire = shouldUseRequire(sourceFile, program);
const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, /*position*/ undefined, preferTypeOnlyImport, useRequire, host, preferences);
addImport({ fixes: [fix], symbolName });
}
@ -211,7 +211,7 @@ namespace ts.codefix {
): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } {
const compilerOptions = program.getCompilerOptions();
const exportInfos = getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, host, program, /*useAutoImportProvider*/ true);
const useRequire = shouldUseRequire(sourceFile, compilerOptions);
const useRequire = shouldUseRequire(sourceFile, program);
const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && !isSourceFileJS(sourceFile) && isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
const moduleSpecifier = first(getNewImportInfos(program, sourceFile, position, preferTypeOnlyImport, useRequire, exportInfos, host, preferences)).moduleSpecifier;
const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, preferTypeOnlyImport, useRequire, host, preferences);
@ -362,10 +362,31 @@ namespace ts.codefix {
});
}
function shouldUseRequire(sourceFile: SourceFile, compilerOptions: CompilerOptions): boolean {
return isSourceFileJS(sourceFile)
&& !sourceFile.externalModuleIndicator
&& (!!sourceFile.commonJsModuleIndicator || getEmitModuleKind(compilerOptions) < ModuleKind.ES2015);
function shouldUseRequire(sourceFile: SourceFile, program: Program): boolean {
// 1. TypeScript files don't use require variable declarations
if (!isSourceFileJS(sourceFile)) {
return false;
}
// 2. If the current source file is unambiguously CJS or ESM, go with that
if (sourceFile.commonJsModuleIndicator && !sourceFile.externalModuleIndicator) return true;
if (sourceFile.externalModuleIndicator && !sourceFile.commonJsModuleIndicator) return false;
// 3. If there's a tsconfig/jsconfig, use its module setting
const compilerOptions = program.getCompilerOptions();
if (compilerOptions.configFile) {
return getEmitModuleKind(compilerOptions) < ModuleKind.ES2015;
}
// 4. Match the first other JS file in the program that's unambiguously CJS or ESM
for (const otherFile of program.getSourceFiles()) {
if (otherFile === sourceFile || !isSourceFileJS(otherFile) || program.isSourceFileFromExternalLibrary(otherFile)) continue;
if (otherFile.commonJsModuleIndicator && !otherFile.externalModuleIndicator) return true;
if (otherFile.externalModuleIndicator && !otherFile.commonJsModuleIndicator) return false;
}
// 5. Literally nothing to go on
return true;
}
function getNewImportInfos(
@ -445,7 +466,7 @@ namespace ts.codefix {
const symbol = checker.getAliasedSymbol(umdSymbol);
const symbolName = umdSymbol.name;
const exportInfos: readonly SymbolExportInfo[] = [{ moduleSymbol: symbol, importKind: getUmdImportKind(sourceFile, program.getCompilerOptions()), exportedSymbolIsTypeOnly: false }];
const useRequire = shouldUseRequire(sourceFile, program.getCompilerOptions());
const useRequire = shouldUseRequire(sourceFile, program);
const fixes = getFixForImport(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, /*preferTypeOnlyImport*/ false, useRequire, program, sourceFile, host, preferences);
return { fixes, symbolName };
}
@ -497,7 +518,7 @@ namespace ts.codefix {
const compilerOptions = program.getCompilerOptions();
const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && isValidTypeOnlyAliasUseSite(symbolToken);
const useRequire = shouldUseRequire(sourceFile, compilerOptions);
const useRequire = shouldUseRequire(sourceFile, program);
const exportInfos = getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, program, useAutoImportProvider, host);
const fixes = arrayFrom(flatMapIterator(exportInfos.entries(), ([_, exportInfos]) =>
getFixForImport(exportInfos, symbolName, symbolToken.getStart(sourceFile), preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences)));

View File

@ -17,6 +17,6 @@
goTo.file("/c.js");
verify.importFixAtPosition([
`import { foo } from "./b";
`const { foo } = require("./b");
foo`]);

View File

@ -1,31 +0,0 @@
/// <reference path="fourslash.ts" />
// @allowJs: true
// @checkJs: true
// @Filename: /foo.d.ts
////declare module "foo" {
//// const foo: number;
//// export = foo;
////}
// @Filename: /a.js
////foo
// @Filename: /b.js
////import "";
////
////foo
// 1. JavaScript should default to 'const ... = require...' without compiler flags set
goTo.file('/a.js');
verify.importFixAtPosition([`const foo = require("foo");
foo`]);
// 2. If there are any ImportDeclarations, assume a default import is fine
goTo.file('/b.js');
verify.importFixAtPosition([`import "";
import foo from "foo";
foo`]);

View File

@ -0,0 +1,19 @@
/// <reference path="fourslash.ts" />
// @allowJs: true
// @checkJs: true
// @Filename: types/dep.d.ts
//// export declare class Dep {}
// @Filename: index.js
//// Dep/**/
// @Filename: util.js
//// import fs from 'fs';
// When current file has no imports/requires, look at other JS files
goTo.marker("");
verify.importFixAtPosition([`import { Dep } from "./types/dep";
Dep`]);

View File

@ -0,0 +1,22 @@
/// <reference path="fourslash.ts" />
// @allowJs: true
// @checkJs: true
// @Filename: types/dep.d.ts
//// export declare class Dep {}
// @Filename: index.js
//// Dep/**/
// @Filename: util1.ts
//// import fs from 'fs';
// @Filename: util2.js
//// const fs = require('fs');
// TypeScript files don't count as indicators of import style for JS
goTo.marker("");
verify.importFixAtPosition([`const { Dep } = require("./types/dep");
Dep`]);

View File

@ -0,0 +1,24 @@
/// <reference path="fourslash.ts" />
// @allowJs: true
// @checkJs: true
// @Filename: types/dep.d.ts
//// export declare class Dep {}
// @Filename: index.js
//// import fs from 'fs';
//// const path = require('path');
////
//// Dep/**/
// @Filename: util2.js
//// export {};
// When current file has both imports and requires, get import style from other files
goTo.marker("");
verify.importFixAtPosition([`import fs from 'fs';
import { Dep } from './types/dep';
const path = require('path');
Dep`]);

View File

@ -3,7 +3,7 @@
// @allowJs: true
// @checkJs: true
// @Filename: blah.js
// @Filename: blah.ts
////export default class Blah {}
////export const Named1 = 0;
////export const Named2 = 1;