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

Check nearest package.json dependencies for possible package names for specifier candidates #58176

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
4 changes: 3 additions & 1 deletion src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,14 @@ export interface PackageJsonPathFields {
imports?: object;
exports?: object;
name?: string;
dependencies?: MapLike<string>;
peerDependencies?: MapLike<string>;
optionalDependencies?: MapLike<string>;
}

interface PackageJson extends PackageJsonPathFields {
name?: string;
version?: string;
peerDependencies?: MapLike<string>;
}

function readPackageJsonField<K extends MatchingKeys<PackageJson, string | undefined>>(jsonContent: PackageJson, fieldName: K, typeOfTag: "string", state: ModuleResolutionState): PackageJson[K] | undefined;
Expand Down
48 changes: 43 additions & 5 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
comparePaths,
Comparison,
CompilerOptions,
concatenate,
containsIgnoredPath,
containsPath,
createGetCanonicalFileName,
Expand Down Expand Up @@ -48,13 +49,15 @@ import {
getOwnKeys,
getPackageJsonTypesVersionsPaths,
getPackageNameFromTypesPackageName,
getPackageScopeForPath,
getPathsBasePath,
getRelativePathFromDirectory,
getRelativePathToDirectoryOrUrl,
getResolvePackageJsonExports,
getResolvePackageJsonImports,
getSourceFileOfModule,
getSupportedExtensions,
getTemporaryModuleResolutionState,
getTextOfIdentifierOrLiteral,
hasJSFileExtension,
hasTSFileExtension,
Expand Down Expand Up @@ -84,6 +87,7 @@ import {
ModuleDeclaration,
ModuleKind,
ModulePath,
ModuleResolutionHost,
ModuleResolutionKind,
ModuleSpecifierCache,
ModuleSpecifierEnding,
Expand All @@ -92,6 +96,7 @@ import {
NodeFlags,
NodeModulePathParts,
normalizePath,
PackageJsonPathFields,
pathContainsNodeModules,
pathIsBareSpecifier,
pathIsRelative,
Expand All @@ -102,6 +107,7 @@ import {
removeTrailingDirectorySeparator,
replaceFirstStar,
ResolutionMode,
resolveModuleName,
resolvePath,
ScriptKind,
shouldAllowImportingTsExtension,
Expand Down Expand Up @@ -254,7 +260,7 @@ export function getNodeModulesPackageName(
options: ModuleSpecifierOptions = {},
): string | undefined {
const info = getInfo(importingSourceFile.fileName, host);
const modulePaths = getAllModulePaths(info, nodeModulesFileName, host, preferences, options);
const modulePaths = getAllModulePaths(info, nodeModulesFileName, host, preferences, compilerOptions, options);
return firstDefined(modulePaths, modulePath => tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, preferences, /*packageNameOnly*/ true, options.overrideImportMode));
}

Expand All @@ -269,7 +275,7 @@ function getModuleSpecifierWorker(
options: ModuleSpecifierOptions = {},
): string {
const info = getInfo(importingSourceFileName, host);
const modulePaths = getAllModulePaths(info, toFileName, host, userPreferences, options);
const modulePaths = getAllModulePaths(info, toFileName, host, userPreferences, compilerOptions, options);
return firstDefined(modulePaths, modulePath => tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, userPreferences, /*packageNameOnly*/ undefined, options.overrideImportMode)) ||
getLocalModuleSpecifier(toFileName, info, compilerOptions, host, options.overrideImportMode || getDefaultResolutionModeForFile(importingSourceFile, host, compilerOptions), preferences);
}
Expand Down Expand Up @@ -361,7 +367,7 @@ export function getModuleSpecifiersWithCacheInfo(
if (!moduleSourceFile) return { moduleSpecifiers: emptyArray, computedWithoutCache };

computedWithoutCache = true;
modulePaths ||= getAllModulePathsWorker(getInfo(importingSourceFile.fileName, host), moduleSourceFile.originalFileName, host);
modulePaths ||= getAllModulePathsWorker(getInfo(importingSourceFile.fileName, host), moduleSourceFile.originalFileName, host, compilerOptions, options);
const result = computeModuleSpecifiers(
modulePaths,
compilerOptions,
Expand Down Expand Up @@ -691,6 +697,7 @@ function getAllModulePaths(
importedFileName: string,
host: ModuleSpecifierResolutionHost,
preferences: UserPreferences,
compilerOptions: CompilerOptions,
options: ModuleSpecifierOptions = {},
) {
const importingFilePath = toPath(info.importingSourceFileName, host.getCurrentDirectory(), hostGetCanonicalFileName(host));
Expand All @@ -700,14 +707,45 @@ function getAllModulePaths(
const cached = cache.get(importingFilePath, importedFilePath, preferences, options);
if (cached?.modulePaths) return cached.modulePaths;
}
const modulePaths = getAllModulePathsWorker(info, importedFileName, host);
const modulePaths = getAllModulePathsWorker(info, importedFileName, host, compilerOptions, options);
if (cache) {
cache.setModulePaths(importingFilePath, importedFilePath, preferences, options, modulePaths);
}
return modulePaths;
}

function getAllModulePathsWorker(info: Info, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] {
const runtimeDependencyFields = ["dependencies", "peerDependencies", "optionalDependencies"] as const;

function getAllRuntimeDependencies(packageJson: PackageJsonPathFields) {
let result;
for (const field of runtimeDependencyFields) {
const deps = packageJson[field];
if (deps && typeof deps === "object") {
result = concatenate(result, getOwnKeys(deps));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably just use append here, given getOwnKeys is going to give a fresh copy anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, wrong argument type. Nevermind.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd want addRange, not append, since append doesn't handle an array for input or take a spread list of arguments. But even that's not perfect - concatenate unconditionally allocates a new output array if both inputs are present (which is actually going to be rare here, usually it'll just be dependencies) - meanwhile addRange unconditionally copies from if to is missing (so it guarantees returning either to to a new array). That means it'd copy the first array needlessly every time. We actually don't have a "use either array mutably if they're present, it's fine"-type array-merging helper.

Honestly, it probably doesn't matter, though. Resizing one temporary array to merge in another one is probably going to trigger the similar allocations in the VM that making a new array would anyway (if they're both reasonably large). It's all short-lived objects.

}
}
return result;
}

function getAllModulePathsWorker(info: Info, importedFileName: string, host: ModuleSpecifierResolutionHost, compilerOptions: CompilerOptions, options: ModuleSpecifierOptions): readonly ModulePath[] {
const cache = host.getModuleResolutionCache?.();
const links = host.getSymlinkCache?.();
if (cache && links && host.readFile && !pathContainsNodeModules(info.importingSourceFileName)) {
Debug.type<ModuleResolutionHost>(host);
// Cache resolutions for all `dependencies` of the `package.json` context of the input file.
// This should populate all the relevant symlinks in the symlink cache, and most, if not all, of these resolutions
// should get (re)used.
const state = getTemporaryModuleResolutionState(cache.getPackageJsonInfoCache(), host, {});
const packageJson = getPackageScopeForPath(info.importingSourceFileName, state);
if (packageJson) {
const toResolve = getAllRuntimeDependencies(packageJson.contents.packageJsonContent);
for (const depName of (toResolve || emptyArray)) {
const resolved = resolveModuleName(depName, combinePaths(packageJson.packageDirectory, "package.json"), compilerOptions, host, cache, /*redirectedReference*/ undefined, options.overrideImportMode);
links.setSymlinksFromResolution(resolved.resolvedModule);
}
}
}

const allFileNames = new Map<string, { path: string; isRedirect: boolean; isInNodeModules: boolean; }>();
let importedFileFromNodeModules = false;
forEachFileNameOfModule(
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2698,12 +2698,15 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
// Before falling back to the host
return host.fileExists(f);
},
realpath: maybeBind(host, host.realpath),
useCaseSensitiveFileNames: () => host.useCaseSensitiveFileNames(),
getBuildInfo: () => program.getBuildInfo?.(),
getSourceFileFromReference: (file, ref) => program.getSourceFileFromReference(file, ref),
redirectTargetsMap,
getFileIncludeReasons: program.getFileIncludeReasons,
createHash: maybeBind(host, host.createHash),
getModuleResolutionCache: () => program.getModuleResolutionCache(),
trace: maybeBind(host, host.trace),
};
}

Expand Down
3 changes: 3 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9709,6 +9709,9 @@ export interface ModuleSpecifierResolutionHost {
getCommonSourceDirectory(): string;
getDefaultResolutionModeForFile(sourceFile: SourceFile): ResolutionMode;
getModeForResolutionAtIndex(file: SourceFile, index: number): ResolutionMode;

getModuleResolutionCache?(): ModuleResolutionCache | undefined;
trace?(s: string): void;
}

/** @internal */
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9080,6 +9080,7 @@ export interface SymlinkCache {
) => void,
typeReferenceDirectives: ModeAwareCache<ResolvedTypeReferenceDirectiveWithFailedLookupLocations>,
): void;
setSymlinksFromResolution(resolution: ResolvedModuleFull | undefined): void;
/**
* @internal
* Whether `setSymlinksFromResolutions` has already been called.
Expand Down Expand Up @@ -9119,6 +9120,9 @@ export function createSymlinkCache(cwd: string, getCanonicalFileName: GetCanonic
typeReferenceDirectives.forEach(resolution => processResolution(this, resolution.resolvedTypeReferenceDirective));
},
hasProcessedResolutions: () => hasProcessedResolutions,
setSymlinksFromResolution(resolution) {
processResolution(this, resolution);
},
hasAnySymlinks,
};

Expand Down
58 changes: 58 additions & 0 deletions src/testRunner/unittests/tsc/moduleResolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,62 @@ describe("unittests:: tsc:: moduleResolution::", () => {
}, { currentDirectory: "/home/src/projects/component-type-checker/packages/app" }),
commandLineArgs: ["--traceResolution", "--explainFiles"],
});

verifyTscWatch({
scenario: "moduleResolution",
subScenario: "late discovered dependency symlink",
sys: () =>
createWatchedSystem({
"/workspace/packageA/index.d.ts": dedent`
export declare class Foo {
private f: any;
}`,
"/workspace/packageB/package.json": dedent`
{
"private": true,
"dependencies": {
"package-a": "file:../packageA"
}
}`,
"/workspace/packageB/index.d.ts": dedent`
import { Foo } from "package-a";
export declare function invoke(): Foo;`,
"/workspace/packageC/package.json": dedent`
{
"private": true,
"dependencies": {
"package-b": "file:../packageB",
"package-a": "file:../packageA"
}
}`,
"/workspace/packageC/index.ts": dedent`
import * as pkg from "package-b";

export const a = pkg.invoke();`,
"/workspace/packageC/node_modules/package-a": { symLink: "/workspace/packageA" },
"/workspace/packageB/node_modules/package-a": { symLink: "/workspace/packageA" },
"/workspace/packageC/node_modules/package-b": { symLink: "/workspace/packageB" },
"/a/lib/lib.d.ts": libContent,
"/workspace/packageC/tsconfig.json": jsonToReadableText({
compilerOptions: {
declaration: true,
},
}),
}, { currentDirectory: "/workspace/packageC" }),
commandLineArgs: ["--traceResolution", "--explainFiles", "--watch"],
edits: [
{
caption: "change index.ts",
edit: fs =>
fs.writeFile(
"/workspace/packageC/index.ts",
dedent`
import * as pkg from "package-b";

export const aa = pkg.invoke();`,
),
timeouts: sys => sys.runQueuedTimeoutCallbacks(),
},
],
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//// [tests/cases/compiler/symlinkedWorkspaceDependenciesNoDirectLinkGeneratesDeepNonrelativeName.ts] ////

//// [foo.d.ts]
export declare class Foo {
private f: any;
}
//// [index.d.ts]
import { Foo } from "./foo.js";
export function create(): Foo;
//// [package.json]
{
"name": "package-a",
"version": "0.0.1",
"exports": {
".": "./index.js",
"./cls": "./foo.js"
}
}
//// [package.json]
{
"private": true,
"dependencies": {
"package-a": "file:../packageA"
}
}
//// [index.d.ts]
import { create } from "package-a";
export declare function invoke(): ReturnType<typeof create>;
//// [package.json]
{
"private": true,
"dependencies": {
"package-b": "file:../packageB",
"package-a": "file:../packageA"
}
}
//// [index.ts]
import * as pkg from "package-b";

export const a = pkg.invoke();

//// [index.js]
"use strict";
var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
var desc = Object.getOwnPropertyDescriptor(m, k);
if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
desc = { enumerable: true, get: function() { return m[k]; } };
}
Object.defineProperty(o, k2, desc);
}) : (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
o[k2] = m[k];
}));
var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) {
Object.defineProperty(o, "default", { enumerable: true, value: v });
}) : function(o, v) {
o["default"] = v;
});
var __importStar = (this && this.__importStar) || function (mod) {
if (mod && mod.__esModule) return mod;
var result = {};
if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k);
__setModuleDefault(result, mod);
return result;
};
Object.defineProperty(exports, "__esModule", { value: true });
exports.a = void 0;
const pkg = __importStar(require("package-b"));
exports.a = pkg.invoke();


//// [index.d.ts]
export declare const a: import("package-a/cls").Foo;
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//// [tests/cases/compiler/symlinkedWorkspaceDependenciesNoDirectLinkGeneratesDeepNonrelativeName.ts] ////

=== workspace/packageA/foo.d.ts ===
export declare class Foo {
>Foo : Symbol(Foo, Decl(foo.d.ts, 0, 0))

private f: any;
>f : Symbol(Foo.f, Decl(foo.d.ts, 0, 26))
}
=== workspace/packageA/index.d.ts ===
import { Foo } from "./foo.js";
>Foo : Symbol(Foo, Decl(index.d.ts, 0, 8))

export function create(): Foo;
>create : Symbol(create, Decl(index.d.ts, 0, 31))
>Foo : Symbol(Foo, Decl(index.d.ts, 0, 8))

=== workspace/packageB/index.d.ts ===
import { create } from "package-a";
>create : Symbol(create, Decl(index.d.ts, 0, 8))

export declare function invoke(): ReturnType<typeof create>;
>invoke : Symbol(invoke, Decl(index.d.ts, 0, 35))
>ReturnType : Symbol(ReturnType, Decl(lib.es5.d.ts, --, --))
>create : Symbol(create, Decl(index.d.ts, 0, 8))

=== workspace/packageC/index.ts ===
import * as pkg from "package-b";
>pkg : Symbol(pkg, Decl(index.ts, 0, 6))

export const a = pkg.invoke();
>a : Symbol(a, Decl(index.ts, 2, 12))
>pkg.invoke : Symbol(pkg.invoke, Decl(index.d.ts, 0, 35))
>pkg : Symbol(pkg, Decl(index.ts, 0, 6))
>invoke : Symbol(pkg.invoke, Decl(index.d.ts, 0, 35))

Loading