Skip to content

Proposal: importModuleSpecifierPreference: project-relative #40637

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

Merged
Merged
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
60 changes: 55 additions & 5 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Used by importFixes, getEditsForFileRename, and declaration emit to synthesize import module specifiers.
/* @internal */
namespace ts.moduleSpecifiers {
const enum RelativePreference { Relative, NonRelative, Auto }
const enum RelativePreference { Relative, NonRelative, Shortest, ExternalNonRelative }
// See UserPreferences#importPathEnding
const enum Ending { Minimal, Index, JsExtension }

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

function getPreferences({ importModuleSpecifierPreference, importModuleSpecifierEnding }: UserPreferences, compilerOptions: CompilerOptions, importingSourceFile: SourceFile): Preferences {
return {
relativePreference: importModuleSpecifierPreference === "relative" ? RelativePreference.Relative : importModuleSpecifierPreference === "non-relative" ? RelativePreference.NonRelative : RelativePreference.Auto,
relativePreference:
importModuleSpecifierPreference === "relative" ? RelativePreference.Relative :
importModuleSpecifierPreference === "non-relative" ? RelativePreference.NonRelative :
importModuleSpecifierPreference === "project-relative" ? RelativePreference.ExternalNonRelative :
RelativePreference.Shortest,
ending: getEnding(),
};
function getEnding(): Ending {
Expand Down Expand Up @@ -147,17 +151,19 @@ namespace ts.moduleSpecifiers {

interface Info {
readonly getCanonicalFileName: GetCanonicalFileName;
readonly importingSourceFileName: Path
readonly sourceDirectory: Path;
}
// importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path
function getInfo(importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info {
const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true);
const sourceDirectory = getDirectoryPath(importingSourceFileName);
return { getCanonicalFileName, sourceDirectory };
return { getCanonicalFileName, importingSourceFileName, sourceDirectory };
}

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

const relativePath = rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName, ending, compilerOptions) ||
removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), ending, compilerOptions);
Expand All @@ -180,7 +186,42 @@ namespace ts.moduleSpecifiers {
return nonRelative;
}

if (relativePreference !== RelativePreference.Auto) Debug.assertNever(relativePreference);
if (relativePreference === RelativePreference.ExternalNonRelative) {
const projectDirectory = host.getCurrentDirectory();
const modulePath = toPath(moduleFileName, projectDirectory, getCanonicalFileName);
const sourceIsInternal = startsWith(sourceDirectory, projectDirectory);
const targetIsInternal = startsWith(modulePath, projectDirectory);
if (sourceIsInternal && !targetIsInternal || !sourceIsInternal && targetIsInternal) {
// 1. The import path crosses the boundary of the tsconfig.json-containing directory.
//
// src/
// tsconfig.json
// index.ts -------
// lib/ | (path crosses tsconfig.json)
// imported.ts <---
//
return nonRelative;
}

const nearestTargetPackageJson = getNearestAncestorDirectoryWithPackageJson(host, getDirectoryPath(modulePath));
const nearestSourcePackageJson = getNearestAncestorDirectoryWithPackageJson(host, sourceDirectory);
if (nearestSourcePackageJson !== nearestTargetPackageJson) {
// 2. The importing and imported files are part of different packages.
//
// packages/a/
// package.json
// index.ts --------
// packages/b/ | (path crosses package.json)
// package.json |
// component.ts <---
//
return nonRelative;
}

return relativePath;
}

if (relativePreference !== RelativePreference.Shortest) Debug.assertNever(relativePreference);

// Prefer a relative import over a baseUrl import if it has fewer components.
return isPathRelativeToParent(nonRelative) || countPathComponents(relativePath) < countPathComponents(nonRelative) ? relativePath : nonRelative;
Expand Down Expand Up @@ -210,6 +251,15 @@ namespace ts.moduleSpecifiers {
);
}

function getNearestAncestorDirectoryWithPackageJson(host: ModuleSpecifierResolutionHost, fileName: string) {
if (host.getNearestAncestorDirectoryWithPackageJson) {
return host.getNearestAncestorDirectoryWithPackageJson(fileName);
}
return !!forEachAncestorDirectory(fileName, directory => {
return host.fileExists(combinePaths(directory, "package.json")) ? true : undefined;
});
}

