Skip to content

Commit d3d34b9

Browse files
KingwlKingwl
committed
refactor extension preference to get extension by file type
1 parent 4d57419 commit d3d34b9

File tree

11 files changed

+153
-142
lines changed

11 files changed

+153
-142
lines changed

‎src/compiler/checker.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4066,7 +4066,7 @@ namespace ts {
40664066
return `"${file.moduleName || moduleSpecifiers.getModuleSpecifiers(
40674067
symbol,
40684068
compilerOptions,
4069-
contextFile,
4069+
contextFile.path,
40704070
context!.tracker.moduleResolverHost!,
40714071
context!.tracker.moduleResolverHost!.getSourceFiles!(),
40724072
{ importModuleSpecifierPreference: "non-relative" }

‎src/compiler/moduleSpecifiers.ts‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,74 +3,73 @@
33
namespace ts.moduleSpecifiers {
44
export interface ModuleSpecifierPreferences {
55
importModuleSpecifierPreference?: "relative" | "non-relative";
6-
includeExtensionInImports?: Extension.Js | Extension.Jsx;
6+
includeExtensionInImports?: boolean;
77
}
88

99
// Note: fromSourceFile is just for usesJsExtensionOnImports
10-
export function getModuleSpecifier(compilerOptions: CompilerOptions, fromSourceFile: SourceFile, fromSourceFileName: string, toFileName: string, host: ModuleSpecifierResolutionHost, preferences: ModuleSpecifierPreferences = {}) {
11-
const info = getInfo(compilerOptions, fromSourceFile, fromSourceFileName, host, preferences);
12-
return getGlobalModuleSpecifier(toFileName, info, host, compilerOptions) ||
10+
export function getModuleSpecifier(compilerOptions: CompilerOptions, fromSourceFileName: string, toFileName: string, host: ModuleSpecifierResolutionHost, preferences: ModuleSpecifierPreferences = {}) {
11+
const info = getInfo(compilerOptions, fromSourceFileName, host);
12+
return getGlobalModuleSpecifier(toFileName, info, host, compilerOptions, preferences) ||
1313
first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences));
1414
}
1515

1616
// For each symlink/original for a module, returns a list of ways to import that file.
1717
export function getModuleSpecifiers(
1818
moduleSymbol: Symbol,
1919
compilerOptions: CompilerOptions,
20-
importingSourceFile: SourceFile,
20+
importingSourceFileName: string,
2121
host: ModuleSpecifierResolutionHost,
2222
files: ReadonlyArray<SourceFile>,
2323
preferences: ModuleSpecifierPreferences,
2424
): ReadonlyArray<ReadonlyArray<string>> {
2525
const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol);
2626
if (ambient) return [[ambient]];
2727

28-
const info = getInfo(compilerOptions, importingSourceFile, importingSourceFile.path, host, preferences);
28+
const info = getInfo(compilerOptions, importingSourceFileName, host);
2929
if (!files) {
3030
return Debug.fail("Files list must be present to resolve symlinks in specifier resolution");
3131
}
3232
const modulePaths = getAllModulePaths(files, getSourceFileOfNode(moduleSymbol.valueDeclaration), info.getCanonicalFileName, host);
3333

34-
const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions));
34+
const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions, preferences));
3535
return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName =>
3636
getLocalModuleSpecifiers(moduleFileName, info, compilerOptions, preferences));
3737
}
3838

3939
interface Info {
4040
readonly moduleResolutionKind: ModuleResolutionKind;
41-
readonly addExtension: Extension.Js | Extension.Jsx | undefined;
4241
readonly getCanonicalFileName: GetCanonicalFileName;
4342
readonly sourceDirectory: string;
4443
}
45-
// importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path
46-
function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: string, host: ModuleSpecifierResolutionHost, preferences: ModuleSpecifierPreferences): Info {
44+
45+
function getInfo(compilerOptions: CompilerOptions, importingSourceFileName: string, host: ModuleSpecifierResolutionHost): Info {
4746
const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
48-
const addExtension = usesExtensionOnImports(importingSourceFile, preferences);
4947
const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true);
5048
const sourceDirectory = getDirectoryPath(importingSourceFileName);
51-
return { moduleResolutionKind, addExtension, getCanonicalFileName, sourceDirectory };
49+
return { moduleResolutionKind, getCanonicalFileName, sourceDirectory };
5250
}
5351

5452
function getGlobalModuleSpecifier(
5553
moduleFileName: string,
56-
{ addExtension, getCanonicalFileName, sourceDirectory }: Info,
54+
{ getCanonicalFileName, sourceDirectory }: Info,
5755
host: ModuleSpecifierResolutionHost,
5856
compilerOptions: CompilerOptions,
57+
preferences: ModuleSpecifierPreferences,
5958
) {
60-
return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, addExtension)
59+
return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, preferences)
6160
|| tryGetModuleNameAsNodeModule(compilerOptions, moduleFileName, host, getCanonicalFileName, sourceDirectory)
6261
|| compilerOptions.rootDirs && tryGetModuleNameFromRootDirs(compilerOptions.rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName);
6362
}
6463

6564
function getLocalModuleSpecifiers(
6665
moduleFileName: string,
67-
{ moduleResolutionKind, addExtension, getCanonicalFileName, sourceDirectory }: Info,
66+
{ moduleResolutionKind, getCanonicalFileName, sourceDirectory }: Info,
6867
compilerOptions: CompilerOptions,
6968
preferences: ModuleSpecifierPreferences,
7069
) {
7170
const { baseUrl, paths } = compilerOptions;
7271

73-
const relativePath = removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), moduleResolutionKind, addExtension);
72+
const relativePath = removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), moduleResolutionKind, preferences);
7473
if (!baseUrl || preferences.importModuleSpecifierPreference === "relative") {
7574
return [relativePath];
7675
}
@@ -80,7 +79,7 @@ namespace ts.moduleSpecifiers {
8079
return [relativePath];
8180
}
8281

83-
const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, moduleResolutionKind, addExtension);
82+
const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, moduleResolutionKind, preferences);
8483
if (paths) {
8584
const fromPaths = tryGetModuleNameFromPaths(removeFileExtension(relativeToBaseUrl), importRelativeToBaseUrl, paths);
8685
if (fromPaths) {
@@ -130,16 +129,6 @@ namespace ts.moduleSpecifiers {
130129
return relativeFirst ? [relativePath, importRelativeToBaseUrl] : [importRelativeToBaseUrl, relativePath];
131130
}
132131

133-
function tryGetFileExtension (text: string) {
134-
return pathIsRelative(text) && fileExtensionIsOneOf(text, [Extension.Js, Extension.Jsx]) ? (
135-
fileExtensionIs(text, Extension.Js) ? Extension.Js : Extension.Jsx
136-
) : undefined;
137-
}
138-
139-
function usesExtensionOnImports({ imports }: SourceFile, preferences: ModuleSpecifierPreferences): Extension.Js | Extension.Jsx | undefined {
140-
return preferences.includeExtensionInImports === undefined ? firstDefined(imports, ({ text }) => tryGetFileExtension(text)) : preferences.includeExtensionInImports;
141-
}
142-
143132
function discoverProbableSymlinks(files: ReadonlyArray<SourceFile>, getCanonicalFileName: (file: string) => string, host: ModuleSpecifierResolutionHost) {
144133
const symlinks = mapDefined(files, sf =>
145134
sf.resolvedModules && firstDefinedIterator(sf.resolvedModules.values(), res =>
@@ -251,14 +240,14 @@ namespace ts.moduleSpecifiers {
251240
host: GetEffectiveTypeRootsHost,
252241
getCanonicalFileName: (file: string) => string,
253242
moduleFileName: string,
254-
addExtension: Extension.Js | Extension.Jsx | undefined,
243+
preferences: ModuleSpecifierPreferences
255244
): string | undefined {
256245
const roots = getEffectiveTypeRoots(options, host);
257246
return firstDefined(roots, unNormalizedTypeRoot => {
258247
const typeRoot = toPath(unNormalizedTypeRoot, /*basePath*/ undefined, getCanonicalFileName);
259248
if (startsWith(moduleFileName, typeRoot)) {
260249
// For a type definition, we can strip `/index` even with classic resolution.
261-
return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), ModuleResolutionKind.NodeJs, addExtension);
250+
return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), ModuleResolutionKind.NodeJs, preferences);
262251
}
263252
});
264253
}
@@ -403,10 +392,21 @@ namespace ts.moduleSpecifiers {
403392
});
404393
}
405394

406-
function removeExtensionAndIndexPostFix(fileName: string, moduleResolutionKind: ModuleResolutionKind, addExtension: Extension.Js | Extension.Jsx | undefined): string {
395+
function tryGetActualExtension(text: string) {
396+
return pathIsRelative(text) && (
397+
fileExtensionIsOneOf(text, [Extension.Js, Extension.Ts])
398+
? Extension.Js
399+
: fileExtensionIsOneOf(text, [Extension.Jsx, Extension.Tsx])
400+
? Extension.Jsx
401+
: undefined
402+
);
403+
}
404+
405+
function removeExtensionAndIndexPostFix(fileName: string, moduleResolutionKind: ModuleResolutionKind, preferences: ModuleSpecifierPreferences): string {
407406
const noExtension = removeFileExtension(fileName);
408-
return addExtension
409-
? noExtension + addExtension
407+
const actualExtension = tryGetActualExtension(fileName);
408+
return (preferences.includeExtensionInImports && actualExtension)
409+
? noExtension + actualExtension
410410
: moduleResolutionKind === ModuleResolutionKind.NodeJs
411411
? removeSuffix(noExtension, "/index")
412412
: noExtension;

‎src/server/protocol.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2733,7 +2733,7 @@ namespace ts.server.protocol {
27332733
readonly includeCompletionsWithInsertText?: boolean;
27342734
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
27352735
readonly allowTextChangesInNewFiles?: boolean;
2736-
readonly includeExtensionInImports?: Extension.Js | Extension.Jsx;
2736+
readonly includeExtensionInImports?: boolean;
27372737
}
27382738

27392739
export interface CompilerOptions {

‎src/services/codefixes/importFixes.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ namespace ts.codefix {
239239
preferences: UserPreferences,
240240
): ReadonlyArray<NewImportInfo> {
241241
const choicesForEachExportingModule = flatMap<SymbolExportInfo, NewImportInfo[]>(moduleSymbols, ({ moduleSymbol, importKind }) => {
242-
const modulePathsGroups = moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences);
242+
const modulePathsGroups = moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile.path, host, program.getSourceFiles(), preferences);
243243
return modulePathsGroups.map(group => group.map(moduleSpecifier => ({ moduleSpecifier, importKind })));
244244
});
245245
// Sort to keep the shortest paths first, but keep [relativePath, importRelativeToBaseUrl] groups together

‎src/services/getEditsForFileRename.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ namespace ts {
128128
// If neither the importing source file nor the imported file moved, do nothing.
129129
return toImport === undefined || !toImport.updated && !importingSourceFileMoved
130130
? undefined
131-
: moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport.newFileName, host, preferences);
131+
: moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), newImportFromPath, toImport.newFileName, host, preferences);
132132
});
133133
}
134134
}

‎src/services/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ namespace ts {
240240
readonly includeCompletionsWithInsertText?: boolean;
241241
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
242242
readonly allowTextChangesInNewFiles?: boolean;
243-
readonly includeExtensionInImports?: Extension.Js | Extension.Jsx;
243+
readonly includeExtensionInImports?: boolean;
244244
}
245245
/* @internal */
246246
export const defaultPreferences: UserPreferences = {};

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ declare namespace FourSlashInterface {
531531
includeCompletionsForModuleExports?: boolean;
532532
includeInsertTextCompletions?: boolean;
533533
importModuleSpecifierPreference?: "relative" | "non-relative";
534-
includeExtensionInImports?: '.js' | '.jsx'
534+
includeExtensionInImports?: boolean
535535
}
536536
interface CompletionsAtOptions extends UserPreferences {
537537
triggerCharacter?: string;

‎tests/cases/fourslash/getEditsForFileRename_jsExtension.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@ verify.getEditsForFileRename({
1313
newPath: "/src/b.js",
1414
newFileContents: {
1515
"/b.js":
16-
`import { a } from "./a.js";`,
16+
`import { a } from "./a";`,
1717
},
1818
});

‎tests/cases/fourslash/importNameCodeFix_ExtensionPreference.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @moduleResolution: node
4+
// @noLib: true
5+
// @allowJs: true
6+
// @checkJs: true
7+
8+
// @Filename: /a.js
9+
////export function a() {}
10+
11+
// @Filename: /b.ts
12+
////export function b() {}
13+
14+
// @Filename: /c.jsx
15+
////export function c() {}
16+
17+
// @Filename: /d.tsx
18+
////export function d() {}
19+
20+
// @Filename: /normalExt.ts
21+
////a;
22+
////b;
23+
////c;
24+
////d;
25+
26+
// @Filename: /includeExt.ts
27+
////a;
28+
////b;
29+
////c;
30+
////d;
31+
32+
// @Filename: /includeExt.js
33+
////a;
34+
////b;
35+
////c;
36+
////d;
37+
38+
39+
goTo.file("/normalExt.ts");
40+
verify.importFixAtPosition([
41+
`import { a } from "./a";
42+
43+
a;
44+
b;
45+
c;
46+
d;`, `import { b } from "./b";
47+
48+
a;
49+
b;
50+
c;
51+
d;`, `import { c } from "./c";
52+
53+
a;
54+
b;
55+
c;
56+
d;`, `import { d } from "./d";
57+
58+
a;
59+
b;
60+
c;
61+
d;`]);
62+
63+
goTo.file("/includeExt.ts");
64+
verify.importFixAtPosition([
65+
`import { a } from "./a.js";
66+
67+
a;
68+
b;
69+
c;
70+
d;`, `import { b } from "./b.js";
71+
72+
a;
73+
b;
74+
c;
75+
d;`, `import { c } from "./c.jsx";
76+
77+
a;
78+
b;
79+
c;
80+
d;`, `import { d } from "./d.jsx";
81+
82+
a;
83+
b;
84+
c;
85+
d;`], /* errorCode */ undefined, {
86+
includeExtensionInImports: true
87+
});
88+
89+
goTo.file("/includeExt.js");
90+
verify.importFixAtPosition([
91+
`import { a } from "./a.js";
92+
93+
a;
94+
b;
95+
c;
96+
d;`, `import { b } from "./b.js";
97+
98+
a;
99+
b;
100+
c;
101+
d;`, `import { c } from "./c.jsx";
102+
103+
a;
104+
b;
105+
c;
106+
d;`, `import { d } from "./d.jsx";
107+
108+
a;
109+
b;
110+
c;
111+
d;`], /* errorCode */ undefined, {
112+
includeExtensionInImports: true
113+
});

‎tests/cases/fourslash/importNameCodeFix_jsExtension.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,6 @@ verify.importFixAtPosition([
2020
`import * as g from "global"; // Global imports skipped
2121
import { a } from "./a.js";
2222
import { a as a2 } from "./a"; // Ignored, only the first relative import is considered
23-
import { b } from "./b.js";
23+
import { b } from "./b";
2424
b;`,
2525
]);

0 commit comments

Comments
 (0)