Skip to content
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

Use more nodelike paths for import types when possible #24610

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
3 changes: 2 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4085,7 +4085,8 @@ namespace ts {
// ambient module, just use declaration/symbol name (fallthrough)
}
else {
return `"${getResolvedExternalModuleName(context!.tracker.moduleResolverHost!, file, getSourceFileOfNode(getOriginalNode(context!.enclosingDeclaration)))}"`;
const contextFile = getSourceFileOfNode(getOriginalNode(context!.enclosingDeclaration))!;
return `"${file.moduleName || moduleSpecifiers.getModuleSpecifier(compilerOptions, contextFile, contextFile.path, file.path, context!.tracker.moduleResolverHost!)}"`;
}
}
const declaration = symbol.declarations[0];
Expand Down
4 changes: 0 additions & 4 deletions src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,6 @@ namespace ts {
}
}

export interface GetEffectiveTypeRootsHost {
directoryExists?(directoryName: string): boolean;
getCurrentDirectory?(): string;
}
export function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined {
if (options.typeRoots) {
return options.typeRoots;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
// Used by importFixes to synthesize import module specifiers.
/* @internal */
namespace ts.moduleSpecifiers {
export interface ModuleSpecifierPreferences {
importModuleSpecifierPreference?: "relative" | "non-relative";
}

// Note: fromSourceFile is just for usesJsExtensionOnImports
export function getModuleSpecifier(program: Program, fromSourceFile: SourceFile, fromSourceFileName: string, toFileName: string, host: LanguageServiceHost, preferences: UserPreferences) {
const info = getInfo(program.getCompilerOptions(), fromSourceFile, fromSourceFileName, host);
const compilerOptions = program.getCompilerOptions();
export function getModuleSpecifier(compilerOptions: CompilerOptions, fromSourceFile: SourceFile, fromSourceFileName: string, toFileName: string, host: ModuleSpecifierResolutionHost, preferences: ModuleSpecifierPreferences = {}) {
const info = getInfo(compilerOptions, fromSourceFile, fromSourceFileName, host);
return getGlobalModuleSpecifier(toFileName, info, host, compilerOptions) ||
first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences));
}
Expand All @@ -14,15 +17,15 @@ namespace ts.moduleSpecifiers {
moduleSymbol: Symbol,
program: Program,
importingSourceFile: SourceFile,
host: LanguageServiceHost,
preferences: UserPreferences,
host: ModuleSpecifierResolutionHost,
preferences: ModuleSpecifierPreferences,
): ReadonlyArray<ReadonlyArray<string>> {
const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol);
if (ambient) return [[ambient]];

const compilerOptions = program.getCompilerOptions();
const info = getInfo(compilerOptions, importingSourceFile, importingSourceFile.fileName, host);
const modulePaths = getAllModulePaths(program, moduleSymbol.valueDeclaration.getSourceFile());
const modulePaths = getAllModulePaths(program, getSourceFileOfNode(moduleSymbol.valueDeclaration));

const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions));
return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName =>
Expand All @@ -36,18 +39,18 @@ namespace ts.moduleSpecifiers {
readonly sourceDirectory: string;
}
// importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path
function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: string, host: LanguageServiceHost): Info {
function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: string, host: ModuleSpecifierResolutionHost): Info {
const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
const addJsExtension = usesJsExtensionOnImports(importingSourceFile);
const getCanonicalFileName = hostGetCanonicalFileName(host);
const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true);
const sourceDirectory = getDirectoryPath(importingSourceFileName);
return { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory };
}

