Skip to content

Tweak module resolution failed lookup watching #53591

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 11 commits into from
Apr 13, 2023
Merged
  •  
  •  
  •  
12 changes: 9 additions & 3 deletions src/compiler/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,9 @@ function pathComponents(path: string, rootLength: number) {
return [root, ...rest];
}

/** @internal */
export type PathPathComponents = Path[] & { __pathComponensBrand: any };

/**
* Parse a path into an array containing a root component (at index 0) and zero or more path
* components (at indices > 0). The result is not normalized.
Expand Down Expand Up @@ -484,6 +487,9 @@ function pathComponents(path: string, rootLength: number) {
*
* @internal
*/
export function getPathComponents(path: Path): PathPathComponents;
/** @internal */
export function getPathComponents(path: string, currentDirectory?: string): string[];
export function getPathComponents(path: string, currentDirectory = "") {
path = combinePaths(currentDirectory, path);
return pathComponents(path, getRootLength(path));
Expand All @@ -501,11 +507,11 @@ export function getPathComponents(path: string, currentDirectory = "") {
*
* @internal
*/
export function getPathFromPathComponents(pathComponents: readonly string[]) {
if (pathComponents.length === 0) return "";
export function getPathFromPathComponents<T extends string>(pathComponents: readonly T[], length?: number) {
if (pathComponents.length === 0) return "" as T;

const root = pathComponents[0] && ensureTrailingDirectorySeparator(pathComponents[0]);
return root + pathComponents.slice(1).join(directorySeparator);
return root + pathComponents.slice(1, length).join(directorySeparator) as T;
}

//// Path Normalization
Expand Down
285 changes: 140 additions & 145 deletions src/compiler/resolutionCache.ts

Large diffs are not rendered by default.

13 changes: 7 additions & 6 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
getFileNamesFromConfigSpecs,
getFileWatcherEventKind,
getNormalizedAbsolutePath,
getPathComponents,
getSnapshotText,
getWatchFactory,
hasExtension,
Expand Down Expand Up @@ -1892,7 +1893,7 @@ export class ProjectService {
// created when any of the script infos are added as root of inferred project
if (this.configFileExistenceImpactsRootOfInferredProject(configFileExistenceInfo)) {
// If we cannot watch config file existence without configured project, close the configured file watcher
if (!canWatchDirectoryOrFile(getDirectoryPath(canonicalConfigFilePath) as Path)) {
if (!canWatchDirectoryOrFile(getPathComponents(getDirectoryPath(canonicalConfigFilePath) as Path))) {
configFileExistenceInfo.watcher!.close();
configFileExistenceInfo.watcher = noopConfigFileWatcher;
}
Expand Down Expand Up @@ -1979,7 +1980,7 @@ export class ProjectService {
(configFileExistenceInfo.openFilesImpactedByConfigFile ||= new Map()).set(info.path, true);

// If there is no configured project for this config file, add the file watcher
configFileExistenceInfo.watcher ||= canWatchDirectoryOrFile(getDirectoryPath(canonicalConfigFilePath) as Path) ?
configFileExistenceInfo.watcher ||= canWatchDirectoryOrFile(getPathComponents(getDirectoryPath(canonicalConfigFilePath) as Path)) ?
this.watchFactory.watchFile(
configFileName,
(_filename, eventKind) => this.onConfigFileChanged(canonicalConfigFilePath, eventKind),
Expand Down Expand Up @@ -2703,12 +2704,12 @@ export class ProjectService {
}

// Single inferred project does not have a project root and hence no current directory
return this.createInferredProject(/*currentDirectory*/ undefined, /*isSingleInferredProject*/ true);
return this.createInferredProject("", /*isSingleInferredProject*/ true);
}

private getOrCreateSingleInferredWithoutProjectRoot(currentDirectory: string | undefined): InferredProject {
private getOrCreateSingleInferredWithoutProjectRoot(currentDirectory: string): InferredProject {
Debug.assert(!this.useSingleInferredProject);
const expectedCurrentDirectory = this.toCanonicalFileName(this.getNormalizedAbsolutePath(currentDirectory || ""));
const expectedCurrentDirectory = this.toCanonicalFileName(this.getNormalizedAbsolutePath(currentDirectory));
// Reuse the project with same current directory but no roots
for (const inferredProject of this.inferredProjects) {
if (!inferredProject.projectRootPath &&
Expand All @@ -2721,7 +2722,7 @@ export class ProjectService {
return this.createInferredProject(currentDirectory);
}

private createInferredProject(currentDirectory: string | undefined, isSingleInferredProject?: boolean, projectRootPath?: NormalizedPath): InferredProject {
private createInferredProject(currentDirectory: string, isSingleInferredProject?: boolean, projectRootPath?: NormalizedPath): InferredProject {
const compilerOptions = projectRootPath && this.compilerOptionsForInferredProjectsPerProjectRoot.get(projectRootPath) || this.compilerOptionsForInferredProjects!; // TODO: GH#18217
let watchOptionsAndErrors: WatchOptionsAndErrors | false | undefined;
let typeAcquisition: TypeAcquisition | undefined;
Expand Down
14 changes: 7 additions & 7 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,11 +465,11 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
public compileOnSaveEnabled: boolean,
protected watchOptions: WatchOptions | undefined,
directoryStructureHost: DirectoryStructureHost,
currentDirectory: string | undefined,
currentDirectory: string,
) {
this.projectName = projectName;
this.directoryStructureHost = directoryStructureHost;
this.currentDirectory = this.projectService.getNormalizedAbsolutePath(currentDirectory || "");
this.currentDirectory = this.projectService.getNormalizedAbsolutePath(currentDirectory);
this.getCanonicalFileName = this.projectService.toCanonicalFileName;

this.cancellationToken = new ThrottledCancellationToken(this.projectService.cancellationToken, this.projectService.throttleWaitMilliseconds);
Expand Down Expand Up @@ -514,7 +514,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
// Use the current directory as resolution root only if the project created using current directory string
this.resolutionCache = createResolutionCache(
this,
currentDirectory && this.currentDirectory,
this.currentDirectory,
/*logChangesWhenResolvingModule*/ true
);
this.languageService = createLanguageService(this, this.documentRegistry, this.projectService.serverMode);
Expand Down Expand Up @@ -2059,7 +2059,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
getNoDtsResolutionProject(rootFileNames: readonly string[]): Project {
Debug.assert(this.projectService.serverMode === LanguageServiceMode.Semantic);
if (!this.noDtsResolutionProject) {
this.noDtsResolutionProject = new AuxiliaryProject(this.projectService, this.documentRegistry, this.getCompilerOptionsForNoDtsResolutionProject());
this.noDtsResolutionProject = new AuxiliaryProject(this.projectService, this.documentRegistry, this.getCompilerOptionsForNoDtsResolutionProject(), this.currentDirectory);
}

enumerateInsertsAndDeletes<NormalizedPath, NormalizedPath>(
Expand Down Expand Up @@ -2176,7 +2176,7 @@ export class InferredProject extends Project {
compilerOptions: CompilerOptions,
watchOptions: WatchOptions | undefined,
projectRootPath: NormalizedPath | undefined,
currentDirectory: string | undefined,
currentDirectory: string,
pluginConfigOverrides: Map<string, any> | undefined,
typeAcquisition: TypeAcquisition | undefined) {
super(projectService.newInferredProjectName(),
Expand Down Expand Up @@ -2246,7 +2246,7 @@ export class InferredProject extends Project {
}

class AuxiliaryProject extends Project {
constructor(projectService: ProjectService, documentRegistry: DocumentRegistry, compilerOptions: CompilerOptions) {
constructor(projectService: ProjectService, documentRegistry: DocumentRegistry, compilerOptions: CompilerOptions, currentDirectory: string) {
super(projectService.newAuxiliaryProjectName(),
ProjectKind.Auxiliary,
projectService,
Expand All @@ -2257,7 +2257,7 @@ class AuxiliaryProject extends Project {
/*compileOnSaveEnabled*/ false,
/*watchOptions*/ undefined,
projectService.host,
/*currentDirectory*/ undefined);
currentDirectory);
}

override isOrphan(): boolean {
Expand Down
2 changes: 1 addition & 1 deletion src/services/rename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ function wouldRenameInOtherNodeModules(
}

function getPackagePathComponents(filePath: Path): string[] | undefined {
const components = getPathComponents(filePath);
const components = getPathComponents(filePath) as string[];
const nodeModulesIdx = components.lastIndexOf("node_modules");
if (nodeModulesIdx === -1) {
return undefined;
Expand Down
58 changes: 28 additions & 30 deletions src/testRunner/unittests/canWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe("unittests:: canWatch::", () => {
paths.forEach(path => baselineCanWatchDirectoryOrFile(combinePaths(path, "package.json"), maxLengths));
baseline.push("", "");
function baselineCanWatchDirectoryOrFile(path: ts.Path, maxLengths: readonly number[]) {
pushRow(baseline, [path, `${ts.canWatchDirectoryOrFile(path)}`], maxLengths);
pushRow(baseline, [path, `${ts.canWatchDirectoryOrFile(ts.getPathComponents(path))}`], maxLengths);
}
},
);
Expand All @@ -25,13 +25,12 @@ describe("unittests:: canWatch::", () => {
(paths, longestPathLength, baseline) => {
const testType = "canWatchAtTypes";
const maxLengths = [longestPathLength + "/node_modules/@types".length, testType.length] as const;
baselineCanWatchForRoot(paths, baseline, root => {
pushHeader(baseline, ["Directory", testType], maxLengths);
paths.forEach(path => {
path = combinePaths(path, "node_modules/@types");
pushRow(baseline, [path, `${ts.canWatchAtTypes(path, root)}`], maxLengths);
});
pushHeader(baseline, ["Directory", testType], maxLengths);
paths.forEach(path => {
path = combinePaths(path, "node_modules/@types");
pushRow(baseline, [path, `${ts.canWatchAtTypes(path)}`], maxLengths);
});
baseline.push("", "");
},
);

Expand All @@ -41,13 +40,12 @@ describe("unittests:: canWatch::", () => {
(paths, longestPathLength, baseline) => {
const testType = "canWatchAffectingLocation";
const maxLengths = [longestPathLength + "/package.json".length, testType.length] as const;
baselineCanWatchForRoot(paths, baseline, _root => {
pushHeader(baseline, ["File", testType], maxLengths);
paths.forEach(path => {
path = combinePaths(path, "package.json");
pushRow(baseline, [path, `${ts.canWatchAffectingLocation(path)}`], maxLengths);
});
pushHeader(baseline, ["File", testType], maxLengths);
paths.forEach(path => {
path = combinePaths(path, "package.json");
pushRow(baseline, [path, `${ts.canWatchAffectingLocation(path)}`], maxLengths);
});
baseline.push("", "");
},
);

Expand All @@ -65,21 +63,21 @@ describe("unittests:: canWatch::", () => {
const recursive = "Recursive";
const maxLength = longestPathLength + ts.combinePaths(forPath, "dir/subdir/somefile.d.ts").length;
const maxLengths = [maxLength, maxLength, recursive.length] as const;
baselineCanWatchForRoot(paths, baseline, root => {
baselineCanWatchForRoot(paths, baseline, (rootPathCompoments, root) => {
pushHeader(baseline, ["Location", "getDirectoryToWatchFailedLookupLocation", recursive], maxLengths);
paths.forEach(path => {
baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "somefile.d.ts"), root, maxLengths);
baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "dir/somefile.d.ts"), root, maxLengths);
baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "dir/subdir/somefile.d.ts"), root, maxLengths);
baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "somefile.d.ts"), root, rootPathCompoments, maxLengths);
baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "dir/somefile.d.ts"), root, rootPathCompoments, maxLengths);
baselineGetDirectoryToWatchFailedLookupLocation(combinePaths(path, forPath, "dir/subdir/somefile.d.ts"), root, rootPathCompoments, maxLengths);
});
});
function baselineGetDirectoryToWatchFailedLookupLocation(path: ts.Path, root: ts.Path | undefined, maxLengths: readonly number[]) {
function baselineGetDirectoryToWatchFailedLookupLocation(path: ts.Path, root: ts.Path, rootPathCompoments: Readonly<ts.PathPathComponents>, maxLengths: readonly number[]) {
const result = ts.getDirectoryToWatchFailedLookupLocation(
path,
path,
root,
root,
root !== undefined ? root.split(ts.directorySeparator).length : 0,
rootPathCompoments,
ts.returnUndefined,
);
pushRow(baseline, [path, result ? result.dir : "", result ? `${!result.nonRecursive}` : ""], maxLengths);
Expand All @@ -94,16 +92,18 @@ describe("unittests:: canWatch::", () => {
(paths, longestPathLength, baseline) => {
const maxLength = longestPathLength + "/node_modules/@types".length;
const maxLengths = [maxLength, maxLength] as const;
baselineCanWatchForRoot(paths, baseline, root => {
baselineCanWatchForRoot(paths, baseline, (rootPathCompoments, root) => {
pushHeader(baseline, ["Directory", "getDirectoryToWatchFailedLookupLocationFromTypeRoot"], maxLengths);
paths.forEach(path => {
path = combinePaths(path, "node_modules/@types");
// This is invoked only on paths that are watched
if (!ts.canWatchAtTypes(path, root)) return;
if (!ts.canWatchAtTypes(path)) return;
const result = ts.getDirectoryToWatchFailedLookupLocationFromTypeRoot(
path,
path,
root,
rootPathCompoments,
ts.returnUndefined,
ts.returnTrue,
);
pushRow(baseline, [path, result !== undefined ? result : ""], maxLengths);
Expand All @@ -112,16 +112,14 @@ describe("unittests:: canWatch::", () => {
},
);

function baselineCanWatchForRoot(paths: readonly ts.Path[], baseline: string[], baselineForRoot: (root: ts.Path | undefined) => void) {
paths.forEach(baselineRoot);
baselineRoot(/*rootDirForResolution*/ undefined);
baseline.push("", "");

function baselineRoot(rootDirForResolution: ts.Path | undefined) {
function baselineCanWatchForRoot(paths: readonly ts.Path[], baseline: string[], baselineForRoot: (rootPathCompoments: Readonly<ts.PathPathComponents>, root: ts.Path) => void) {
paths.forEach(rootDirForResolution => {
const root = ts.getRootDirectoryOfResolutionCache(rootDirForResolution, ts.returnUndefined) as ts.Path;
baseline.push("", `## RootDirForResolution: ${rootDirForResolution}`, "", `Root: ${root}`);
baselineForRoot(root);
}
assert(root === rootDirForResolution);
baseline.push("", `## RootDirForResolution: ${rootDirForResolution}`);
baselineForRoot(ts.getPathComponents(root), root);
});
baseline.push("", "");
}

function baselineCanWatch(
Expand Down
Loading