fix(cli): reduce tsconfig paths precedence below pnpm workspace proje…

…ct deps (#7060)

An issue found at a client when upgrading from an ancient cli version to
more recent.

This changes the order of how dependencies can be resolved, reducing the
precedence of tsconfig paths to be one of the lowest priorities instead
of being on-par with relative file imports.

The client use case and the test case added is when a tsconfig file has
`paths` essentially just for the IDE dev experience or maybe for legacy
non-bazel (or non-pnpm-workspace) reasons. Imports of local workspace
projects should use `:node_modules/@wksp/project-a` via package.json
instead of `//projects/a` vis tsconfig.json.

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

Pnpm workspace project dependencies resolved with `aspect configure` are
now higher priority then resolving dependencies via tsconfig paths.

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 7cf0f95489f5600001351d36a7f13500bc465b62

gazelle/js/generate.go

@@ -589,7 +589,6 @@ func (ts *typeScriptLang) collectProtoImports(cfg *JsGazelleConfig, args languag
 						Lang: LanguageName,
 						Imp:  dts,
 					},
-					Alt:        []string{},
 					ImportPath: imp,
 					SourcePath: sourceFile,
 				})
@@ -653,30 +652,25 @@ func (ts *typeScriptLang) collectImports(cfg *JsGazelleConfig, rootDir, sourcePa
 	}
 
 	for _, importPath := range parseResults.Imports {
+		if cfg.IsImportIgnored(importPath) {
+			BazelLog.Tracef("Import ignored: %q", importPath)
+			continue
+		}
+
 		// The path from the root
 		workspacePath := toImportSpecPath(sourcePath, importPath)
 
-		if !cfg.IsImportIgnored(importPath) {
-			alternates := make([]string, 0)
-			for _, alt := range ts.tsconfig.ExpandPaths(sourcePath, importPath) {
-				alternates = append(alternates, toImportSpecPath(sourcePath, alt))
-			}
-
-			// Record all imports. Maybe local, maybe data, maybe in other BUILD etc.
-			result.Imports = append(result.Imports, ImportStatement{
-				ImportSpec: resolve.ImportSpec{
-					Lang: LanguageName,
-					Imp:  workspacePath,
-				},
-				Alt:        alternates,
-				ImportPath: importPath,
-				SourcePath: sourcePath,
-			})
+		// Record all imports. Maybe local, maybe data, maybe in other BUILD etc.
+		result.Imports = append(result.Imports, ImportStatement{
+			ImportSpec: resolve.ImportSpec{
+				Lang: LanguageName,
+				Imp:  workspacePath,
+			},
+			ImportPath: importPath,
+			SourcePath: sourcePath,
+		})
 
-			BazelLog.Tracef("Import: %q -> %q (alt: %v)", workspacePath, importPath, alternates)
-		} else {
-			BazelLog.Tracef("Import ignored: %q -> %q", workspacePath, importPath)
-		}
+		BazelLog.Tracef("Import: %q -> %q (via %q)", sourcePath, workspacePath, importPath)
 	}
 
 	return result

gazelle/js/resolve.go

@@ -366,41 +366,9 @@ func (ts *typeScriptLang) resolveImport(
 		return Resolution_Override, res, nil
 	}
 
