Skip to content

Commit 266d8de

Browse files
andrewbranchandrewbranch
authored
Proposal: importModuleSpecifierPreference: project-relative (microsoft#40637)
* Add new importModuleSpecifierPreference value * Add second test * Update API baselines * Clean up and add some comments * Rename option value
1 parent de4a57a commit 266d8de

13 files changed

+197
-13
lines changed

‎src/compiler/moduleSpecifiers.ts

+55-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Used by importFixes, getEditsForFileRename, and declaration emit to synthesize import module specifiers.
22
/* @internal */
33
namespace ts.moduleSpecifiers {
4-
const enum RelativePreference { Relative, NonRelative, Auto }
4+
const enum RelativePreference { Relative, NonRelative, Shortest, ExternalNonRelative }
55
// See UserPreferences#importPathEnding
66
const enum Ending { Minimal, Index, JsExtension }
77

@@ -13,7 +13,11 @@ namespace ts.moduleSpecifiers {
1313

1414
function getPreferences({ importModuleSpecifierPreference, importModuleSpecifierEnding }: UserPreferences, compilerOptions: CompilerOptions, importingSourceFile: SourceFile): Preferences {
1515
return {
16-
relativePreference: importModuleSpecifierPreference === "relative" ? RelativePreference.Relative : importModuleSpecifierPreference === "non-relative" ? RelativePreference.NonRelative : RelativePreference.Auto,
16+
relativePreference:
17+
importModuleSpecifierPreference === "relative" ? RelativePreference.Relative :
18+
importModuleSpecifierPreference === "non-relative" ? RelativePreference.NonRelative :
19+
importModuleSpecifierPreference === "project-relative" ? RelativePreference.ExternalNonRelative :
20+
RelativePreference.Shortest,
1721
ending: getEnding(),
1822
};
1923
function getEnding(): Ending {
@@ -147,17 +151,19 @@ namespace ts.moduleSpecifiers {
147151

148152
interface Info {
149153
readonly getCanonicalFileName: GetCanonicalFileName;
154+
readonly importingSourceFileName: Path
150155
readonly sourceDirectory: Path;
151156
}
152157
// importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path
153158
function getInfo(importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info {
154159
const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true);
155160
const sourceDirectory = getDirectoryPath(importingSourceFileName);
156-
return { getCanonicalFileName, sourceDirectory };
161+
return { getCanonicalFileName, importingSourceFileName, sourceDirectory };
157162
}
158163

159-
function getLocalModuleSpecifier(moduleFileName: string, { getCanonicalFileName, sourceDirectory }: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, { ending, relativePreference }: Preferences): string {
164+
function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, { ending, relativePreference }: Preferences): string {
160165
const { baseUrl, paths, rootDirs, bundledPackageName } = compilerOptions;
166+
const { sourceDirectory, getCanonicalFileName } = info;
161167

162168
const relativePath = rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName, ending, compilerOptions) ||
163169
removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), ending, compilerOptions);
@@ -183,7 +189,42 @@ namespace ts.moduleSpecifiers {
183189
return nonRelative;
184190
}
185191

186-
if (relativePreference !== RelativePreference.Auto) Debug.assertNever(relativePreference);
192+
if (relativePreference === RelativePreference.ExternalNonRelative) {
193+
const projectDirectory = host.getCurrentDirectory();
194+
const modulePath = toPath(moduleFileName, projectDirectory, getCanonicalFileName);
195+
const sourceIsInternal = startsWith(sourceDirectory, projectDirectory);
196+
const targetIsInternal = startsWith(modulePath, projectDirectory);
197+
if (sourceIsInternal && !targetIsInternal || !sourceIsInternal && targetIsInternal) {
198+
// 1. The import path crosses the boundary of the tsconfig.json-containing directory.
199+
//
200+
// src/
201+
// tsconfig.json
202+
// index.ts -------
203+
// lib/ | (path crosses tsconfig.json)
204+
// imported.ts <---
205+
//
206+
return nonRelative;
207+
}
208+
209+
const nearestTargetPackageJson = getNearestAncestorDirectoryWithPackageJson(host, getDirectoryPath(modulePath));
210+
const nearestSourcePackageJson = getNearestAncestorDirectoryWithPackageJson(host, sourceDirectory);
211+
if (nearestSourcePackageJson !== nearestTargetPackageJson) {
212+
// 2. The importing and imported files are part of different packages.
213+
//
214+
// packages/a/
215+
// package.json
216+
// index.ts --------
217+
// packages/b/ | (path crosses package.json)
218+
// package.json |
219+
// component.ts <---
220+
//
221+
return nonRelative;
222+
}
223+
224+
return relativePath;
225+
}
226+
227+
if (relativePreference !== RelativePreference.Shortest) Debug.assertNever(relativePreference);
187228

188229
// Prefer a relative import over a baseUrl import if it has fewer components.
189230
return isPathRelativeToParent(nonRelative) || countPathComponents(relativePath) < countPathComponents(nonRelative) ? relativePath : nonRelative;
@@ -213,6 +254,15 @@ namespace ts.moduleSpecifiers {
213254
);
214255
}
215256

257+
function getNearestAncestorDirectoryWithPackageJson(host: ModuleSpecifierResolutionHost, fileName: string) {
258+
if (host.getNearestAncestorDirectoryWithPackageJson) {
259+
return host.getNearestAncestorDirectoryWithPackageJson(fileName);
260+
}
261+
return !!forEachAncestorDirectory(fileName, directory => {
262+
return host.fileExists(combinePaths(directory, "package.json")) ? true : undefined;
263+
});
264+
}
265+
216266
export function forEachFileNameOfModule<T>(
217267
importingFileName: string,
218268
importedFileName: string,

‎src/compiler/types.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -7858,6 +7858,7 @@ namespace ts {
78587858
realpath?(path: string): string;
78597859
getSymlinkCache?(): SymlinkCache;
78607860
getGlobalTypingsCacheLocation?(): string | undefined;
7861+
getNearestAncestorDirectoryWithPackageJson?(fileName: string, rootDir?: string): string | undefined;
78617862

78627863
getSourceFiles(): readonly SourceFile[];
78637864
readonly redirectTargetsMap: RedirectTargetsMap;
@@ -8159,7 +8160,7 @@ namespace ts {
81598160
readonly includeCompletionsForModuleExports?: boolean;
81608161
readonly includeAutomaticOptionalChainCompletions?: boolean;
81618162
readonly includeCompletionsWithInsertText?: boolean;
8162-
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
8163+
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
81638164
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
81648165
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
81658166
readonly allowTextChangesInNewFiles?: boolean;

‎src/harness/fourslashImpl.ts

+4
Original file line numberDiff line numberDiff line change
@@ -2937,6 +2937,10 @@ namespace FourSlash {
29372937
}
29382938
const range = ts.firstOrUndefined(ranges);
29392939

2940+
if (preferences) {
2941+
this.configure(preferences);
2942+
}
2943+
29402944
const codeFixes = this.getCodeFixes(fileName, errorCode, preferences).filter(f => f.fixName === ts.codefix.importFixName);
29412945

29422946
if (codeFixes.length === 0) {

‎src/server/editorServices.ts

+15-1
Original file line numberDiff line numberDiff line change
@@ -3878,7 +3878,7 @@ namespace ts.server {
38783878
const info = packageJsonCache.getInDirectory(directory);
38793879
if (info) result.push(info);
38803880
}
3881-
if (rootPath && rootPath === this.toPath(directory)) {
3881+
if (rootPath && rootPath === directory) {
38823882
return true;
38833883
}
38843884
};
@@ -3887,6 +3887,20 @@ namespace ts.server {
38873887
return result;
38883888
}
38893889

3890+
/*@internal*/
3891+
getNearestAncestorDirectoryWithPackageJson(fileName: string): string | undefined {
3892+
return forEachAncestorDirectory(fileName, directory => {
3893+
switch (this.packageJsonCache.directoryHasPackageJson(this.toPath(directory))) {
3894+
case Ternary.True: return directory;
3895+
case Ternary.False: return undefined;
3896+
case Ternary.Maybe:
3897+
return this.host.fileExists(combinePaths(directory, "package.json"))
3898+
? directory
3899+
: undefined;
3900+
}
3901+
});
3902+
}
3903+
38903904
/*@internal*/
38913905
private watchPackageJsonFile(path: Path) {
38923906
const watchers = this.packageJsonFilesMap || (this.packageJsonFilesMap = new Map());

‎src/server/project.ts

+9
Original file line numberDiff line numberDiff line change
@@ -1651,6 +1651,11 @@ namespace ts.server {
16511651
return this.projectService.getPackageJsonsVisibleToFile(fileName, rootDir);
16521652
}
16531653

1654+
/*@internal*/
1655+
getNearestAncestorDirectoryWithPackageJson(fileName: string): string | undefined {
1656+
return this.projectService.getNearestAncestorDirectoryWithPackageJson(fileName);
1657+
}
1658+
16541659
/*@internal*/
16551660
getPackageJsonsForAutoImport(rootDir?: string): readonly PackageJsonInfo[] {
16561661
const packageJsons = this.getPackageJsonsVisibleToFile(combinePaths(this.currentDirectory, inferredTypesContainingFile), rootDir);
@@ -1994,6 +1999,10 @@ namespace ts.server {
19941999
return super.updateGraph();
19952000
}
19962001

2002+
hasRoots() {
2003+
return !!this.rootFileNames?.length;
2004+
}
2005+
19972006
markAsDirty() {
19982007
this.rootFileNames = undefined;
19992008
super.markAsDirty();

‎src/server/protocol.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -3231,7 +3231,7 @@ namespace ts.server.protocol {
32313231
* values, with insertion text to replace preceding `.` tokens with `?.`.
32323232
*/
32333233
readonly includeAutomaticOptionalChainCompletions?: boolean;
3234-
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
3234+
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
32353235
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
32363236
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
32373237
readonly allowTextChangesInNewFiles?: boolean;

‎src/services/types.ts

+2
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ namespace ts {
301301
/* @internal */
302302
getPackageJsonsVisibleToFile?(fileName: string, rootDir?: string): readonly PackageJsonInfo[];
303303
/* @internal */
304+
getNearestAncestorDirectoryWithPackageJson?(fileName: string): string | undefined;
305+
/* @internal */
304306
getPackageJsonsForAutoImport?(rootDir?: string): readonly PackageJsonInfo[];
305307
/* @internal */
306308
getImportSuggestionsCache?(): Completions.ImportSuggestionsForFileCache;

‎src/services/utilities.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1808,7 +1808,8 @@ namespace ts {
18081808
redirectTargetsMap: program.redirectTargetsMap,
18091809
getProjectReferenceRedirect: fileName => program.getProjectReferenceRedirect(fileName),
18101810
isSourceOfProjectReferenceRedirect: fileName => program.isSourceOfProjectReferenceRedirect(fileName),
1811-
getCompilerOptions: () => program.getCompilerOptions()
1811+
getCompilerOptions: () => program.getCompilerOptions(),
1812+
getNearestAncestorDirectoryWithPackageJson: maybeBind(host, host.getNearestAncestorDirectoryWithPackageJson),
18121813
};
18131814
}
18141815

‎tests/baselines/reference/api/tsserverlibrary.d.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -3866,7 +3866,7 @@ declare namespace ts {
38663866
readonly includeCompletionsForModuleExports?: boolean;
38673867
readonly includeAutomaticOptionalChainCompletions?: boolean;
38683868
readonly includeCompletionsWithInsertText?: boolean;
3869-
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
3869+
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
38703870
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
38713871
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
38723872
readonly allowTextChangesInNewFiles?: boolean;
@@ -9004,7 +9004,7 @@ declare namespace ts.server.protocol {
90049004
* values, with insertion text to replace preceding `.` tokens with `?.`.
90059005
*/
90069006
readonly includeAutomaticOptionalChainCompletions?: boolean;
9007-
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
9007+
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
90089008
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
90099009
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
90109010
readonly allowTextChangesInNewFiles?: boolean;
@@ -9387,6 +9387,7 @@ declare namespace ts.server {
93879387
private rootFileNames;
93889388
isOrphan(): boolean;
93899389
updateGraph(): boolean;
9390+
hasRoots(): boolean;
93909391
markAsDirty(): void;
93919392
getScriptFileNames(): string[];
93929393
getLanguageService(): never;

‎tests/baselines/reference/api/typescript.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -3866,7 +3866,7 @@ declare namespace ts {
38663866
readonly includeCompletionsForModuleExports?: boolean;
38673867
readonly includeAutomaticOptionalChainCompletions?: boolean;
38683868
readonly includeCompletionsWithInsertText?: boolean;
3869-
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
3869+
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
38703870
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
38713871
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
38723872
readonly allowTextChangesInNewFiles?: boolean;

‎tests/cases/fourslash/fourslash.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ declare namespace FourSlashInterface {
610610
readonly includeCompletionsForModuleExports?: boolean;
611611
readonly includeInsertTextCompletions?: boolean;
612612
readonly includeAutomaticOptionalChainCompletions?: boolean;
613-
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
613+
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
614614
readonly importModuleSpecifierEnding?: "minimal" | "index" | "js";
615615
}
616616
interface CompletionsOptions {

‎tests/cases/fourslash/server/importNameCodeFix_externalNonRelateive2.ts

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/// <reference path="../fourslash.ts" />
2+
3+
// @Filename: /apps/app1/tsconfig.json
4+
//// {
5+
//// "compilerOptions": {
6+
//// "module": "commonjs",
7+
//// "paths": {
8+
//// "shared/*": ["../../shared/*"]
9+
//// }
10+
//// },
11+
//// "include": ["src", "../../shared"]
12+
//// }
13+
14+
// @Filename: /apps/app1/src/index.ts
15+
//// shared/*internal2external*/
16+
17+
// @Filename: /apps/app1/src/app.ts
18+
//// utils/*internal2internal*/
19+
20+
// @Filename: /apps/app1/src/utils.ts
21+
//// export const utils = 0;
22+
23+
// @Filename: /shared/constants.ts
24+
//// export const shared = 0;
25+
26+
// @Filename: /shared/data.ts
27+
//// shared/*external2external*/
28+
29+
format.setOption("newline", "\n");
30+
31+
goTo.marker("internal2external");
32+
verify.importFixAtPosition([`import { shared } from "shared/constants";\n\nshared`], /*errorCode*/ undefined, {
33+
importModuleSpecifierPreference: "project-relative"
34+
});
35+
36+
goTo.marker("internal2internal");
37+
verify.importFixAtPosition([`import { utils } from "./utils";\n\nutils`], /*errorCode*/ undefined, {
38+
importModuleSpecifierPreference: "project-relative"
39+
});
40+
41+
goTo.marker("external2external");
42+
verify.importFixAtPosition([`import { shared } from "./constants";\n\nshared`], /*errorCode*/ undefined, {
43+
importModuleSpecifierPreference: "project-relative"
44+
});

‎tests/cases/fourslash/server/importNameCodeFix_externalNonRelative1.ts

+58
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/// <reference path="../fourslash.ts" />
2+
3+
// @Filename: /tsconfig.base.json
4+
//// {
5+
//// "compilerOptions": {
6+
//// "module": "commonjs",
7+
//// "paths": {
8+
//// "pkg-1/*": ["./packages/pkg-1/src/*"],
9+
//// "pkg-2/*": ["./packages/pkg-2/src/*"]
10+
//// }
11+
//// }
12+
//// }
13+
14+
// @Filename: /packages/pkg-1/package.json
15+
//// { "dependencies": { "pkg-2": "*" } }
16+
17+
// @Filename: /packages/pkg-1/tsconfig.json
18+
//// {
19+
//// "extends": "../../tsconfig.base.json",
20+
//// "references": [
21+
//// { "path": "../pkg-2" }
22+
//// ]
23+
//// }
24+
25+
// @Filename: /packages/pkg-1/src/index.ts
26+
//// Pkg2/*external*/
27+
28+
// @Filename: /packages/pkg-2/package.json
29+
//// { "types": "dist/index.d.ts" }
30+
31+
// @Filename: /packages/pkg-2/tsconfig.json
32+
//// {
33+
//// "extends": "../../tsconfig.base.json",
34+
//// "compilerOptions": { "outDir": "dist", "rootDir": "src", "composite": true }
35+
//// }
36+
37+
// @Filename: /packages/pkg-2/src/index.ts
38+
//// import "./utils";
39+
40+
// @Filename: /packages/pkg-2/src/utils.ts
41+
//// export const Pkg2 = {};
42+
43+
// @Filename: /packages/pkg-2/src/blah/foo/data.ts
44+
//// Pkg2/*internal*/
45+
46+
// @link: /packages/pkg-2 -> /packages/pkg-1/node_modules/pkg-2
47+
48+
format.setOption("newline", "\n");
49+
50+
goTo.marker("external");
51+
verify.importFixAtPosition([`import { Pkg2 } from "pkg-2/utils";\n\nPkg2`], /*errorCode*/ undefined, {
52+
importModuleSpecifierPreference: "project-relative"
53+
});
54+
55+
goTo.marker("internal");
56+
verify.importFixAtPosition([`import { Pkg2 } from "../../utils";\n\nPkg2`], /*errorCode*/ undefined, {
57+
importModuleSpecifierPreference: "project-relative"
58+
});

0 commit comments

Comments
 (0)