Update cached import list update as part of resolving modules instead of another pass

Author: sheetalkamat

src/compiler/resolutionCache.ts

@@ -84,6 +84,14 @@ export interface HasInvalidatedFromResolutionCache {
     hasInvalidatedResolutions: HasInvalidatedResolutions;
     hasInvalidatedLibResolutions: HasInvalidatedLibResolutions;
 }
+/** @internal */
+export type CallbackOnNewResolution<T extends ResolutionWithFailedLookupLocations> = (
+    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
  *
@@ -103,8 +111,6 @@ export interface ResolutionCache {
     dirPathToSymlinkPackageRefCount: Map<Path, number>;
     countResolutionsResolvedWithGlobalCache(): number;
     countResolutionsResolvedWithoutGlobalCache(): number;
-    startRecordingFilesWithChangedResolutions(): void;
-    finishRecordingFilesWithChangedResolutions(): Path[] | undefined;
 
     watchFailedLookupLocationsOfExternalModuleResolutions<T extends ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName>(
         name: string,
@@ -121,6 +127,7 @@ export interface ResolutionCache {
         options: CompilerOptions,
         containingSourceFile: SourceFile,
         reusedNames: readonly StringLiteralLike[] | undefined,
+        onNewResolution?: CallbackOnNewResolution<ResolvedModuleWithFailedLookupLocations>,
     ): readonly ResolvedModuleWithFailedLookupLocations[];
     resolveTypeReferenceDirectiveReferences<T extends FileReference | string>(
         typeDirectiveReferences: readonly T[],
@@ -526,7 +533,8 @@ export function needsResolutionFromGlobalCache(moduleName: string, resolution: R
     return !isExternalModuleNameRelative(moduleName) && isUnresolvedOrResolvedToJs(resolution);
 }
 
-function isUnresolvedOrResolvedToJs(resolution: ResolvedModuleWithFailedLookupLocations) {
+/** @internal */
+export function isUnresolvedOrResolvedToJs(resolution: ResolvedModuleWithFailedLookupLocations) {
     return !resolution.resolvedModule || !resolutionExtensionIsTSOrJson(resolution.resolvedModule.extension);
 }
 
@@ -586,7 +594,6 @@ export function createResolutionCache(
     resolutionHost: ResolutionCacheHost,
     rootDirForResolution: string,
 ): ResolutionCache {
-    let filesWithChangedSetOfUnresolvedImports: Path[] | undefined;
     let filesWithInvalidatedResolutions: Set<Path> | undefined;
     const nonRelativeExternalModuleResolutions = createMultiMap<string, ResolutionWithFailedLookupLocations>();
 
@@ -668,8 +675,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,
@@ -735,16 +740,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,
@@ -875,8 +870,8 @@ export function createResolutionCache(
         perFileCache: Map<Path, ModeAwareCache<T>>;
         loader: ResolutionLoader<Entry, T, SourceFile>;
         getResolutionWithResolvedFileName: GetResolutionWithResolvedFileName<T, R>;
-        logChanges?: boolean;
         deferWatchingNonRelativeResolution: boolean;
+        onNewResolution?: CallbackOnNewResolution<T>;
     }
     function resolveNamesWithLocalCache<Entry, SourceFile, T extends ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName>({
         entries,
@@ -889,7 +884,7 @@ export function createResolutionCache(
         loader,
         getResolutionWithResolvedFileName,
         deferWatchingNonRelativeResolution,
-        logChanges,
+        onNewResolution,
     }: ResolveNamesWithLocalCacheInput<Entry, SourceFile, T, R>): readonly T[] {
         const path = resolutionHost.toPath(containingFile);
         const resolutionsInFile = perFileCache.get(path) || perFileCache.set(path, createModeAwareCache()).get(path)!;
@@ -930,12 +925,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);
@@ -982,24 +972,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<T extends FileReference | string>(
@@ -1037,6 +1009,7 @@ export function createResolutionCache(
         options: CompilerOptions,
         containingSourceFile: SourceFile,
         reusedNames: readonly StringLiteralLike[] | undefined,
+        onNewResolution?: CallbackOnNewResolution<ResolvedModuleWithFailedLookupLocations>,
     ): readonly ResolvedModuleWithFailedLookupLocations[] {
         return resolveNamesWithLocalCache({
             entries: moduleLiterals,
@@ -1054,8 +1027,8 @@ export function createResolutionCache(
                 moduleResolutionCache,
             ),
             getResolutionWithResolvedFileName: getResolvedModuleFromResolution,
-            logChanges: !!resolutionHost.getGlobalCache,
             deferWatchingNonRelativeResolution: true, // Defer non relative resolution watch because we could be using ambient modules
+            onNewResolution,
         });
     }
 

src/server/project.ts

@@ -74,7 +74,9 @@ import {
     InstallPackageOptions,
     IScriptSnapshot,
     isDeclarationFileName,
+    isExternalModuleNameRelative,
     isInsideNodeModules,
+    isUnresolvedOrResolvedToJs,
     JSDocParsingMode,
     JsTyping,
     LanguageService,
@@ -386,6 +388,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
      * @internal
      */
     cachedUnresolvedImportsPerFile = new Map<Path, readonly string[]>();
+    private recordChangesToUnresolvedImports = false;
 
     /** @internal */
     lastCachedUnresolvedImportsList: SortedReadonlyArray<string> | undefined;
@@ -784,13 +787,22 @@ 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,
             redirectedReference,
             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,
         );
     }
 
@@ -1430,26 +1442,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,

tests/baselines/reference/api/typescript.d.ts

@@ -2804,6 +2804,7 @@ declare namespace ts {
             private externalFiles;
             private missingFilesMap;
             private generatedFilesMap;
+            private recordChangesToUnresolvedImports;
             private hasAddedorRemovedFiles;
             private hasAddedOrRemovedSymlinks;
             protected languageService: LanguageService;