Skip to content

Make import fix for 'export =' respect module target and allowSyntheticDefaultImports/esModuleInterop #32150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2328,7 +2328,10 @@ Actual: ${stringify(fullActual)}`);
public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) {
this.goToMarker(markerName);

const details = this.getCompletionEntryDetails(options.name, options.source, options.preferences)!;
const details = this.getCompletionEntryDetails(options.name, options.source, options.preferences);
if (!details) {
return this.raiseError(`No completions were found for the given name, source, and preferences.`);
}
const codeActions = details.codeActions!;
if (codeActions.length !== 1) {
this.raiseError(`Expected one code action, got ${codeActions.length}`);
Expand Down
30 changes: 18 additions & 12 deletions src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ namespace ts.codefix {
position: number,
preferences: UserPreferences,
): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } {
const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getCompilerOptions(), program.getTypeChecker(), program.getSourceFiles());
const exportInfos = getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getCompilerOptions(), program.getTypeChecker(), program.getSourceFiles());
Debug.assert(exportInfos.some(info => info.moduleSymbol === moduleSymbol));
// We sort the best codefixes first, so taking `first` is best for completions.
const moduleSpecifier = first(getNewImportInfos(program, sourceFile, position, exportInfos, host, preferences)).moduleSpecifier;
Expand All @@ -175,15 +175,15 @@ namespace ts.codefix {
return { description, changes, commands };
}

