From 2ac4cb78d6930302eb0a55d07f154a2b0597ae32 Mon Sep 17 00:00:00 2001
From: Andrew Branch <andrewbranch@users.noreply.github.com>
Date: Wed, 23 Oct 2024 13:22:23 -0700
Subject: [PATCH] Fix prioritization of `paths` specifiers over node_modules
 package specifiers (#60238)

---
 src/compiler/moduleSpecifiers.ts              | 74 +++++++++----------
 .../fourslash/autoImportPathsNodeModules.ts   | 26 +++++++
 2 files changed, 62 insertions(+), 38 deletions(-)
 create mode 100644 tests/cases/fourslash/autoImportPathsNodeModules.ts

diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts
index 5af3aab7d0f..eacba122e49 100644
--- a/src/compiler/moduleSpecifiers.ts
+++ b/src/compiler/moduleSpecifiers.ts
@@ -508,46 +508,44 @@ function computeModuleSpecifiers(
             }
         }
 
-        if (!specifier) {
-            const local = getLocalModuleSpecifier(
-                modulePath.path,
-                info,
-                compilerOptions,
-                host,
-                options.overrideImportMode || importingSourceFile.impliedNodeFormat,
-                preferences,
-                /*pathsOnly*/ modulePath.isRedirect,
-            );
-            if (!local || forAutoImport && isExcludedByRegex(local, preferences.excludeRegexes)) {
-                continue;
-            }
-            if (modulePath.isRedirect) {
-                redirectPathsSpecifiers = append(redirectPathsSpecifiers, local);
-            }
-            else if (pathIsBareSpecifier(local)) {
-                if (pathContainsNodeModules(local)) {
-                    // We could be in this branch due to inappropriate use of `baseUrl`, not intentional `paths`
-                    // usage. It's impossible to reason about where to prioritize baseUrl-generated module
-                    // specifiers, but if they contain `/node_modules/`, they're going to trigger a portability
-                    // error, so *at least* don't prioritize those.
-                    relativeSpecifiers = append(relativeSpecifiers, local);
-                }
-                else {
-                    pathsSpecifiers = append(pathsSpecifiers, local);
-                }
-            }
-            else if (forAutoImport || !importedFileIsInNodeModules || modulePath.isInNodeModules) {
-                // Why this extra conditional, not just an `else`? If some path to the file contained
-                // 'node_modules', but we can't create a non-relative specifier (e.g. "@foo/bar/path/to/file"),
-                // that means we had to go through a *sibling's* node_modules, not one we can access directly.
-                // If some path to the file was in node_modules but another was not, this likely indicates that
-                // we have a monorepo structure with symlinks. In this case, the non-node_modules path is
-                // probably the realpath, e.g. "../bar/path/to/file", but a relative path to another package
-                // in a monorepo is probably not portable. So, the module specifier we actually go with will be
-                // the relative path through node_modules, so that the declaration emitter can produce a
-                // portability error. (See declarationEmitReexportedSymlinkReference3)
+        const local = getLocalModuleSpecifier(
+            modulePath.path,
+            info,
+            compilerOptions,
+            host,
+            options.overrideImportMode || importingSourceFile.impliedNodeFormat,
+            preferences,
+            /*pathsOnly*/ modulePath.isRedirect || !!specifier,
+        );
+        if (!local || forAutoImport && isExcludedByRegex(local, preferences.excludeRegexes)) {
+            continue;
+        }
+        if (modulePath.isRedirect) {
+            redirectPathsSpecifiers = append(redirectPathsSpecifiers, local);
+        }
+        else if (pathIsBareSpecifier(local)) {
+            if (pathContainsNodeModules(local)) {
+                // We could be in this branch due to inappropriate use of `baseUrl`, not intentional `paths`
+                // usage. It's impossible to reason about where to prioritize baseUrl-generated module
+                // specifiers, but if they contain `/node_modules/`, they're going to trigger a portability
+                // error, so *at least* don't prioritize those.
                 relativeSpecifiers = append(relativeSpecifiers, local);
             }
+            else {
+                pathsSpecifiers = append(pathsSpecifiers, local);
+            }
+        }
+        else if (forAutoImport || !importedFileIsInNodeModules || modulePath.isInNodeModules) {
+            // Why this extra conditional, not just an `else`? If some path to the file contained
+            // 'node_modules', but we can't create a non-relative specifier (e.g. "@foo/bar/path/to/file"),
+            // that means we had to go through a *sibling's* node_modules, not one we can access directly.
+            // If some path to the file was in node_modules but another was not, this likely indicates that
+            // we have a monorepo structure with symlinks. In this case, the non-node_modules path is
+            // probably the realpath, e.g. "../bar/path/to/file", but a relative path to another package
+            // in a monorepo is probably not portable. So, the module specifier we actually go with will be
+            // the relative path through node_modules, so that the declaration emitter can produce a
+            // portability error. (See declarationEmitReexportedSymlinkReference3)
+            relativeSpecifiers = append(relativeSpecifiers, local);
         }
     }
 
diff --git a/tests/cases/fourslash/autoImportPathsNodeModules.ts b/tests/cases/fourslash/autoImportPathsNodeModules.ts
new file mode 100644
index 00000000000..18df371fb1d
--- /dev/null
+++ b/tests/cases/fourslash/autoImportPathsNodeModules.ts
@@ -0,0 +1,26 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: tsconfig.json
+//// {
+////     "compilerOptions": {
+////         "module": "amd",
+////         "moduleResolution": "node",
+////         "rootDir": "ts",
+////         "baseUrl": ".",
+////         "paths": {
+////             "*": ["node_modules/@woltlab/wcf/ts/*"]
+////         }
+////     },
+////     "include": [
+////         "ts",
+////         "node_modules/@woltlab/wcf/ts",
+////      ]
+//// }
+
+// @Filename: node_modules/@woltlab/wcf/ts/WoltLabSuite/Core/Component/Dialog.ts
+//// export class Dialog {}
+
+// @Filename: ts/main.ts
+//// Dialog/**/
+
+verify.importFixModuleSpecifiers("", ["WoltLabSuite/Core/Component/Dialog"]);