From dad88515644ee3ea078ce4b64141a92baba6d703 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 20 Oct 2023 13:10:34 -0700 Subject: [PATCH] Update cached import list update as part of resolving modules instead of another pass --- src/compiler/resolutionCache.ts | 59 +++++-------------- src/server/project.ts | 22 ++++--- tests/baselines/reference/api/typescript.d.ts | 1 + 3 files changed, 32 insertions(+), 50 deletions(-) diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index 3ec9c81b57a..1f36465f810 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -81,6 +81,14 @@ export interface HasInvalidatedFromResolutionCache { hasInvalidatedResolutions: HasInvalidatedResolutions; hasInvalidatedLibResolutions: HasInvalidatedLibResolutions; } +/** @internal */ +export type CallbackOnNewResolution = ( + existing: T | undefined, + current: T, + path: Path, + name: string, + mode: ResolutionMode, +) => void; /** * This is the cache of module/typedirectives resolution that can be retained across program * @@ -100,8 +108,6 @@ export interface ResolutionCache { dirPathToSymlinkPackageRefCount: Map; countResolutionsResolvedWithGlobalCache(): number; countResolutionsResolvedWithoutGlobalCache(): number; - startRecordingFilesWithChangedResolutions(): void; - finishRecordingFilesWithChangedResolutions(): Path[] | undefined; watchFailedLookupLocationsOfExternalModuleResolutions( name: string, @@ -118,6 +124,7 @@ export interface ResolutionCache { options: CompilerOptions, containingSourceFile: SourceFile, reusedNames: readonly StringLiteralLike[] | undefined, + onNewResolution?: CallbackOnNewResolution, ): readonly ResolvedModuleWithFailedLookupLocations[]; resolveTypeReferenceDirectiveReferences( typeDirectiveReferences: readonly T[], @@ -533,7 +540,8 @@ export function needsResolutionFromGlobalCache(moduleName: string, resolution: R return !isExternalModuleNameRelative(moduleName) && isUnresolvedOrResolvedToJs(resolution); } -function isUnresolvedOrResolvedToJs(resolution: ResolvedModuleWithFailedLookupLocations) { +/** @internal */ +export function isUnresolvedOrResolvedToJs(resolution: ResolvedModuleWithFailedLookupLocations): boolean { return !resolution.resolvedModule || !resolutionExtensionIsTSOrJson(resolution.resolvedModule.extension); } @@ -592,7 +600,6 @@ export function createResolutionCache( resolutionHost: ResolutionCacheHost, rootDirForResolution: string, ): ResolutionCache { - let filesWithChangedSetOfUnresolvedImports: Path[] | undefined; let filesWithInvalidatedResolutions: Set | undefined; const nonRelativeExternalModuleResolutions = new Set(); @@ -675,8 +682,6 @@ export function createResolutionCache( countResolutionsResolvedWithoutGlobalCache: () => resolutionsResolvedWithoutGlobalCache, watchFailedLookupLocationsOfExternalModuleResolutions, getModuleResolutionCache: () => moduleResolutionCache, - startRecordingFilesWithChangedResolutions, - finishRecordingFilesWithChangedResolutions, // perDirectoryResolvedModuleNames and perDirectoryResolvedTypeReferenceDirectives could be non empty if there was exception during program update // (between startCachingPerDirectoryResolution and finishCachingPerDirectoryResolution) startCachingPerDirectoryResolution, @@ -742,16 +747,6 @@ export function createResolutionCache( typeReferenceDirectiveResolutionCache.update(resolutionHost.getCompilationSettings()); } - function startRecordingFilesWithChangedResolutions() { - filesWithChangedSetOfUnresolvedImports = []; - } - - function finishRecordingFilesWithChangedResolutions() { - const collected = filesWithChangedSetOfUnresolvedImports; - filesWithChangedSetOfUnresolvedImports = undefined; - return collected; - } - function createHasInvalidatedResolutions( customHasInvalidatedResolutions: HasInvalidatedResolutions, customHasInvalidatedLibResolutions: HasInvalidatedLibResolutions, @@ -880,8 +875,8 @@ export function createResolutionCache( perFileCache: Map>; loader: ResolutionLoader; getResolutionWithResolvedFileName: GetResolutionWithResolvedFileName; - logChanges?: boolean; deferWatchingNonRelativeResolution: boolean; + onNewResolution?: CallbackOnNewResolution; } function resolveNamesWithLocalCache({ entries, @@ -894,7 +889,7 @@ export function createResolutionCache( loader, getResolutionWithResolvedFileName, deferWatchingNonRelativeResolution, - logChanges, + onNewResolution, }: ResolveNamesWithLocalCacheInput): readonly T[] { const path = resolutionHost.toPath(containingFile); const resolutionsInFile = perFileCache.get(path) || perFileCache.set(path, createModeAwareCache()).get(path)!; @@ -935,12 +930,7 @@ export function createResolutionCache( stopWatchFailedLookupLocationOfResolution(existingResolution, path, getResolutionWithResolvedFileName); } } - - if (logChanges && filesWithChangedSetOfUnresolvedImports && !resolutionIsEqualTo(existingResolution, resolution)) { - filesWithChangedSetOfUnresolvedImports.push(path); - // reset log changes to avoid recording the same file multiple times - logChanges = false; - } + onNewResolution?.(existingResolution, resolution, path, name, mode); } else { const host = getModuleResolutionHost(resolutionHost); @@ -987,24 +977,6 @@ export function createResolutionCache( }); } return resolvedModules; - - function resolutionIsEqualTo(oldResolution: T | undefined, newResolution: T | undefined): boolean { - if (oldResolution === newResolution) { - return true; - } - if (!oldResolution || !newResolution) { - return false; - } - const oldResult = getResolutionWithResolvedFileName(oldResolution); - const newResult = getResolutionWithResolvedFileName(newResolution); - if (oldResult === newResult) { - return true; - } - if (!oldResult || !newResult) { - return false; - } - return oldResult.resolvedFileName === newResult.resolvedFileName; - } } function resolveTypeReferenceDirectiveReferences( @@ -1042,6 +1014,7 @@ export function createResolutionCache( options: CompilerOptions, containingSourceFile: SourceFile, reusedNames: readonly StringLiteralLike[] | undefined, + onNewResolution?: CallbackOnNewResolution, ): readonly ResolvedModuleWithFailedLookupLocations[] { return resolveNamesWithLocalCache({ entries: moduleLiterals, @@ -1059,8 +1032,8 @@ export function createResolutionCache( moduleResolutionCache, ), getResolutionWithResolvedFileName: getResolvedModuleFromResolution, - logChanges: !!resolutionHost.getGlobalTypingsCacheLocation, deferWatchingNonRelativeResolution: true, // Defer non relative resolution watch because we could be using ambient modules + onNewResolution, }); } diff --git a/src/server/project.ts b/src/server/project.ts index b4ffe7e309f..3eb7cdb8fff 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -76,7 +76,9 @@ import { InstallPackageOptions, IScriptSnapshot, isDeclarationFileName, + isExternalModuleNameRelative, isInsideNodeModules, + isUnresolvedOrResolvedToJs, JSDocParsingMode, JsTyping, LanguageService, @@ -390,6 +392,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo * @internal */ cachedUnresolvedImportsPerFile: Map = new Map(); + private recordChangesToUnresolvedImports = false; /** @internal */ lastCachedUnresolvedImportsList: SortedReadonlyArray | undefined; @@ -796,6 +799,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo /** @internal */ resolveModuleNameLiterals(moduleLiterals: readonly StringLiteralLike[], containingFile: string, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingSourceFile: SourceFile, reusedNames: readonly StringLiteralLike[] | undefined): readonly ResolvedModuleWithFailedLookupLocations[] { + let needsUpdate = this.recordChangesToUnresolvedImports && this.cachedUnresolvedImportsPerFile.has(this.toPath(containingFile)); return this.resolutionCache.resolveModuleNameLiterals( moduleLiterals, containingFile, @@ -803,6 +807,14 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo options, containingSourceFile, reusedNames, + needsUpdate ? (existing, current, path, name) => { + if (!needsUpdate || isExternalModuleNameRelative(name)) return; + // If only unresolved flag is changed, update + if ((existing && isUnresolvedOrResolvedToJs(existing)) === isUnresolvedOrResolvedToJs(current)) return; + needsUpdate = false; + this.cachedUnresolvedImportsPerFile.delete(path); + this.lastCachedUnresolvedImportsList = undefined; + } : undefined, ); } @@ -1448,26 +1460,22 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo const useTypingsFromGlobalCache = this.useTypingsFromGlobalCache(); if (!useTypingsFromGlobalCache) this.resolutionCache.invalidateResolutionsWithGlobalCachePass(); else this.resolutionCache.invalidateResolutionsWithoutGlobalCachePass(); - if (useTypingsFromGlobalCache && this.cachedUnresolvedImportsPerFile.size) this.resolutionCache.startRecordingFilesWithChangedResolutions(); + if (useTypingsFromGlobalCache && this.cachedUnresolvedImportsPerFile.size) this.recordChangesToUnresolvedImports = true; const hasNewProgram = this.updateGraphWorker(); const hasAddedorRemovedFiles = this.hasAddedorRemovedFiles; this.hasAddedorRemovedFiles = false; this.hasAddedOrRemovedSymlinks = false; + this.recordChangesToUnresolvedImports = false; if (useTypingsFromGlobalCache) { - const changedFiles: readonly Path[] = this.resolutionCache.finishRecordingFilesWithChangedResolutions() || emptyArray; - for (const file of changedFiles) { - // delete cached information for changed files - this.cachedUnresolvedImportsPerFile.delete(file); - } // 1. no changes in structure, no changes in unresolved imports - do nothing // 2. no changes in structure, unresolved imports were changed - collect unresolved imports for all files // (can reuse cached imports for files that were not changed) // 3. new files were added/removed, but compilation settings stays the same - collect unresolved imports for all new/modified files // (can reuse cached imports for files that were not changed) // 4. compilation settings were changed in the way that might affect module resolution - drop all caches and collect all data from the scratch - if (!this.lastCachedUnresolvedImportsList || hasNewProgram || changedFiles.length) { + if (!this.lastCachedUnresolvedImportsList || hasNewProgram) { this.lastCachedUnresolvedImportsList = getUnresolvedImports( this.program!, this.cachedUnresolvedImportsPerFile, diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 36a50307085..c2a6a5f1706 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -2862,6 +2862,7 @@ declare namespace ts { private externalFiles; private missingFilesMap; private generatedFilesMap; + private recordChangesToUnresolvedImports; private hasAddedorRemovedFiles; private hasAddedOrRemovedSymlinks; protected languageService: LanguageService;