Skip to content

Commit 7c293c8

Browse files
andrewbranchandrewbranch
authored
Include actual generated module specifiers in module specifier cache (#44176)
* Add cache invalidation for node_modules, config, and preferences changes * Share watches with closed script info * Update API baseline * Small adjustments for fewer object allocations * Document overloaded return value * Update baselines * Store isAutoImportable separately from modulePaths * Add back missing return * Return wrapped watcher instead of underlying one * Make user preferences part of the cache key instead of implicitly clearing in editor services * Fix missed merge conflict
1 parent 130e16d commit 7c293c8

18 files changed

+348
-130
lines changed

‎src/compiler/moduleSpecifiers.ts

+44-17
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ namespace ts.moduleSpecifiers {
4747
host: ModuleSpecifierResolutionHost,
4848
oldImportSpecifier: string,
4949
): string | undefined {
50-
const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferencesForUpdate(compilerOptions, oldImportSpecifier));
50+
const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferencesForUpdate(compilerOptions, oldImportSpecifier), {});
5151
if (res === oldImportSpecifier) return undefined;
5252
return res;
5353
}
@@ -59,19 +59,19 @@ namespace ts.moduleSpecifiers {
5959
importingSourceFileName: Path,
6060
toFileName: string,
6161
host: ModuleSpecifierResolutionHost,
62-
preferences: UserPreferences = {},
6362
): string {
64-
return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferences(preferences, compilerOptions, importingSourceFile));
63+
return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferences({}, compilerOptions, importingSourceFile), {});
6564
}
6665

6766
export function getNodeModulesPackageName(
6867
compilerOptions: CompilerOptions,
6968
importingSourceFileName: Path,
7069
nodeModulesFileName: string,
7170
host: ModuleSpecifierResolutionHost,
71+
preferences: UserPreferences,
7272
): string | undefined {
7373
const info = getInfo(importingSourceFileName, host);
74-
const modulePaths = getAllModulePaths(importingSourceFileName, nodeModulesFileName, host);
74+
const modulePaths = getAllModulePaths(importingSourceFileName, nodeModulesFileName, host, preferences);
7575
return firstDefined(modulePaths,
7676
modulePath => tryGetModuleNameAsNodeModule(modulePath, info, host, compilerOptions, /*packageNameOnly*/ true));
7777
}
@@ -81,10 +81,11 @@ namespace ts.moduleSpecifiers {
8181
importingSourceFileName: Path,
8282
toFileName: string,
8383
host: ModuleSpecifierResolutionHost,
84-
preferences: Preferences
84+
preferences: Preferences,
85+
userPreferences: UserPreferences,
8586
): string {
8687
const info = getInfo(importingSourceFileName, host);
87-
const modulePaths = getAllModulePaths(importingSourceFileName, toFileName, host);
88+
const modulePaths = getAllModulePaths(importingSourceFileName, toFileName, host, userPreferences);
8889
return firstDefined(modulePaths, modulePath => tryGetModuleNameAsNodeModule(modulePath, info, host, compilerOptions)) ||
8990
getLocalModuleSpecifier(toFileName, info, compilerOptions, host, preferences);
9091
}
@@ -106,9 +107,17 @@ namespace ts.moduleSpecifiers {
106107
if (!moduleSourceFile) {
107108
return [];
108109
}
109-
const modulePaths = getAllModulePaths(importingSourceFile.path, moduleSourceFile.originalFileName, host);
110-
const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile);
111110

111+
const cache = host.getModuleSpecifierCache?.();
112+
const cached = cache?.get(importingSourceFile.path, moduleSourceFile.path, userPreferences);
113+
let modulePaths;
114+
if (cached) {
115+
if (cached.moduleSpecifiers) return cached.moduleSpecifiers;
116+
modulePaths = cached.modulePaths;
117+
}
118+
119+
modulePaths ||= getAllModulePathsWorker(importingSourceFile.path, moduleSourceFile.originalFileName, host);
120+
const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile);
112121
const existingSpecifier = forEach(modulePaths, modulePath => forEach(
113122
host.getFileIncludeReasons().get(toPath(modulePath.path, host.getCurrentDirectory(), info.getCanonicalFileName)),
114123
reason => {
@@ -120,7 +129,11 @@ namespace ts.moduleSpecifiers {
120129
undefined;
121130
}
122131
));
123-
if (existingSpecifier) return [existingSpecifier];
132+
if (existingSpecifier) {
133+
const moduleSpecifiers = [existingSpecifier];
134+
cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, modulePaths, moduleSpecifiers);
135+
return moduleSpecifiers;
136+
}
124137

125138
const importedFileIsInNodeModules = some(modulePaths, p => p.isInNodeModules);
126139

