From a0afacd262f7d51fedf2bc61a459a5316c898de5 Mon Sep 17 00:00:00 2001
From: Andrew Branch <andrewbranch@users.noreply.github.com>
Date: Wed, 8 Feb 2023 12:36:12 -0800
Subject: [PATCH] Avoid caching duplicate export info from AutoImportProvider
 (#52683)

---
 src/services/exportInfoMap.ts                 | 11 +++-
 .../unittests/tsserver/autoImportProvider.ts  | 53 ++++++++++++++++++-
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/services/exportInfoMap.ts b/src/services/exportInfoMap.ts
index d08bc6487e6..2d48f98141f 100644
--- a/src/services/exportInfoMap.ts
+++ b/src/services/exportInfoMap.ts
@@ -422,7 +422,16 @@ export function forEachExternalModuleToImportFrom(
     const autoImportProvider = useAutoImportProvider && host.getPackageJsonAutoImportProvider?.();
     if (autoImportProvider) {
         const start = timestamp();
-        forEachExternalModule(autoImportProvider.getTypeChecker(), autoImportProvider.getSourceFiles(), excludePatterns, (module, file) => cb(module, file, autoImportProvider, /*isFromPackageJson*/ true));
+        const checker = program.getTypeChecker();
+        forEachExternalModule(autoImportProvider.getTypeChecker(), autoImportProvider.getSourceFiles(), excludePatterns, (module, file) => {
+            if (file && !program.getSourceFile(file.fileName) || !file && !checker.resolveName(module.name, /*location*/ undefined, SymbolFlags.Module, /*excludeGlobals*/ false)) {
+                // The AutoImportProvider filters files already in the main program out of its *root* files,
+                // but non-root files can still be present in both programs, and already in the export info map
+                // at this point. This doesn't create any incorrect behavior, but is a waste of time and memory,
+                // so we filter them out here.
+                cb(module, file, autoImportProvider, /*isFromPackageJson*/ true);
+            }
+        });
         host.log?.(`forEachExternalModuleToImportFrom autoImportProvider: ${timestamp() - start}`);
     }
 }
diff --git a/src/testRunner/unittests/tsserver/autoImportProvider.ts b/src/testRunner/unittests/tsserver/autoImportProvider.ts
index 5fdd48039fe..ee27115d8a8 100644
--- a/src/testRunner/unittests/tsserver/autoImportProvider.ts
+++ b/src/testRunner/unittests/tsserver/autoImportProvider.ts
@@ -260,6 +260,41 @@ describe("unittests:: tsserver:: autoImportProvider", () => {
         const project = projectService.configuredProjects.get(tsconfig.path)!;
         assert.isUndefined(project.getPackageJsonAutoImportProvider());
     });
+
+    it("Shared source files between AutoImportProvider and main program do not cause duplicate entries in export info map", () => {
+        const files = [
+            // node_modules/memfs - AutoImportProvider only
+            { path: "/node_modules/memfs/package.json", content: `{ "name": "memfs", "version": "1.0.0", "types": "lib/index.d.ts" }` },
+            { path: "/node_modules/memfs/lib/index.d.ts", content: `/// <reference types="node" />\nexport declare class Volume {}` },
+
+            // node_modules/@types/node - AutoImportProvider and main program
+            { path: "/node_modules/@types/node/package.json", content: `{ "name": "@types/node", "version": "1.0.0" }` },
+            { path: "/node_modules/@types/node/index.d.ts", content: `export declare class Stats {}` },
+
+            // root
+            { path: "/package.json", content: `{ "dependencies": { "memfs": "*" }, "devDependencies": { "@types/node": "*" } }` },
+            { path: "/tsconfig.json", content: `{ "compilerOptions": { "types": ["node"] }` },
+            { path: "/index.ts", content: `export {};` },
+        ];
+
+        const { projectService, session, triggerCompletions } = setup(files);
+        openFilesForSession([files[files.length - 1]], session);
+        const project = projectService.configuredProjects.get("/tsconfig.json")!;
+        const autoImportProvider = project.getPackageJsonAutoImportProvider()!;
+        assert.isDefined(autoImportProvider);
+
+        // Trigger completions to ensure export info map is populated
+        triggerCompletions("/index.ts", 0, 0);
+        const exportInfoMap = project.getCachedExportInfoMap();
+        const seenSymbolNames = new Set<string>();
+        exportInfoMap.search("/index.ts" as ts.Path, /*preferCapitalized*/ false, ts.returnTrue, (info, symbolName) => {
+            assert.lengthOf(info, 1);
+            seenSymbolNames.add(symbolName);
+        });
+        assert.equal(seenSymbolNames.size, 2);
+        assert.ok(seenSymbolNames.has("Stats"));
+        assert.ok(seenSymbolNames.has("Volume"));
+    });
 });
 
 describe("unittests:: tsserver:: autoImportProvider - monorepo", () => {
@@ -333,7 +368,8 @@ function setup(files: File[]) {
         projectService,
         session,
         updateFile,
-        findAllReferences
+        findAllReferences,
+        triggerCompletions,
     };
 
     function updateFile(path: string, newText: string) {
@@ -360,4 +396,19 @@ function setup(files: File[]) {
             }
         });
     }
+
+    function triggerCompletions(file: string, line: number, offset: number) {
+        const requestLocation: ts.server.protocol.FileLocationRequestArgs = {
+            file,
+            line,
+            offset,
+        };
+        session.executeCommandSeq<ts.server.protocol.CompletionsRequest>({
+            command: ts.server.protocol.CommandTypes.CompletionInfo,
+            arguments: {
+                ...requestLocation,
+                includeExternalModuleExports: true,
+            }
+        });
+    }
 }