Skip to content

Commit ed80039

Browse files
sheetalkamatsheetalkamat
committed
Verify resolution cache after resolveSingleModuleNameWithoutWatching
// This shows issue with test where package json info is added to cache and thats retained without watching
1 parent f6937f7 commit ed80039

File tree

5 files changed

+137
-35
lines changed

5 files changed

+137
-35
lines changed

‎src/compiler/moduleNameResolver.ts

+4
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,7 @@ export interface PerDirectoryResolutionCache<T> {
850850
* This updates the redirects map as well if needed so module resolutions are cached if they can across the projects
851851
*/
852852
update(options: CompilerOptions): void;
853+
/** @internal */ directoryToModuleNameMap: CacheWithRedirects<Path, ModeAwareCache<T>>;
853854
}
854855

855856
export interface NonRelativeNameResolutionCache<T> {
@@ -920,6 +921,7 @@ export interface CacheWithRedirects<K, V> {
920921
getOrCreateMapOfCacheRedirects(redirectedReference: ResolvedProjectReference | undefined): Map<K, V>;
921922
update(newOptions: CompilerOptions): void;
922923
clear(): void;
924+
getOwnMap(): Map<K, V>;
923925
}
924926

925927
/** @internal */
@@ -936,6 +938,7 @@ export function createCacheWithRedirects<K, V>(ownOptions: CompilerOptions | und
936938
getOrCreateMapOfCacheRedirects,
937939
update,
938940
clear,
941+
getOwnMap: () => ownMap,
939942
};
940943

941944
function getMapOfCacheRedirects(redirectedReference: ResolvedProjectReference | undefined): Map<K, V> | undefined {
@@ -1042,6 +1045,7 @@ function createPerDirectoryResolutionCache<T>(
10421045
getOrCreateCacheForDirectory,
10431046
clear,
10441047
update,
1048+
directoryToModuleNameMap,
10451049
};
10461050

10471051
function clear() {

‎src/compiler/resolutionCache.ts

+32-5
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,18 @@ export interface ResolutionCacheHost extends MinimalResolutionCacheHost {
205205
getCurrentProgram(): Program | undefined;
206206
fileIsOpen(filePath: Path): boolean;
207207
onDiscoveredSymlink?(): void;
208+
209+
// For incremental testing
210+
beforeResolveSingleModuleNameWithoutWatching?(
211+
moduleResolutionCache: ModuleResolutionCache,
212+
): any;
213+
afterResolveSingleModuleNameWithoutWatching?(
214+
moduleResolutionCache: ModuleResolutionCache,
215+
moduleName: string,
216+
containingFile: string,
217+
result: ResolvedModuleWithFailedLookupLocations,
218+
data: any,
219+
): any;
208220
}
209221

210222
/** @internal */
@@ -439,6 +451,10 @@ export function getRootPathSplitLength(rootPath: Path) {
439451
return rootPath.split(directorySeparator).length - (hasTrailingDirectorySeparator(rootPath) ? 1 : 0);
440452
}
441453

454+
function getModuleResolutionHost(resolutionHost: ResolutionCacheHost) {
455+
return resolutionHost.getCompilerHost?.() || resolutionHost;
456+
}
457+
442458
/** @internal */
443459
export function createModuleResolutionLoaderUsingGlobalCache(
444460
containingFile: string,
@@ -471,7 +487,7 @@ function resolveModuleNameUsingGlobalCache(
471487
redirectedReference?: ResolvedProjectReference,
472488
mode?: ResolutionMode,
473489
): ResolvedModuleWithFailedLookupLocations {
474-
const host = resolutionHost.getCompilerHost?.() || resolutionHost;
490+
const host = getModuleResolutionHost(resolutionHost);
475491
const primaryResult = ts_resolveModuleName(moduleName, containingFile, compilerOptions, host, moduleResolutionCache, redirectedReference, mode);
476492
// return result immediately only if global cache support is not enabled or if it is .ts, .tsx or .d.ts
477493
if (!resolutionHost.getGlobalCache) {
@@ -828,7 +844,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
828844
}
829845
}
830846
else {
831-
const host = resolutionHost.getCompilerHost?.() || resolutionHost;
847+
const host = getModuleResolutionHost(resolutionHost);
832848
if (isTraceEnabled(options, host) && !seenNamesInFile.has(name, mode)) {
833849
const resolved = getResolutionWithResolvedFileName(resolution!);
834850
trace(
@@ -912,7 +928,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
912928
containingFile,
913929
redirectedReference,
914930
options,
915-
resolutionHost.getCompilerHost?.() || resolutionHost,
931+
getModuleResolutionHost(resolutionHost),
916932
typeReferenceDirectiveResolutionCache,
917933
),
918934
getResolutionWithResolvedFileName: getResolvedTypeReferenceDirective,
@@ -957,7 +973,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
957973
options: CompilerOptions,
958974
libFileName: string,
959975
) {
960-
const host = resolutionHost.getCompilerHost?.() || resolutionHost;
976+
const host = getModuleResolutionHost(resolutionHost);
961977
let resolution = resolvedLibraries?.get(libFileName);
962978
if (!resolution || resolution.isInvalidated) {
963979
const existingResolution = resolution;
@@ -994,7 +1010,18 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
9941010
const resolutionsInFile = resolvedModuleNames.get(path);
9951011
const resolution = resolutionsInFile?.get(moduleName, /*mode*/ undefined);
9961012
if (resolution && !resolution.isInvalidated) return resolution;
997-
return resolveModuleNameUsingGlobalCache(resolutionHost, moduleResolutionCache, moduleName, containingFile, resolutionHost.getCompilationSettings());
1013+
const data = resolutionHost.beforeResolveSingleModuleNameWithoutWatching?.(moduleResolutionCache);
1014+
const host = getModuleResolutionHost(resolutionHost);
1015+
// We are not resolving d.ts so just normal resolution instead of doing resolution pass to global cache
1016+
const result = ts_resolveModuleName(
1017+
moduleName,
1018+
containingFile,
1019+
resolutionHost.getCompilationSettings(),
1020+
host,
1021+
moduleResolutionCache,
1022+
);
1023+
resolutionHost.afterResolveSingleModuleNameWithoutWatching?.(moduleResolutionCache, moduleName, containingFile, result, data);
1024+
return result;
9981025
}
9991026

10001027
function isNodeModulesAtTypesDirectory(dirPath: Path) {

‎src/harness/incrementalUtils.ts

+99-30
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ export function verifyResolutionCache(
264264
resolutionToExpected.get(resolution)!.refCount === resolution.refCount,
265265
`${projectName}:: Expected Resolution ref count ${resolutionToExpected.get(resolution)!.refCount} but got ${resolution.refCount}`,
266266
);
267-
verifySet(resolutionToExpected.get(resolution)!.files, resolution.files, `Resolution files`);
267+
verifySet(resolutionToExpected.get(resolution)!.files, resolution.files, `${projectName}:: Resolution files`);
268268
});
269269
verifyMapOfResolutionSet(expected.resolvedFileToResolution, actual.resolvedFileToResolution, `resolvedFileToResolution`);
270270
verifyResolutionSet(expected.resolutionsWithFailedLookups, actual.resolutionsWithFailedLookups, `resolutionsWithFailedLookups`);
@@ -339,35 +339,6 @@ export function verifyResolutionCache(
339339
return expectedResolution;
340340
}
341341

342-
function verifyMap<Expected, Actual>(
343-
expected: Map<string, Expected> | undefined,
344-
actual: Map<string, Actual> | undefined,
345-
verifyValue: (expected: Expected | undefined, actual: Actual | undefined, key: string) => void,
346-
caption: string,
347-
) {
348-
expected?.forEach((expected, path) => verifyValue(expected, actual?.get(path), `${caption}:: ${path}`));
349-
actual?.forEach((actual, path) => verifyValue(expected?.get(path), actual, `${caption}:: ${path}`));
350-
}
351-
352-
function verifySet(
353-
expected: Set<string> | undefined,
354-
actual: Set<string> | undefined,
355-
caption: string,
356-
) {
357-
expected?.forEach(expected =>
358-
ts.Debug.assert(
359-
actual?.has(expected),
360-
`${projectName}:: ${caption}:: Expected should be present in actual`,
361-
)
362-
);
363-
actual?.forEach(actual =>
364-
ts.Debug.assert(
365-
expected?.has(actual),
366-
`${projectName}:: ${caption}:: Actual should be present in expected`,
367-
)
368-
);
369-
}
370-
371342
function verifyMapOfResolutionSet(
372343
expected: Map<ts.Path, Set<ts.ResolutionWithFailedLookupLocations>> | undefined,
373344
actual: Map<ts.Path, Set<ts.ResolutionWithFailedLookupLocations>> | undefined,
@@ -421,6 +392,35 @@ export function verifyResolutionCache(
421392
}
422393
}
423394

395+
function verifyMap<Key extends string, Expected, Actual>(
396+
expected: Map<Key, Expected> | undefined,
397+
actual: Map<Key, Actual> | undefined,
398+
verifyValue: (expected: Expected | undefined, actual: Actual | undefined, key: string) => void,
399+
caption: string,
400+
) {
401+
expected?.forEach((expected, path) => verifyValue(expected, actual?.get(path), `${caption}:: ${path}`));
402+
actual?.forEach((actual, path) => verifyValue(expected?.get(path), actual, `${caption}:: ${path}`));
403+
}
404+
405+
function verifySet(
406+
expected: Set<string> | undefined,
407+
actual: Set<string> | undefined,
408+
caption: string,
409+
) {
410+
expected?.forEach(expected =>
411+
ts.Debug.assert(
412+
actual?.has(expected),
413+
`${caption}:: Expected should be present in actual`,
414+
)
415+
);
416+
actual?.forEach(actual =>
417+
ts.Debug.assert(
418+
expected?.has(actual),
419+
`${caption}:: Actual should be present in expected`,
420+
)
421+
);
422+
}
423+
424424
function verifyProgram(service: ts.server.ProjectService, project: ts.server.Project) {
425425
if (service.serverMode === ts.LanguageServiceMode.Syntactic) return;
426426
const options = project.getCompilerOptions();
@@ -510,6 +510,74 @@ function verifyProgram(service: ts.server.ProjectService, project: ts.server.Pro
510510
verifyResolutionCache(project.resolutionCache, project.getCurrentProgram()!, resolutionHostCacheHost, project.projectName);
511511
}
512512

513+
interface ResolveSingleModuleNameWithoutWatchingData {
514+
resolutionToData: Map<ts.ResolutionWithFailedLookupLocations, Pick<ts.ResolvedModuleWithFailedLookupLocations, "failedLookupLocations" | "affectingLocations" | "resolutionDiagnostics">>;
515+
packageJsonMap: Map<ts.Path, ts.PackageJsonInfo | boolean> | undefined;
516+
}
517+
518+
function beforeResolveSingleModuleNameWithoutWatching(
519+
moduleResolutionCache: ts.ModuleResolutionCache,
520+
): ResolveSingleModuleNameWithoutWatchingData {
521+
const resolutionToData: ResolveSingleModuleNameWithoutWatchingData["resolutionToData"] = new Map();
522+
// Currently it doesnt matter if moduleResolutionCache itself changes or not so just verify resolutions:
523+
moduleResolutionCache.directoryToModuleNameMap.getOwnMap().forEach(cache => {
524+
cache.forEach(resolution => {
525+
if (resolutionToData.has(resolution)) return;
526+
resolutionToData.set(resolution, {
527+
failedLookupLocations: resolution.failedLookupLocations?.slice(),
528+
affectingLocations: resolution.affectingLocations?.slice(),
529+
resolutionDiagnostics: resolution.resolutionDiagnostics?.slice(),
530+
});
531+
});
532+
});
533+
534+
// We also care about package json info cache
535+
const packageJsonMap = moduleResolutionCache.getPackageJsonInfoCache().getInternalMap();
536+
return {
537+
resolutionToData,
538+
packageJsonMap: packageJsonMap && new Map(packageJsonMap),
539+
};
540+
}
541+
542+
function afterResolveSingleModuleNameWithoutWatching(
543+
moduleResolutionCache: ts.ModuleResolutionCache,
544+
moduleName: string,
545+
containingFile: string,
546+
result: ts.ResolvedModuleWithFailedLookupLocations,
547+
data: ResolveSingleModuleNameWithoutWatchingData,
548+
) {
549+
const existing = data.resolutionToData.get(result);
550+
if (existing) {
551+
verifyArrayLength(existing.failedLookupLocations, result.failedLookupLocations, "failedLookupLocations");
552+
verifyArrayLength(existing.affectingLocations, result.affectingLocations, "affectingLocations");
553+
verifyArrayLength(existing.resolutionDiagnostics, result.resolutionDiagnostics, "resolutionDiagnostics");
554+
}
555+
556+
verifyMap(
557+
data.packageJsonMap,
558+
moduleResolutionCache.getPackageJsonInfoCache().getInternalMap(),
559+
(expected, actual, caption) => ts.Debug.assert(expected === actual, caption),
560+
`Expected packageJsonInfo to not change: ${moduleName} ${containingFile}`,
561+
);
562+
563+
function verifyArrayLength<T>(expected: T[] | undefined, actual: T[] | undefined, caption: string) {
564+
ts.Debug.assert(
565+
expected?.length === actual?.length,
566+
`Expected ${caption} to not change: ${moduleName} ${containingFile}`,
567+
() =>
568+
`Expected: ${JSON.stringify(expected, undefined, " ")}` +
569+
`Actual: ${JSON.stringify(actual, undefined, " ")}`,
570+
);
571+
}
572+
}
573+
574+
function onProjectCreation(project: ts.server.Project) {
575+
if (project.projectKind !== ts.server.ProjectKind.Auxiliary) return;
576+
577+
(project as ts.ResolutionCacheHost).beforeResolveSingleModuleNameWithoutWatching = beforeResolveSingleModuleNameWithoutWatching;
578+
(project as ts.ResolutionCacheHost).afterResolveSingleModuleNameWithoutWatching = afterResolveSingleModuleNameWithoutWatching;
579+
}
580+
513581
export interface IncrementalVerifierCallbacks {
514582
beforeVerification?(): any;
515583
afterVerification?(dataFromBefore: any): void;
@@ -518,6 +586,7 @@ export interface IncrementalVerifierCallbacks {
518586
export function incrementalVerifier(service: ts.server.ProjectService) {
519587
service.verifyDocumentRegistry = withIncrementalVerifierCallbacks(service, verifyDocumentRegistry);
520588
service.verifyProgram = withIncrementalVerifierCallbacks(service, verifyProgram);
589+
service.onProjectCreation = onProjectCreation;
521590
}
522591

523592
function withIncrementalVerifierCallbacks(

‎src/server/editorServices.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1131,6 +1131,7 @@ export class ProjectService {
11311131

11321132
/** @internal */ verifyDocumentRegistry = noop;
11331133
/** @internal */ verifyProgram: (project: Project) => void = noop;
1134+
/** @internal */ onProjectCreation: (project: Project) => void = noop;
11341135

11351136
readonly jsDocParsingMode: JSDocParsingMode | undefined;
11361137

‎src/server/project.ts

+1
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
582582
if (!isBackgroundProject(this)) {
583583
this.projectService.pendingEnsureProjectForOpenFiles = true;
584584
}
585+
this.projectService.onProjectCreation(this);
585586
}
586587

587588
isKnownTypesPackageName(name: string): boolean {

0 commit comments

Comments
 (0)