@@ -138,6 +151,7 @@ namespace ts.moduleSpecifiers {
138151
if (specifier && modulePath.isRedirect) {
139152
// If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar",
140153
// not "@foo/bar/path/to/file"). No other specifier will be this good, so stop looking.
154+
cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, modulePaths, nodeModulesSpecifiers!);
141155
return nodeModulesSpecifiers!;
142156
}
143157

@@ -161,9 +175,11 @@ namespace ts.moduleSpecifiers {
161175
}
162176
}
163177

164-
return pathsSpecifiers?.length ? pathsSpecifiers :
178+
const moduleSpecifiers = pathsSpecifiers?.length ? pathsSpecifiers :
165179
nodeModulesSpecifiers?.length ? nodeModulesSpecifiers :
166180
Debug.checkDefined(relativeSpecifiers);
181+
cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, modulePaths, moduleSpecifiers);
182+
return moduleSpecifiers;
167183
}
168184

169185
interface Info {
@@ -329,13 +345,27 @@ namespace ts.moduleSpecifiers {
329345
* Looks for existing imports that use symlinks to this module.
330346
* Symlinks will be returned first so they are preferred over the real path.
331347
*/
332-
function getAllModulePaths(importingFileName: Path, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] {
348+
function getAllModulePaths(
349+
importingFilePath: Path,
350+
importedFileName: string,
351+
host: ModuleSpecifierResolutionHost,
352+
preferences: UserPreferences,
353+
importedFilePath = toPath(importedFileName, host.getCurrentDirectory(), hostGetCanonicalFileName(host))
354+
) {
333355
const cache = host.getModuleSpecifierCache?.();
334-
const getCanonicalFileName = hostGetCanonicalFileName(host);
335356
if (cache) {
336-
const cached = cache.get(importingFileName, toPath(importedFileName, host.getCurrentDirectory(), getCanonicalFileName));
337-
if (typeof cached === "object") return cached;
357+
const cached = cache.get(importingFilePath, importedFilePath, preferences);
358+
if (cached?.modulePaths) return cached.modulePaths;
359+
}
360+
const modulePaths = getAllModulePathsWorker(importingFilePath, importedFileName, host);
361+
if (cache) {
362+
cache.setModulePaths(importingFilePath, importedFilePath, preferences, modulePaths);
338363
}
364+
return modulePaths;
365+
}
366+
367+
function getAllModulePathsWorker(importingFileName: Path, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] {
368+
const getCanonicalFileName = hostGetCanonicalFileName(host);
339369
const allFileNames = new Map<string, { path: string, isRedirect: boolean, isInNodeModules: boolean }>();
340370
let importedFileFromNodeModules = false;
341371
forEachFileNameOfModule(
@@ -381,9 +411,6 @@ namespace ts.moduleSpecifiers {
381411
sortedPaths.push(...remainingPaths);
382412
}
383413

384-
if (cache) {
385-
cache.set(importingFileName, toPath(importedFileName, host.getCurrentDirectory(), getCanonicalFileName), sortedPaths);
386-
}
387414
return sortedPaths;
388415
}
389416

‎src/compiler/transformers/declarations.ts

-1
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,6 @@ namespace ts {
380380
toPath(outputFilePath, host.getCurrentDirectory(), host.getCanonicalFileName),
381381
toPath(declFileName, host.getCurrentDirectory(), host.getCanonicalFileName),
382382
host,
383-
/*preferences*/ undefined,
384383
);
385384
if (!pathIsRelative(specifier)) {
386385
// If some compiler option/symlink/whatever allows access to the file containing the ambient module declaration

‎src/compiler/types.ts

+11-2
Original file line numberDiff line numberDiff line change
@@ -8155,10 +8155,19 @@ namespace ts {
81558155
isRedirect: boolean;
81568156
}
81578157

8158+
/*@internal*/
8159+
export interface ResolvedModuleSpecifierInfo {
8160+
modulePaths: readonly ModulePath[] | undefined;
8161+
moduleSpecifiers: readonly string[] | undefined;
8162+
isAutoImportable: boolean | undefined;
8163+
}
8164+
81588165
/* @internal */
81598166
export interface ModuleSpecifierCache {
8160-
get(fromFileName: Path, toFileName: Path): boolean | readonly ModulePath[] | undefined;
8161-
set(fromFileName: Path, toFileName: Path, moduleSpecifiers: boolean | readonly ModulePath[]): void;
8167+
get(fromFileName: Path, toFileName: Path, preferences: UserPreferences): Readonly<ResolvedModuleSpecifierInfo> | undefined;
8168+
set(fromFileName: Path, toFileName: Path, preferences: UserPreferences, modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[]): void;
8169+
setIsAutoImportable(fromFileName: Path, toFileName: Path, preferences: UserPreferences, isAutoImportable: boolean): void;
8170+
setModulePaths(fromFileName: Path, toFileName: Path, preferences: UserPreferences, modulePaths: readonly ModulePath[]): void;
81628171
clear(): void;
81638172
count(): number;
81648173
}

‎src/server/editorServices.ts

+71-36
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,11 @@ namespace ts.server {
589589
);
590590
}
591591

592-
interface ScriptInfoInNodeModulesWatcher extends FileWatcher {
593-
refCount: number;
592+
interface NodeModulesWatcher extends FileWatcher {
593+
/** How many watchers of this directory were for closed ScriptInfo */
594+
refreshScriptInfoRefCount: number;
595+
/** List of project names whose module specifier cache should be cleared when package.jsons change */
596+
affectedModuleSpecifierCacheProjects?: Set<string>;
594597
}
595598

596599
function getDetailWatchInfo(watchType: WatchType, project: Project | NormalizedPath | undefined) {
@@ -676,7 +679,7 @@ namespace ts.server {
676679
*/
677680
/*@internal*/
678681
readonly filenameToScriptInfo = new Map<string, ScriptInfo>();
679-
private readonly scriptInfoInNodeModulesWatchers = new Map<string, ScriptInfoInNodeModulesWatcher>();
682+
private readonly nodeModulesWatchers = new Map<string, NodeModulesWatcher>();
680683
/**
681684
* Contains all the deleted script info's version information so that
682685
* it does not reset when creating script info again
@@ -2626,59 +2629,87 @@ namespace ts.server {
26262629
}
26272630
}
26282631

2629-
private watchClosedScriptInfoInNodeModules(dir: Path): ScriptInfoInNodeModulesWatcher {
2630-
// Watch only directory
2631-
const existing = this.scriptInfoInNodeModulesWatchers.get(dir);
2632-
if (existing) {
2633-
existing.refCount++;
2634-
return existing;
2635-
}
2636-
2637-
const watchDir = dir + "/node_modules" as Path;
2632+
private createNodeModulesWatcher(dir: Path) {
26382633
const watcher = this.watchFactory.watchDirectory(
2639-
watchDir,
2634+
dir,
26402635
fileOrDirectory => {
26412636
const fileOrDirectoryPath = removeIgnoredPath(this.toPath(fileOrDirectory));
26422637
if (!fileOrDirectoryPath) return;
26432638

2644-
// Has extension
2645-
Debug.assert(result.refCount > 0);
2646-
if (watchDir === fileOrDirectoryPath) {
2647-
this.refreshScriptInfosInDirectory(watchDir);
2639+
// Clear module specifier cache for any projects whose cache was affected by
2640+
// dependency package.jsons in this node_modules directory
2641+
const basename = getBaseFileName(fileOrDirectoryPath);
2642+
if (result.affectedModuleSpecifierCacheProjects?.size && (
2643+
basename === "package.json" || basename === "node_modules"
2644+
)) {
2645+
result.affectedModuleSpecifierCacheProjects.forEach(projectName => {
2646+
this.findProject(projectName)?.getModuleSpecifierCache()?.clear();
2647+
});
26482648
}
2649-
else {
2650-
const info = this.getScriptInfoForPath(fileOrDirectoryPath);
2651-
if (info) {
2652-
if (isScriptInfoWatchedFromNodeModules(info)) {
2653-
this.refreshScriptInfo(info);
2654-
}
2649+
2650+
// Refresh closed script info after an npm install
2651+
if (result.refreshScriptInfoRefCount) {
2652+
if (dir === fileOrDirectoryPath) {
2653+
this.refreshScriptInfosInDirectory(dir);
26552654
}
2656-
// Folder
2657-
else if (!hasExtension(fileOrDirectoryPath)) {
2658-
this.refreshScriptInfosInDirectory(fileOrDirectoryPath);
2655+
else {
2656+
const info = this.getScriptInfoForPath(fileOrDirectoryPath);
2657+
if (info) {
2658+
if (isScriptInfoWatchedFromNodeModules(info)) {
2659+
this.refreshScriptInfo(info);
2660+
}
2661+
}
2662+
// Folder
2663+
else if (!hasExtension(fileOrDirectoryPath)) {
2664+
this.refreshScriptInfosInDirectory(fileOrDirectoryPath);
2665+
}
26592666
}
26602667
}
26612668
},
26622669
WatchDirectoryFlags.Recursive,
26632670
this.hostConfiguration.watchOptions,
2664-
WatchType.NodeModulesForClosedScriptInfo
2671+
WatchType.NodeModules
26652672
);
2666-
const result: ScriptInfoInNodeModulesWatcher = {
2673+
const result: NodeModulesWatcher = {
2674+
refreshScriptInfoRefCount: 0,
2675+
affectedModuleSpecifierCacheProjects: undefined,
26672676
close: () => {
2668-
if (result.refCount === 1) {
2677+
if (!result.refreshScriptInfoRefCount && !result.affectedModuleSpecifierCacheProjects?.size) {
26692678
watcher.close();
2670-
this.scriptInfoInNodeModulesWatchers.delete(dir);
2671-
}
2672-
else {
2673-
result.refCount--;
2679+
this.nodeModulesWatchers.delete(dir);
26742680
}
26752681
},
2676-
refCount: 1
26772682
};
2678-
this.scriptInfoInNodeModulesWatchers.set(dir, result);
2683+
this.nodeModulesWatchers.set(dir, result);
26792684
return result;
26802685
}
26812686

2687+
/*@internal*/
2688+
watchPackageJsonsInNodeModules(dir: Path, project: Project): FileWatcher {
2689+
const watcher = this.nodeModulesWatchers.get(dir) || this.createNodeModulesWatcher(dir);
2690+
(watcher.affectedModuleSpecifierCacheProjects ||= new Set()).add(project.getProjectName());
2691+
2692+
return {
2693+
close: () => {
2694+
watcher.affectedModuleSpecifierCacheProjects?.delete(project.getProjectName());
2695+
watcher.close();
2696+
},
2697+
};
2698+
}
2699+
2700+
private watchClosedScriptInfoInNodeModules(dir: Path): FileWatcher {
2701+
const watchDir = dir + "/node_modules" as Path;
2702+
const watcher = this.nodeModulesWatchers.get(watchDir) || this.createNodeModulesWatcher(watchDir);
2703+
watcher.refreshScriptInfoRefCount++;
2704+
2705+
return {
2706+
close: () => {
2707+
watcher.refreshScriptInfoRefCount--;
2708+
watcher.close();
2709+
},
2710+
};
2711+
}
2712+
26822713
private getModifiedTime(info: ScriptInfo) {
26832714
return (this.host.getModifiedTime!(info.path) || missingFileModifiedTime).getTime();
26842715
}
@@ -2954,7 +2985,11 @@ namespace ts.server {
29542985
this.logger.info("Format host information updated");
29552986
}
29562987
if (args.preferences) {
2957-
const { lazyConfiguredProjectsFromExternalProject, includePackageJsonAutoImports } = this.hostConfiguration.preferences;
2988+
const {
2989+
lazyConfiguredProjectsFromExternalProject,
2990+
includePackageJsonAutoImports,
2991+
} = this.hostConfiguration.preferences;
2992+
29582993
this.hostConfiguration.preferences = { ...this.hostConfiguration.preferences, ...args.preferences };
29592994
if (lazyConfiguredProjectsFromExternalProject && !this.hostConfiguration.preferences.lazyConfiguredProjectsFromExternalProject) {
29602995
// Load configured projects for external projects that are pending reload

‎src/server/project.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ namespace ts.server {
254254
/*@internal*/
255255
private changedFilesForExportMapCache: Set<Path> | undefined;
256256
/*@internal*/
257-
private moduleSpecifierCache = createModuleSpecifierCache();
257+
private moduleSpecifierCache = createModuleSpecifierCache(this);
258258
/*@internal*/
259259
private symlinks: SymlinkCache | undefined;
260260
/*@internal*/
@@ -790,6 +790,7 @@ namespace ts.server {
790790
this.resolutionCache.clear();
791791
this.resolutionCache = undefined!;
792792
this.cachedUnresolvedImportsPerFile = undefined!;
793+
this.moduleSpecifierCache = undefined!;
793794
this.directoryStructureHost = undefined!;
794795
this.projectErrors = undefined;
795796

@@ -1394,6 +1395,7 @@ namespace ts.server {
13941395
this.cachedUnresolvedImportsPerFile.clear();
13951396
this.lastCachedUnresolvedImportsList = undefined;
13961397
this.resolutionCache.clear();
1398+
this.moduleSpecifierCache.clear();
13971399
}
13981400
this.markAsDirty();
13991401
}
@@ -1735,6 +1737,11 @@ namespace ts.server {
17351737
this.projectService.openFiles,
17361738
(_, fileName) => this.projectService.tryGetDefaultProjectForFile(toNormalizedPath(fileName)) === this);
17371739
}
1740+
1741+
/*@internal*/
1742+
watchNodeModulesForPackageJsonChanges(directoryPath: string) {
1743+
return this.projectService.watchPackageJsonsInNodeModules(this.toPath(directoryPath), this);
1744+
}
17381745
}
17391746

17401747
function getUnresolvedImports(program: Program, cachedUnresolvedImportsPerFile: ESMap<Path, readonly string[]>): SortedReadonlyArray<string> {

0 commit comments

Comments
 (0)