Skip to content

add support for Extension Preference in import #25073

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 2 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
52 changes: 35 additions & 17 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace ts.moduleSpecifiers {
export interface ModuleSpecifierPreferences {
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
readonly includeExtensionInImports?: boolean;
}

// Note: importingSourceFile is just for usesJsExtensionOnImports
Expand All @@ -17,7 +18,7 @@ namespace ts.moduleSpecifiers {
): string {
const info = getInfo(compilerOptions, importingSourceFile, importingSourceFileName, host);
const modulePaths = getAllModulePaths(files, toFileName, info.getCanonicalFileName, host);
return firstDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)) ||
return firstDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions, preferences)) ||
first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences));
}

Expand All @@ -39,46 +40,47 @@ namespace ts.moduleSpecifiers {
}
const modulePaths = getAllModulePaths(files, getSourceFileOfNode(moduleSymbol.valueDeclaration).fileName, info.getCanonicalFileName, host);

const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions));
const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions, preferences));
return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName =>
getLocalModuleSpecifiers(moduleFileName, info, compilerOptions, preferences));
}

interface Info {
readonly moduleResolutionKind: ModuleResolutionKind;
readonly addJsExtension: boolean;
readonly addExtension: boolean;
readonly getCanonicalFileName: GetCanonicalFileName;
readonly sourceDirectory: Path;
}
// importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path
function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info {
const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
const addJsExtension = usesJsExtensionOnImports(importingSourceFile);
const addExtension = usesExtensionOnImports(importingSourceFile);
const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true);
const sourceDirectory = getDirectoryPath(importingSourceFileName);
return { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory };
return { moduleResolutionKind, addExtension, getCanonicalFileName, sourceDirectory };
}

function getGlobalModuleSpecifier(
moduleFileName: string,
{ addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
{ addExtension, getCanonicalFileName, sourceDirectory }: Info,
host: ModuleSpecifierResolutionHost,
compilerOptions: CompilerOptions,
preferences: ModuleSpecifierPreferences
) {
return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, addJsExtension)
return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, addExtension, preferences)
|| tryGetModuleNameAsNodeModule(compilerOptions, moduleFileName, host, getCanonicalFileName, sourceDirectory);
}

function getLocalModuleSpecifiers(
moduleFileName: string,
{ moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
{ moduleResolutionKind, addExtension, getCanonicalFileName, sourceDirectory }: Info,
compilerOptions: CompilerOptions,
preferences: ModuleSpecifierPreferences,
): ReadonlyArray<string> {
const { baseUrl, paths, rootDirs } = compilerOptions;

const relativePath = rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName) ||
removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), moduleResolutionKind, addJsExtension);
removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), moduleResolutionKind, addExtension, compilerOptions, preferences);
if (!baseUrl || preferences.importModuleSpecifierPreference === "relative") {
return [relativePath];
}
Expand All @@ -88,7 +90,7 @@ namespace ts.moduleSpecifiers {
return [relativePath];
}

const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, moduleResolutionKind, addJsExtension);
const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, moduleResolutionKind, addExtension, compilerOptions, preferences);
if (paths) {
const fromPaths = tryGetModuleNameFromPaths(removeFileExtension(relativeToBaseUrl), importRelativeToBaseUrl, paths);
if (fromPaths) {
Expand Down Expand Up @@ -138,8 +140,8 @@ namespace ts.moduleSpecifiers {
return relativeFirst ? [relativePath, importRelativeToBaseUrl] : [importRelativeToBaseUrl, relativePath];
}

function usesJsExtensionOnImports({ imports }: SourceFile): boolean {
return firstDefined(imports, ({ text }) => pathIsRelative(text) ? fileExtensionIs(text, Extension.Js) : undefined) || false;
function usesExtensionOnImports({ imports }: SourceFile): boolean {
return firstDefined(imports, ({ text }) => pathIsRelative(text) ? fileExtensionIsOneOf(text, [Extension.Js, Extension.Jsx]) : undefined) || false;
}

function discoverProbableSymlinks(files: ReadonlyArray<SourceFile>, getCanonicalFileName: (file: string) => string, host: ModuleSpecifierResolutionHost) {
Expand Down Expand Up @@ -256,14 +258,15 @@ namespace ts.moduleSpecifiers {
host: GetEffectiveTypeRootsHost,
getCanonicalFileName: (file: string) => string,
moduleFileName: string,
addJsExtension: boolean,
addExtension: boolean,
preferences: ModuleSpecifierPreferences
): string | undefined {
const roots = getEffectiveTypeRoots(options, host);
return firstDefined(roots, unNormalizedTypeRoot => {
const typeRoot = toPath(unNormalizedTypeRoot, /*basePath*/ undefined, getCanonicalFileName);
if (startsWith(moduleFileName, typeRoot)) {
// For a type definition, we can strip `/index` even with classic resolution.
return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), ModuleResolutionKind.NodeJs, addJsExtension);
return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), ModuleResolutionKind.NodeJs, addExtension, options, preferences);
}
});
}
Expand Down Expand Up @@ -408,10 +411,25 @@ namespace ts.moduleSpecifiers {
});
}

