When the imported module is through node_modules and symlink to folder that isnt node_modules

Most of the monorepo like scenarios are like this so looking at symlink to decide if file can be imported is essential
Fixes #28689

Author: sheetalkamat

src/compiler/moduleSpecifiers.ts

@@ -178,40 +178,70 @@ namespace ts.moduleSpecifiers {
         );
     }
 
-    /**
-     * Looks for existing imports that use symlinks to this module.
-     * Symlinks will be returned first so they are preferred over the real path.
-     */
-    function getAllModulePaths(files: readonly SourceFile[], importingFileName: string, importedFileName: string, getCanonicalFileName: GetCanonicalFileName, host: ModuleSpecifierResolutionHost, redirectTargetsMap: RedirectTargetsMap): readonly string[] {
+    export function forEachFileNameOfModule<T>(
+        files: readonly SourceFile[],
+        importingFileName: string,
+        importedFileName: string,
+        getCanonicalFileName: GetCanonicalFileName,
+        host: ModuleSpecifierResolutionHost,
+        redirectTargetsMap: RedirectTargetsMap,
+        preferSymlinks: boolean,
+        cb: (fileName: string) => T | undefined
+    ): T | undefined {
         const redirects = redirectTargetsMap.get(importedFileName);
         const importedFileNames = redirects ? [...redirects, importedFileName] : [importedFileName];
         const cwd = host.getCurrentDirectory ? host.getCurrentDirectory() : "";
         const targets = importedFileNames.map(f => getNormalizedAbsolutePath(f, cwd));
+        if (!preferSymlinks) {
+            const result = forEach(targets, cb);
+            if (result) return result;
+        }
         const links = host.getProbableSymlinks
             ? host.getProbableSymlinks(files)
             : discoverProbableSymlinks(files, getCanonicalFileName, cwd);
 
-        const result: string[] = [];
         const compareStrings = (!host.useCaseSensitiveFileNames || host.useCaseSensitiveFileNames()) ? compareStringsCaseSensitive : compareStringsCaseInsensitive;
-        links.forEach((resolved, path) => {
+        const result = forEachEntry(links, (resolved, path) => {
             if (startsWithDirectory(importingFileName, resolved, getCanonicalFileName)) {
-                return; // Don't want to a package to globally import from itself
+                return undefined; // Don't want to a package to globally import from itself
             }
 
             const target = find(targets, t => compareStrings(t.slice(0, resolved.length + 1), resolved + "/") === Comparison.EqualTo);
-            if (target === undefined) return;
+            if (target === undefined) return undefined;
 
             const relative = getRelativePathFromDirectory(resolved, target, getCanonicalFileName);
             const option = resolvePath(path, relative);
             if (!host.fileExists || host.fileExists(option)) {
-                result.push(option);
+                const result = cb(option);
+                if (result) return result;
             }
         });
-        result.push(...targets);
-        if (result.length < 2) return result;
+        return result ||
+            (preferSymlinks ? forEach(targets, cb) : undefined);
+    }
+
+    /**
+     * Looks for existing imports that use symlinks to this module.
+     * Symlinks will be returned first so they are preferred over the real path.
+     */
+    function getAllModulePaths(files: readonly SourceFile[], importingFileName: string, importedFileName: string, getCanonicalFileName: GetCanonicalFileName, host: ModuleSpecifierResolutionHost, redirectTargetsMap: RedirectTargetsMap): readonly string[] {
+        const cwd = host.getCurrentDirectory ? host.getCurrentDirectory() : "";
+        const allFileNames = createMap<string>();
+        forEachFileNameOfModule(
+            files,
+            importingFileName,
+            importedFileName,
+            getCanonicalFileName,
+            host,
+            redirectTargetsMap,
+            /*preferSymlinks*/ true,
+            path => {
+                // dont return value, so we collect everything
+                allFileNames.set(path, getCanonicalFileName(path));
+            }
+        );
 
         // Sort by paths closest to importing file Name directory
-        const allFileNames = arrayToMap(result, identity, getCanonicalFileName);
         const sortedPaths: string[] = [];
         for (
             let directory = getDirectoryPath(toPath(importingFileName, cwd, getCanonicalFileName));

src/services/codefixes/importFixes.ts

@@ -786,9 +786,9 @@ namespace ts.codefix {
         cb: (module: Symbol) => void,
     ) {
         let filteredCount = 0;
-        const packageJson = filterByPackageJson && createAutoImportFilter(from, program, host);
+        const moduleSpecifierResolutionHost = createModuleSpecifierResolutionHost(program, host);
+        const packageJson = filterByPackageJson && createAutoImportFilter(from, program, host, moduleSpecifierResolutionHost);
         const allSourceFiles = program.getSourceFiles();
-        const globalTypingsCache = host.getGlobalTypingsCacheLocation && host.getGlobalTypingsCacheLocation();
         forEachExternalModule(program.getTypeChecker(), allSourceFiles, (module, sourceFile) => {
             if (sourceFile === undefined) {
                 if (!packageJson || packageJson.allowsImportingAmbientModule(module, allSourceFiles)) {
@@ -800,7 +800,7 @@ namespace ts.codefix {
             }
             else if (sourceFile &&
                 sourceFile !== from &&
-                isImportablePath(from.fileName, sourceFile.fileName, hostGetCanonicalFileName(host), globalTypingsCache)
+                isImportableFile(program, from, sourceFile, moduleSpecifierResolutionHost)
             ) {
                 if (!packageJson || packageJson.allowsImportingSourceFile(sourceFile, allSourceFiles)) {
                     cb(module);
@@ -826,6 +826,32 @@ namespace ts.codefix {
         }
     }
 
+    function isImportableFile(
+        program: Program,
+        from: SourceFile,
+        to: SourceFile,
+        moduleSpecifierResolutionHost: ModuleSpecifierResolutionHost
+    ) {
+        const getCanonicalFileName = hostGetCanonicalFileName(moduleSpecifierResolutionHost);
+        const globalTypingsCache = moduleSpecifierResolutionHost.getGlobalTypingsCacheLocation?.();
+        return !!moduleSpecifiers.forEachFileNameOfModule(
+            program.getSourceFiles(),
+            from.fileName,
+            to.fileName,
+            hostGetCanonicalFileName(moduleSpecifierResolutionHost),
+            moduleSpecifierResolutionHost,
+            program.redirectTargetsMap,
+            /*preferSymlinks*/ false,
+            toPath => {
+                const toFile = program.getSourceFile(toPath);
+                // Determine to import using toPath only if toPath is what we were looking at
+                // or there doesnt exist the file in the program by the symlink
+                return (toFile === to || !toFile) &&
+                    isImportablePath(from.fileName, toPath, getCanonicalFileName, globalTypingsCache);
+            }
+        );
+    }
+
     /**
      * Don't include something from a `node_modules` that isn't actually reachable by a global import.
      * A relative import to node_modules is usually a bad idea.
@@ -870,12 +896,10 @@ namespace ts.codefix {
         return !isStringANonContextualKeyword(res) ? res || "_" : `_${res}`;
     }
 
-    function createAutoImportFilter(fromFile: SourceFile, program: Program, host: LanguageServiceHost) {
-        const packageJsons = host.getPackageJsonsVisibleToFile && host.getPackageJsonsVisibleToFile(fromFile.fileName) || getPackageJsonsVisibleToFile(fromFile.fileName, host);
-        const dependencyGroups = PackageJsonDependencyGroup.Dependencies | PackageJsonDependencyGroup.DevDependencies | PackageJsonDependencyGroup.OptionalDependencies;
+    function createModuleSpecifierResolutionHost(program: Program, host: LanguageServiceHost): ModuleSpecifierResolutionHost {
         // Mix in `getProbablySymlinks` from Program when host doesn't have it
         // in order for non-Project hosts to have a symlinks cache.
-        const moduleSpecifierResolutionHost: ModuleSpecifierResolutionHost = {
+        return {
             directoryExists: maybeBind(host, host.directoryExists),
             fileExists: maybeBind(host, host.fileExists),
             getCurrentDirectory: maybeBind(host, host.getCurrentDirectory),
@@ -884,9 +908,14 @@ namespace ts.codefix {
             getProbableSymlinks: maybeBind(host, host.getProbableSymlinks) || program.getProbableSymlinks,
             getGlobalTypingsCacheLocation: maybeBind(host, host.getGlobalTypingsCacheLocation),
         };
+    }
+
+    function createAutoImportFilter(fromFile: SourceFile, program: Program, host: LanguageServiceHost, moduleSpecifierResolutionHost = createModuleSpecifierResolutionHost(program, host)) {
+        const packageJsons = host.getPackageJsonsVisibleToFile && host.getPackageJsonsVisibleToFile(fromFile.fileName) || getPackageJsonsVisibleToFile(fromFile.fileName, host);
+        const dependencyGroups = PackageJsonDependencyGroup.Dependencies | PackageJsonDependencyGroup.DevDependencies | PackageJsonDependencyGroup.OptionalDependencies;
 
         let usesNodeCoreModules: boolean | undefined;
-        return { allowsImportingAmbientModule, allowsImportingSourceFile, allowsImportingSpecifier };
+        return { allowsImportingAmbientModule, allowsImportingSourceFile, allowsImportingSpecifier, moduleSpecifierResolutionHost };
 
         function moduleSpecifierIsCoveredByPackageJson(specifier: string) {
             const packageName = getNodeModuleRootSpecifier(specifier);