function getAllReExportingModules(exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>): ReadonlyArray<SymbolExportInfo> {
function getAllReExportingModules(importingFile: SourceFile, exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker, allSourceFiles: ReadonlyArray<SourceFile>): ReadonlyArray<SymbolExportInfo> {
const result: SymbolExportInfo[] = [];
forEachExternalModule(checker, allSourceFiles, (moduleSymbol, moduleFile) => {
// Don't import from a re-export when looking "up" like to `./index` or `../index`.
if (moduleFile && moduleSymbol !== exportingModuleSymbol && startsWith(sourceFile.fileName, getDirectoryPath(moduleFile.fileName))) {
return;
}

const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
const defaultInfo = getDefaultLikeExportInfo(importingFile, moduleSymbol, checker, compilerOptions);
if (defaultInfo && defaultInfo.name === symbolName && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) {
result.push({ moduleSymbol, importKind: defaultInfo.kind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(defaultInfo.symbol, checker) });
}
Expand Down Expand Up @@ -329,7 +329,7 @@ namespace ts.codefix {
if (!umdSymbol) return undefined;
const symbol = checker.getAliasedSymbol(umdSymbol);
const symbolName = umdSymbol.name;
const exportInfos: ReadonlyArray<SymbolExportInfo> = [{ moduleSymbol: symbol, importKind: getUmdImportKind(program.getCompilerOptions()), exportedSymbolIsTypeOnly: false }];
const exportInfos: ReadonlyArray<SymbolExportInfo> = [{ moduleSymbol: symbol, importKind: getUmdImportKind(sourceFile, program.getCompilerOptions()), exportedSymbolIsTypeOnly: false }];
const fixes = getFixForImport(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, program, sourceFile, host, preferences);
return { fixes, symbolName };
}
Expand All @@ -345,7 +345,7 @@ namespace ts.codefix {
: undefined;
}

function getUmdImportKind(compilerOptions: CompilerOptions): ImportKind {
function getUmdImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind {
// Import a synthetic `default` if enabled.
if (getAllowSyntheticDefaultImports(compilerOptions)) {
return ImportKind.Default;
Expand All @@ -357,7 +357,7 @@ namespace ts.codefix {
case ModuleKind.AMD:
case ModuleKind.CommonJS:
case ModuleKind.UMD:
return ImportKind.Equals;
return isInJSFile(importingFile) ? ImportKind.Default : ImportKind.Equals;
case ModuleKind.System:
case ModuleKind.ES2015:
case ModuleKind.ESNext:
Expand Down Expand Up @@ -403,7 +403,7 @@ namespace ts.codefix {
forEachExternalModuleToImportFrom(checker, sourceFile, program.getSourceFiles(), moduleSymbol => {
cancellationToken.throwIfCancellationRequested();

const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, program.getCompilerOptions());
const defaultInfo = getDefaultLikeExportInfo(sourceFile, moduleSymbol, checker, program.getCompilerOptions());
if (defaultInfo && defaultInfo.name === symbolName && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) {
addSymbol(moduleSymbol, defaultInfo.symbol, defaultInfo.kind);
}
Expand All @@ -418,20 +418,26 @@ namespace ts.codefix {
}

function getDefaultLikeExportInfo(
moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions,
): { readonly symbol: Symbol, readonly symbolForMeaning: Symbol, readonly name: string, readonly kind: ImportKind.Default | ImportKind.Equals } | undefined {
const exported = getDefaultLikeExportWorker(moduleSymbol, checker);
importingFile: SourceFile, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions,
): { readonly symbol: Symbol, readonly symbolForMeaning: Symbol, readonly name: string, readonly kind: ImportKind } | undefined {
const exported = getDefaultLikeExportWorker(importingFile, moduleSymbol, checker, compilerOptions);
if (!exported) return undefined;
const { symbol, kind } = exported;
const info = getDefaultExportInfoWorker(symbol, moduleSymbol, checker, compilerOptions);
return info && { symbol, kind, ...info };
}

function getDefaultLikeExportWorker(moduleSymbol: Symbol, checker: TypeChecker): { readonly symbol: Symbol, readonly kind: ImportKind.Default | ImportKind.Equals } | undefined {
function getDefaultLikeExportWorker(importingFile: SourceFile, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbol: Symbol, readonly kind: ImportKind } | undefined {
const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol);
if (defaultExport) return { symbol: defaultExport, kind: ImportKind.Default };
const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol);
return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: ImportKind.Equals };
return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getImportKindForExportEquals(importingFile, compilerOptions) };
}

function getImportKindForExportEquals(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind {
return (isInJSFile(importingFile) || getAllowSyntheticDefaultImports(compilerOptions) && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015)
? ImportKind.Default
: ImportKind.Equals;
}

function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <reference path="fourslash.ts" />

// @EsModuleInterop: true
// @Module: es2015

// @Filename: /foo.d.ts
////declare module "foo" {
//// const _default: number;
//// export = _default;
////}

// @Filename: /index.ts
////foo/**/

goTo.marker('');
verify.getAndApplyCodeFix(2304, 0);
verify.currentFileContentIs(
`import foo from "foo";

foo`);
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/// <reference path="fourslash.ts" />

// @Module: esnext

// @Filename: /foo.d.ts
////declare module "foo" {
//// const _default: number;
//// export = _default;
////}

// @Filename: /index.ts
////foo/**/

goTo.marker('');
verify.getAndApplyCodeFix(2304, 0);
verify.currentFileContentIs(
`import foo = require("foo");

foo`);
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <reference path="fourslash.ts" />

// @allowJs: true
// @checkJs: true

// @Filename: /foo.d.ts
////declare module "foo" {
//// const _default: number;
//// export = _default;
////}

// @Filename: /index.js
////foo/**/

goTo.marker('');
verify.getAndApplyCodeFix(2304, 0);
verify.currentFileContentIs(
`import foo from "foo";

foo`);
21 changes: 21 additions & 0 deletions tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/// <reference path="fourslash.ts" />

// @AllowSyntheticDefaultImports: false
// @Module: commonjs
// @CheckJs: true
// @AllowJs: true

// @Filename: a/f1.js
//// [|export function test() { };
//// bar1/*0*/.bar;|]

// @Filename: a/foo.d.ts
//// export declare function bar(): number;
//// export as namespace bar1;

verify.importFixAtPosition([
`import bar1 from "./foo";

export function test() { };
bar1.bar;`
]);