-	possible := make([]resolve.ImportSpec, 0, 1)
-	possible = append(possible, impStm.ImportSpec)
-	for _, expandedImp := range impStm.Alt {
-		possible = append(possible, resolve.ImportSpec{Lang: impStm.Lang, Imp: expandedImp})
-	}
-
-	// Gazelle rule index. Try each potential expanded path
-	for _, eImp := range possible {
-		if matches := ix.FindRulesByImportWithConfig(c, eImp, LanguageName); len(matches) > 0 {
-			filteredMatches := make([]label.Label, 0, len(matches))
-			for _, match := range matches {
-				// Prevent from adding itself as a dependency.
-				if !match.IsSelfImport(from) {
-					filteredMatches = append(filteredMatches, match.Label)
-				}
-			}
-
-			// Too many results, don't know which is correct
-			if len(filteredMatches) > 1 {
-				return Resolution_Error, nil, fmt.Errorf(
-					"Import %q from %q resolved to multiple targets (%s) - this must be fixed using the \"gazelle:resolve\" directive",
-					impStm.ImportPath, impStm.SourcePath, targetListFromResults(matches))
-			}
-
-			// The matches were self imports, no dependency is needed
-			if len(filteredMatches) == 0 {
-				return Resolution_None, nil, nil
-			}
-
-			match := filteredMatches[0]
-
-			BazelLog.Tracef("resolve %q import %q as %q", from, impStm.Imp, match)
-
-			return Resolution_Override, &match, nil
-		}
+	// Gazelle rule index
+	if resolution, match, err := ts.resolveImportFromIndex(c, ix, from, impStm); resolution != Resolution_NotFound {
+		return resolution, match, err
 	}
 
 	// References to a label such as a file or file-generating rule
@@ -413,6 +381,24 @@ func (ts *typeScriptLang) resolveImport(
 		return Resolution_Package, pkg, nil
 	}
 
+	// References via tsconfig mappings (paths, baseUrl, rootDirs etc.)
+	if tsconfigPaths := ts.tsconfig.ExpandPaths(impStm.SourcePath, impStm.ImportPath); len(tsconfigPaths) > 0 {
+		for _, p := range tsconfigPaths {
+			pImp := ImportStatement{
+				ImportSpec: resolve.ImportSpec{
+					Lang: impStm.ImportSpec.Lang,
+					Imp:  toImportSpecPath(impStm.SourcePath, p),
+				},
+				SourcePath: impStm.SourcePath,
+				ImportPath: impStm.ImportPath,
+				Optional:   impStm.Optional,
+			}
+			if resolution, match, err := ts.resolveImportFromIndex(c, ix, from, pImp); resolution != Resolution_NotFound {
+				return resolution, match, err
+			}
+		}
+	}
+
 	// Native node imports
 	if node.IsNodeImport(imp.Imp) {
 		return Resolution_NativeNode, nil, nil
@@ -421,6 +407,44 @@ func (ts *typeScriptLang) resolveImport(
 	return Resolution_NotFound, nil, nil
 }
 
+func (ts *typeScriptLang) resolveImportFromIndex(
+	c *config.Config,
+	ix *resolve.RuleIndex,
+	from label.Label,
+	impStm ImportStatement) (ResolutionType, *label.Label, error) {
+
+	matches := ix.FindRulesByImportWithConfig(c, impStm.ImportSpec, LanguageName)
+	if len(matches) == 0 {
+		return Resolution_NotFound, nil, nil
+	}
+
+	filteredMatches := make([]label.Label, 0, len(matches))
+	for _, match := range matches {
+		// Prevent from adding itself as a dependency.
+		if !match.IsSelfImport(from) {
+			filteredMatches = append(filteredMatches, match.Label)
+		}
+	}
+
+	// Too many results, don't know which is correct
+	if len(filteredMatches) > 1 {
+		return Resolution_Error, nil, fmt.Errorf(
+			"Import %q from %q resolved to multiple targets (%s) - this must be fixed using the \"gazelle:resolve\" directive",
+			impStm.ImportPath, impStm.SourcePath, targetListFromResults(matches))
+	}
+
+	// The matches were self imports, no dependency is needed
+	if len(filteredMatches) == 0 {
+		return Resolution_None, nil, nil
+	}
+
+	match := filteredMatches[0]
+
+	BazelLog.Tracef("resolve %q import %q as %q", from, impStm.Imp, match)
+
+	return Resolution_Override, &match, nil
+}
+
 func (ts *typeScriptLang) resolvePackageImport(from label.Label, imp string) *label.Label {
 	impPkg, _ := node.ParseImportPath(imp)
 

gazelle/js/target.go

@@ -13,9 +13,6 @@ import (
 type ImportStatement struct {
 	resolve.ImportSpec
 
-	// Alternative paths this statement may resolve to
-	Alt []string
-
 	// The path of the file containing the import
 	SourcePath string
 

gazelle/js/tests/pnpm_workspace/tsconfig.json

@@ -0,0 +1,10 @@
+// This tsconfig also maps paths but is only for IDEs!
+{
+    "compilerOptions": {
+        "paths": {
+            // The BUILD dependencies should prioritize the `workspace:*` packages
+            // instead of mapping to the `ts_project` targets that this tsconfig maps to.
+            "@lib/*": ["lib/*"]
+        }
+    }
+}