export function forEachFileNameOfModule<T>(
importingFileName: string,
importedFileName: string,
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7847,6 +7847,7 @@ namespace ts {
realpath?(path: string): string;
getSymlinkCache?(): SymlinkCache;
getGlobalTypingsCacheLocation?(): string | undefined;
getNearestAncestorDirectoryWithPackageJson?(fileName: string, rootDir?: string): string | undefined;

getSourceFiles(): readonly SourceFile[];
readonly redirectTargetsMap: RedirectTargetsMap;
Expand Down Expand Up @@ -8148,7 +8149,7 @@ namespace ts {
readonly includeCompletionsForModuleExports?: boolean;
readonly includeAutomaticOptionalChainCompletions?: boolean;
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
readonly allowTextChangesInNewFiles?: boolean;
Expand Down
4 changes: 4 additions & 0 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2927,6 +2927,10 @@ namespace FourSlash {
}
const range = ts.firstOrUndefined(ranges);

if (preferences) {
this.configure(preferences);
}

const codeFixes = this.getCodeFixes(fileName, errorCode, preferences).filter(f => f.fixName === ts.codefix.importFixName);

if (codeFixes.length === 0) {
Expand Down
16 changes: 15 additions & 1 deletion src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3778,7 +3778,7 @@ namespace ts.server {
const info = packageJsonCache.getInDirectory(directory);
if (info) result.push(info);
}
if (rootPath && rootPath === this.toPath(directory)) {
if (rootPath && rootPath === directory) {
return true;
}
};
Expand All @@ -3787,6 +3787,20 @@ namespace ts.server {
return result;
}

/*@internal*/
getNearestAncestorDirectoryWithPackageJson(fileName: string): string | undefined {
return forEachAncestorDirectory(fileName, directory => {
switch (this.packageJsonCache.directoryHasPackageJson(this.toPath(directory))) {
case Ternary.True: return directory;
case Ternary.False: return undefined;
case Ternary.Maybe:
return this.host.fileExists(combinePaths(directory, "package.json"))
? directory
: undefined;
}
});
}

/*@internal*/
private watchPackageJsonFile(path: Path) {
const watchers = this.packageJsonFilesMap || (this.packageJsonFilesMap = new Map());
Expand Down
9 changes: 9 additions & 0 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1625,6 +1625,11 @@ namespace ts.server {
return this.projectService.getPackageJsonsVisibleToFile(fileName, rootDir);
}

/*@internal*/
getNearestAncestorDirectoryWithPackageJson(fileName: string): string | undefined {
return this.projectService.getNearestAncestorDirectoryWithPackageJson(fileName);
}

/*@internal*/
getPackageJsonsForAutoImport(rootDir?: string): readonly PackageJsonInfo[] {
const packageJsons = this.getPackageJsonsVisibleToFile(combinePaths(this.currentDirectory, inferredTypesContainingFile), rootDir);
Expand Down Expand Up @@ -1955,6 +1960,10 @@ namespace ts.server {
return super.updateGraph();
}

hasRoots() {
return !!this.rootFileNames?.length;
}

markAsDirty() {
this.rootFileNames = undefined;
super.markAsDirty();
Expand Down
2 changes: 1 addition & 1 deletion src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3217,7 +3217,7 @@ namespace ts.server.protocol {
* values, with insertion text to replace preceding `.` tokens with `?.`.
*/
readonly includeAutomaticOptionalChainCompletions?: boolean;
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
readonly allowTextChangesInNewFiles?: boolean;
Expand Down
2 changes: 2 additions & 0 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,8 @@ namespace ts {
/* @internal */
getPackageJsonsVisibleToFile?(fileName: string, rootDir?: string): readonly PackageJsonInfo[];
/* @internal */
getNearestAncestorDirectoryWithPackageJson?(fileName: string): string | undefined;
/* @internal */
getPackageJsonsForAutoImport?(rootDir?: string): readonly PackageJsonInfo[];
/* @internal */
getImportSuggestionsCache?(): Completions.ImportSuggestionsForFileCache;
Expand Down
3 changes: 2 additions & 1 deletion src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1773,7 +1773,8 @@ namespace ts {
redirectTargetsMap: program.redirectTargetsMap,
getProjectReferenceRedirect: fileName => program.getProjectReferenceRedirect(fileName),
isSourceOfProjectReferenceRedirect: fileName => program.isSourceOfProjectReferenceRedirect(fileName),
getCompilerOptions: () => program.getCompilerOptions()
getCompilerOptions: () => program.getCompilerOptions(),
getNearestAncestorDirectoryWithPackageJson: maybeBind(host, host.getNearestAncestorDirectoryWithPackageJson),
};
}

Expand Down
5 changes: 3 additions & 2 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3868,7 +3868,7 @@ declare namespace ts {
readonly includeCompletionsForModuleExports?: boolean;
readonly includeAutomaticOptionalChainCompletions?: boolean;
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
readonly allowTextChangesInNewFiles?: boolean;
Expand Down Expand Up @@ -8984,7 +8984,7 @@ declare namespace ts.server.protocol {
* values, with insertion text to replace preceding `.` tokens with `?.`.
*/
readonly includeAutomaticOptionalChainCompletions?: boolean;
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
readonly allowTextChangesInNewFiles?: boolean;
Expand Down Expand Up @@ -9360,6 +9360,7 @@ declare namespace ts.server {
private rootFileNames;
isOrphan(): boolean;
updateGraph(): boolean;
hasRoots(): boolean;
markAsDirty(): void;
getScriptFileNames(): string[];
getLanguageService(): never;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3868,7 +3868,7 @@ declare namespace ts {
readonly includeCompletionsForModuleExports?: boolean;
readonly includeAutomaticOptionalChainCompletions?: boolean;
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
readonly allowTextChangesInNewFiles?: boolean;
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ declare namespace FourSlashInterface {
readonly includeCompletionsForModuleExports?: boolean;
readonly includeInsertTextCompletions?: boolean;
readonly includeAutomaticOptionalChainCompletions?: boolean;
readonly importModuleSpecifierPreference?: "auto" | "relative" | "non-relative";
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
readonly importModuleSpecifierEnding?: "minimal" | "index" | "js";
}
interface CompletionsOptions {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/// <reference path="../fourslash.ts" />

// @Filename: /apps/app1/tsconfig.json
//// {
//// "compilerOptions": {
//// "module": "commonjs",
//// "paths": {
//// "shared/*": ["../../shared/*"]
//// }
//// },
//// "include": ["src", "../../shared"]
//// }

// @Filename: /apps/app1/src/index.ts
//// shared/*internal2external*/

// @Filename: /apps/app1/src/app.ts
//// utils/*internal2internal*/

// @Filename: /apps/app1/src/utils.ts
//// export const utils = 0;

// @Filename: /shared/constants.ts
//// export const shared = 0;

// @Filename: /shared/data.ts
//// shared/*external2external*/

format.setOption("newline", "\n");

goTo.marker("internal2external");
verify.importFixAtPosition([`import { shared } from "shared/constants";\n\nshared`], /*errorCode*/ undefined, {
importModuleSpecifierPreference: "project-relative"
});

goTo.marker("internal2internal");
verify.importFixAtPosition([`import { utils } from "./utils";\n\nutils`], /*errorCode*/ undefined, {
importModuleSpecifierPreference: "project-relative"
});

goTo.marker("external2external");
verify.importFixAtPosition([`import { shared } from "./constants";\n\nshared`], /*errorCode*/ undefined, {
importModuleSpecifierPreference: "project-relative"
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/// <reference path="../fourslash.ts" />

// @Filename: /tsconfig.base.json
//// {
//// "compilerOptions": {
//// "module": "commonjs",
//// "paths": {
//// "pkg-1/*": ["./packages/pkg-1/src/*"],
//// "pkg-2/*": ["./packages/pkg-2/src/*"]
//// }
//// }
//// }

// @Filename: /packages/pkg-1/package.json
//// { "dependencies": { "pkg-2": "*" } }

// @Filename: /packages/pkg-1/tsconfig.json
//// {
//// "extends": "../../tsconfig.base.json",
//// "references": [
//// { "path": "../pkg-2" }
//// ]
//// }

// @Filename: /packages/pkg-1/src/index.ts
//// Pkg2/*external*/

// @Filename: /packages/pkg-2/package.json
//// { "types": "dist/index.d.ts" }

// @Filename: /packages/pkg-2/tsconfig.json
//// {
//// "extends": "../../tsconfig.base.json",
//// "compilerOptions": { "outDir": "dist", "rootDir": "src", "composite": true }
//// }

// @Filename: /packages/pkg-2/src/index.ts
//// import "./utils";

// @Filename: /packages/pkg-2/src/utils.ts
//// export const Pkg2 = {};

// @Filename: /packages/pkg-2/src/blah/foo/data.ts
//// Pkg2/*internal*/

// @link: /packages/pkg-2 -> /packages/pkg-1/node_modules/pkg-2

format.setOption("newline", "\n");

goTo.marker("external");
verify.importFixAtPosition([`import { Pkg2 } from "pkg-2/utils";\n\nPkg2`], /*errorCode*/ undefined, {
importModuleSpecifierPreference: "project-relative"
});

goTo.marker("internal");
verify.importFixAtPosition([`import { Pkg2 } from "../../utils";\n\nPkg2`], /*errorCode*/ undefined, {
importModuleSpecifierPreference: "project-relative"
});