function removeExtensionAndIndexPostFix(fileName: string, moduleResolutionKind: ModuleResolutionKind, addJsExtension: boolean): string {
function tryGetActualExtension(text: string, compilerOptions: CompilerOptions) {
const extension = pathIsRelative(text) && tryGetExtensionFromPath(text);
if (!extension) return undefined;

switch (extension) {
case Extension.Ts:
return Extension.Js;
case Extension.Tsx:
return compilerOptions.jsx === JsxEmit.React || compilerOptions.jsx === JsxEmit.ReactNative ? Extension.Js : Extension.Jsx;
default:
return extension;
}
}

function removeExtensionAndIndexPostFix(fileName: string, moduleResolutionKind: ModuleResolutionKind, addJsExtension: boolean, compilerOptions: CompilerOptions, preferences: ModuleSpecifierPreferences): string {
const noExtension = removeFileExtension(fileName);
return addJsExtension
? noExtension + ".js"
const actualExtension = tryGetActualExtension(fileName, compilerOptions);
return (actualExtension && (preferences.includeExtensionInImports !== undefined && preferences.includeExtensionInImports || addJsExtension))
? noExtension + actualExtension
: moduleResolutionKind === ModuleResolutionKind.NodeJs
? removeSuffix(noExtension, "/index")
: noExtension;
Expand Down
1 change: 1 addition & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2802,6 +2802,7 @@ namespace ts.server.protocol {
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
readonly allowTextChangesInNewFiles?: boolean;
readonly includeExtensionInImports?: boolean;
}

export interface CompilerOptions {
Expand Down
1 change: 1 addition & 0 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ namespace ts {
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
readonly allowTextChangesInNewFiles?: boolean;
readonly includeExtensionInImports?: boolean;
}
/* @internal */
export const emptyOptions = {};
Expand Down
2 changes: 2 additions & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4811,6 +4811,7 @@ declare namespace ts {
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
readonly allowTextChangesInNewFiles?: boolean;
readonly includeExtensionInImports?: boolean;
}
interface LanguageService {
cleanupSemanticCache(): void;
Expand Down Expand Up @@ -7962,6 +7963,7 @@ declare namespace ts.server.protocol {
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
readonly allowTextChangesInNewFiles?: boolean;
readonly includeExtensionInImports?: boolean;
}
interface CompilerOptions {
allowJs?: boolean;
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4811,6 +4811,7 @@ declare namespace ts {
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
readonly allowTextChangesInNewFiles?: boolean;
readonly includeExtensionInImports?: boolean;
}
interface LanguageService {
cleanupSemanticCache(): void;
Expand Down
114 changes: 114 additions & 0 deletions tests/cases/fourslash/importNameCodeFix_ExtensionPreference.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/// <reference path="fourslash.ts" />

// @moduleResolution: node
// @noLib: true
// @allowJs: true
// @checkJs: true
// @jsx: preserve

// @Filename: /a.js
////export function a() {}

// @Filename: /b.ts
////export function b() {}

// @Filename: /c.jsx
////export function c() {}

// @Filename: /d.tsx
////export function d() {}

// @Filename: /normalExt.ts
////a;
////b;
////c;
////d;

// @Filename: /includeExt.ts
////a;
////b;
////c;
////d;

// @Filename: /includeExt.js
////a;
////b;
////c;
////d;


goTo.file("/normalExt.ts");
verify.importFixAtPosition([
`import { a } from "./a";

a;
b;
c;
d;`, `import { b } from "./b";

a;
b;
c;
d;`, `import { c } from "./c";

a;
b;
c;
d;`, `import { d } from "./d";

a;
b;
c;
d;`]);

goTo.file("/includeExt.ts");
verify.importFixAtPosition([
`import { a } from "./a.js";

a;
b;
c;
d;`, `import { b } from "./b.js";

a;
b;
c;
d;`, `import { c } from "./c.jsx";

a;
b;
c;
d;`, `import { d } from "./d.jsx";

a;
b;
c;
d;`], /* errorCode */ undefined, {
includeExtensionInImports: true
});

goTo.file("/includeExt.js");
verify.importFixAtPosition([
`import { a } from "./a.js";

a;
b;
c;
d;`, `import { b } from "./b.js";

a;
b;
c;
d;`, `import { c } from "./c.jsx";

a;
b;
c;
d;`, `import { d } from "./d.jsx";

a;
b;
c;
d;`], /* errorCode */ undefined, {
includeExtensionInImports: true
});