function getGlobalModuleSpecifier(
moduleFileName: string,
{ addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
host: LanguageServiceHost,
host: ModuleSpecifierResolutionHost,
compilerOptions: CompilerOptions,
) {
return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, addJsExtension)
Expand All @@ -59,7 +62,7 @@ namespace ts.moduleSpecifiers {
moduleFileName: string,
{ moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
compilerOptions: CompilerOptions,
preferences: UserPreferences,
preferences: ModuleSpecifierPreferences,
) {
const { baseUrl, paths } = compilerOptions;

Expand Down Expand Up @@ -210,7 +213,7 @@ namespace ts.moduleSpecifiers {
function tryGetModuleNameAsNodeModule(
options: CompilerOptions,
moduleFileName: string,
host: LanguageServiceHost,
host: ModuleSpecifierResolutionHost,
getCanonicalFileName: (file: string) => string,
sourceDirectory: string,
): string | undefined {
Expand Down Expand Up @@ -256,14 +259,26 @@ namespace ts.moduleSpecifiers {
const fullModulePathWithoutExtension = removeFileExtension(path);

// If the file is /index, it can be imported by its directory name
if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index") {
// IFF there is not _also_ a file by the same name
if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index" && !tryGetAnyFileFromPath(host, fullModulePathWithoutExtension.substring(0, parts.fileNameIndex))) {
return fullModulePathWithoutExtension.substring(0, parts.fileNameIndex);
}

return fullModulePathWithoutExtension;
}
}

function tryGetAnyFileFromPath(host: ModuleSpecifierResolutionHost, path: string) {
// We check all js, `node` and `json` extensions in addition to TS, since node module resolution would also choose those over the directory
const extensions = getSupportedExtensions({ allowJs: true }, [{ extension: "node", isMixedContent: false }, { extension: "json", isMixedContent: false, scriptKind: ScriptKind.JSON }]);
for (const e of extensions) {
const fullPath = path + e;
if (host.fileExists!(fullPath)) { // TODO: GH#18217
return fullPath;
}
}
}

interface NodeModulePathParts {
readonly topLevelNodeModulesIndex: number;
readonly topLevelPackageNameIndex: number;
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,9 @@ namespace ts {
writeFile: writeFileCallback || (
(fileName, data, writeByteOrderMark, onError, sourceFiles) => host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles)),
isEmitBlocked,
readFile: f => host.readFile(f),
fileExists: f => host.fileExists(f),
...(host.directoryExists ? { directoryExists: f => host.directoryExists!(f) } : {}),
};
}

Expand Down
1 change: 1 addition & 0 deletions src/compiler/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"builderState.ts",
"builder.ts",
"resolutionCache.ts",
"moduleSpecifiers.ts",
"watch.ts",
"commandLineParser.ts",
"tsc.ts"
Expand Down
19 changes: 12 additions & 7 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5019,8 +5019,9 @@ namespace ts {
}

/* @internal */
export interface EmitHost extends ScriptReferenceHost {
export interface EmitHost extends ScriptReferenceHost, ModuleSpecifierResolutionHost {
getSourceFiles(): ReadonlyArray<SourceFile>;
getCurrentDirectory(): string;

/* @internal */
isSourceFileFromExternalLibrary(file: SourceFile): boolean;
Expand Down Expand Up @@ -5282,11 +5283,15 @@ namespace ts {
isAtStartOfLine(): boolean;
}

/* @internal */
export interface ModuleNameResolverHost {
getCanonicalFileName(f: string): string;
getCommonSourceDirectory(): string;
getCurrentDirectory(): string;
export interface GetEffectiveTypeRootsHost {
directoryExists?(directoryName: string): boolean;
getCurrentDirectory?(): string;
}
/** @internal */
export interface ModuleSpecifierResolutionHost extends GetEffectiveTypeRootsHost {
useCaseSensitiveFileNames?(): boolean;
fileExists?(path: string): boolean;
readFile?(path: string): string | undefined;
}

/** @deprecated See comment on SymbolWriter */
Expand All @@ -5300,7 +5305,7 @@ namespace ts {
reportPrivateInBaseOfClassExpression?(propertyName: string): void;
reportInaccessibleUniqueSymbolError?(): void;
/* @internal */
moduleResolverHost?: ModuleNameResolverHost;
moduleResolverHost?: ModuleSpecifierResolutionHost;
/* @internal */
trackReferencedAmbientModule?(decl: ModuleDeclaration): void;
}
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2934,11 +2934,11 @@ namespace ts {
};
}

export function getResolvedExternalModuleName(host: ModuleNameResolverHost, file: SourceFile, referenceFile?: SourceFile): string {
export function getResolvedExternalModuleName(host: EmitHost, file: SourceFile, referenceFile?: SourceFile): string {
return file.moduleName || getExternalModuleNameFromPath(host, file.fileName, referenceFile && referenceFile.fileName);
}

export function getExternalModuleNameFromDeclaration(host: ModuleNameResolverHost, resolver: EmitResolver, declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration | ModuleDeclaration | ImportTypeNode): string | undefined {
export function getExternalModuleNameFromDeclaration(host: EmitHost, resolver: EmitResolver, declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration | ModuleDeclaration | ImportTypeNode): string | undefined {
const file = resolver.getExternalModuleFileFromDeclaration(declaration);
if (!file || file.isDeclarationFile) {
return undefined;
Expand All @@ -2949,7 +2949,7 @@ namespace ts {
/**
* Resolves a local path to a path which is absolute to the base of the emit
*/
export function getExternalModuleNameFromPath(host: ModuleNameResolverHost, fileName: string, referencePath?: string): string {
export function getExternalModuleNameFromPath(host: EmitHost, fileName: string, referencePath?: string): string {
const getCanonicalFileName = (f: string) => host.getCanonicalFileName(f);
const dir = toPath(referencePath ? getDirectoryPath(referencePath) : host.getCommonSourceDirectory(), host.getCurrentDirectory(), getCanonicalFileName);
const filePath = getNormalizedAbsolutePath(fileName, host.getCurrentDirectory());
Expand Down
2 changes: 1 addition & 1 deletion src/harness/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"../compiler/builderState.ts",
"../compiler/builder.ts",
"../compiler/resolutionCache.ts",
"../compiler/moduleSpecifiers.ts",
"../compiler/watch.ts",
"../compiler/commandLineParser.ts",

Expand Down Expand Up @@ -115,7 +116,6 @@
"../services/codefixes/inferFromUsage.ts",
"../services/codefixes/fixInvalidImportSyntax.ts",
"../services/codefixes/fixStrictClassInitialization.ts",
"../services/codefixes/moduleSpecifiers.ts",
"../services/codefixes/requireInTs.ts",
"../services/codefixes/useDefaultImport.ts",
"../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
Expand Down
2 changes: 1 addition & 1 deletion src/server/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"../compiler/builderState.ts",
"../compiler/builder.ts",
"../compiler/resolutionCache.ts",
"../compiler/moduleSpecifiers.ts",
"../compiler/watch.ts",
"../compiler/commandLineParser.ts",

Expand Down Expand Up @@ -111,7 +112,6 @@
"../services/codefixes/inferFromUsage.ts",
"../services/codefixes/fixInvalidImportSyntax.ts",
"../services/codefixes/fixStrictClassInitialization.ts",
"../services/codefixes/moduleSpecifiers.ts",
"../services/codefixes/requireInTs.ts",
"../services/codefixes/useDefaultImport.ts",
"../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
Expand Down
2 changes: 1 addition & 1 deletion src/server/tsconfig.library.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"../compiler/builderState.ts",
"../compiler/builder.ts",
"../compiler/resolutionCache.ts",
"../compiler/moduleSpecifiers.ts",
"../compiler/watch.ts",
"../compiler/commandLineParser.ts",

Expand Down Expand Up @@ -117,7 +118,6 @@
"../services/codefixes/inferFromUsage.ts",
"../services/codefixes/fixInvalidImportSyntax.ts",
"../services/codefixes/fixStrictClassInitialization.ts",
"../services/codefixes/moduleSpecifiers.ts",
"../services/codefixes/requireInTs.ts",
"../services/codefixes/useDefaultImport.ts",
"../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
Expand Down
2 changes: 1 addition & 1 deletion src/services/getEditsForFileRename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ namespace ts {
// TODO:GH#18217
? getSourceFileToImportFromResolved(resolveModuleName(importLiteral.text, oldImportFromPath, program.getCompilerOptions(), host as ModuleResolutionHost), oldToNew, program)
: getSourceFileToImport(importLiteral, sourceFile, program, host, oldToNew);
return toImport === undefined ? undefined : moduleSpecifiers.getModuleSpecifier(program, sourceFile, newImportFromPath, toImport, host, preferences);
return toImport === undefined ? undefined : moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport, host, preferences);
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"../compiler/builderState.ts",
"../compiler/builder.ts",
"../compiler/resolutionCache.ts",
"../compiler/moduleSpecifiers.ts",
"../compiler/watch.ts",
"../compiler/commandLineParser.ts",

Expand Down Expand Up @@ -108,7 +109,6 @@
"codefixes/inferFromUsage.ts",
"codefixes/fixInvalidImportSyntax.ts",
"codefixes/fixStrictClassInitialization.ts",
"codefixes/moduleSpecifiers.ts",
"codefixes/requireInTs.ts",
"codefixes/useDefaultImport.ts",
"codefixes/fixAddModuleReferTypeMissingTypeof.ts",
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2876,6 +2876,10 @@ declare namespace ts {
omitTrailingSemicolon?: boolean;
noEmitHelpers?: boolean;
}
interface GetEffectiveTypeRootsHost {
directoryExists?(directoryName: string): boolean;
getCurrentDirectory?(): string;
}
/** @deprecated See comment on SymbolWriter */
interface SymbolTracker {
trackSymbol?(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags): void;
Expand Down Expand Up @@ -3467,10 +3471,6 @@ declare namespace ts {
function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile;
}
declare namespace ts {
interface GetEffectiveTypeRootsHost {
directoryExists?(directoryName: string): boolean;
getCurrentDirectory?(): string;
}
function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined;
/**
* @param {string | undefined} containingFile - file that contains type reference directive, can be undefined if containing file is unknown.
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2876,6 +2876,10 @@ declare namespace ts {
omitTrailingSemicolon?: boolean;
noEmitHelpers?: boolean;
}
interface GetEffectiveTypeRootsHost {
directoryExists?(directoryName: string): boolean;
getCurrentDirectory?(): string;
}
/** @deprecated See comment on SymbolWriter */
interface SymbolTracker {
trackSymbol?(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags): void;
Expand Down Expand Up @@ -3467,10 +3471,6 @@ declare namespace ts {
function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile;
}
declare namespace ts {
interface GetEffectiveTypeRootsHost {
directoryExists?(directoryName: string): boolean;
getCurrentDirectory?(): string;
}
function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined;
/**
* @param {string | undefined} containingFile - file that contains type reference directive, can be undefined if containing file is unknown.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//// [tests/cases/compiler/declarationEmitCommonJsModuleReferencedType.ts] ////

//// [index.d.ts]
export interface NestedProps {}
//// [index.d.ts]
export interface OtherIndexProps {}
//// [other.d.ts]
export interface OtherProps {}
//// [index.d.ts]
import { OtherProps } from "./other";
import { OtherIndexProps } from "./other/index";
import { NestedProps } from "nested";
export interface SomeProps {}

export function foo(): [SomeProps, OtherProps, OtherIndexProps, NestedProps];
//// [index.d.ts]
export interface RootProps {}

export function bar(): RootProps;
//// [entry.ts]
import { foo } from "foo";
import { bar } from "root";
export const x = foo();
export const y = bar();


//// [entry.js]
"use strict";
exports.__esModule = true;
var foo_1 = require("foo");
var root_1 = require("root");
exports.x = foo_1.foo();
exports.y = root_1.bar();


//// [entry.d.ts]
export declare const x: [import("foo").SomeProps, import("foo/other").OtherProps, import("foo/other/index").OtherIndexProps, import("foo/node_modules/nested").NestedProps];
export declare const y: import("root").RootProps;
Loading