Fix bug with phantom transitive dependencies

change/change-2e852706-e348-4268-baee-efe7ed5475ca.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "type": "minor",
+      "comment": "Fix bug where we have unnecessary dependency edges added to the graph when a package does not have the relevant target in scripts",
+      "packageName": "@lage-run/target-graph",
+      "email": "1581488+christiango@users.noreply.github.com",
+      "dependentChangeType": "patch"
+    }
+  ]
+}

packages/e2e-tests/src/transitiveTaskDeps.test.ts

@@ -207,4 +207,51 @@ describe("transitive task deps test", () => {
 
     await repo.cleanup();
   });
+
+  it("reproduce bug where transitive dependencies were being added that were not necessary", async () => {
+    // Simulates a bug from an internal repo that implemented isolated declarations for some packages
+    const repo = new Monorepo("transitiveDeps-isolated-declarations-info");
+
+    await repo.init();
+
+    // This repo has some packages that have isolated declarations configured and some that do not.
+    // For the packages that do not have isolatedDeclarations enabled, we have a dummy emitDeclarations task defined for them whose sole purpose is to make sure we block on those package's typecheck step for d.ts emission
+    // For packages that do have isolatedDeclarations enabled, we emit the d.ts during transpile so we omit the emitDeclarations task.
+    await repo.setLageConfig(`module.exports = {
+      pipeline: {
+        transpile: [],
+        emitDeclarations: ["typecheck"],
+        typecheck: ["^^emitDeclarations", "transpile", "^^transpile"]
+      },
+    }`);
+
+    await repo.addPackage("dep", [], {
+      transpile: "echo dep:transpile",
+      typecheck: "echo dep:typecheck",
+    });
+
+    await repo.addPackage("app", ["dep"], {
+      transpile: "echo app:transpile",
+      typecheck: "echo app:typecheck",
+      emitDeclarations: "echo app:emitDeclarations",
+    });
+
+    await repo.install();
+
+    const results = await repo.run("writeInfo", ["typecheck", "--scope", "app"]);
+
+    const output = results.stdout + results.stderr;
+    const jsonOutput = parseNdJson(output);
+    const packageTasks = jsonOutput[0].data.packageTasks;
+
+    const appTypecheckTask = packageTasks.find(({ id }: { id: string }) => id === "app#typecheck");
+    expect(appTypecheckTask).toBeTruthy();
+
+    expect(appTypecheckTask.dependencies).toContain("app#transpile");
+
+    // This was the bug, we'd end up with app depending on the typecheck step of the dependency which does not have an emitDeclarations step
+    expect(appTypecheckTask.dependencies).not.toContain("dep#typecheck");
+
+    await repo.cleanup();
+  });
 });

packages/target-graph/src/WorkspaceTargetGraphBuilder.ts

@@ -95,8 +95,10 @@ export class WorkspaceTargetGraphBuilder {
       this.processStagedConfig(target, config, changedFiles);
     } else {
       const packages = Object.keys(this.packageInfos);
+
       for (const packageName of packages) {
         const task = id;
+
         const targetConfig = this.determineFinalTargetConfig(getTargetId(packageName, task), config);
         const target = this.targetFactory.createPackageTarget(packageName!, task, targetConfig);
         this.graphBuilder.addTarget(target);
@@ -207,7 +209,7 @@ export class WorkspaceTargetGraphBuilder {
     priorities?: { package?: string; task: string; priority: number }[]
   ): Promise<TargetGraph> {
     // Expands the dependency specs from the target definitions
-    const fullDependencies = expandDepSpecs(this.graphBuilder.targets, this.dependencyMap);
+    const fullDependencies = expandDepSpecs(this.graphBuilder.targets, this.dependencyMap, this.packageInfos);
 
     for (const [from, to] of fullDependencies) {
       this.graphBuilder.addDependency(from, to);

packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts

@@ -402,4 +402,77 @@ describe("workspace target graph builder", () => {
       ]
     `);
   });
+
+  it("should not create phantom transitive deps for packages missing a script", async () => {
+    const root = "/repos/a";
+
+    // "app" has emitDeclarations in scripts, "dep" does not
+    const packageInfos = createPackageInfoWithScripts({
+      app: { deps: ["dep"], scripts: ["transpile", "typecheck", "emitDeclarations"] },
+      dep: { deps: [], scripts: ["transpile", "typecheck"] },
+    });
+
+    const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false);
+    await builder.addTargetConfig("transpile");
+    await builder.addTargetConfig("emitDeclarations", {
+      dependsOn: ["typecheck"],
+    });
+    await builder.addTargetConfig("typecheck", {
+      dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"],
+    });
+
+    const targetGraph = await builder.build(["typecheck"], ["app"]);
+    const graph = getGraphFromTargets(targetGraph);
+
+    // app#typecheck should depend on app#transpile (same-package dep)
+    expect(graph).toContainEqual(["app#transpile", "app#typecheck"]);
+
+    // app#typecheck should depend on dep#transpile (via ^^transpile, dep has the script)
+    expect(graph).toContainEqual(["dep#transpile", "app#typecheck"]);
+
+    // app#typecheck should NOT depend on dep#typecheck — dep doesn't have emitDeclarations,
+    // so the phantom dep#emitDeclarations should not create a transitive link
+    expect(graph).not.toContainEqual(["dep#typecheck", "app#typecheck"]);
+  });
+
+  it("should preserve ^^ deps for non-npmScript typed targets even without scripts entry", async () => {
+    const root = "/repos/a";
+
+    // "dep" doesn't have "customTask" in scripts, but it's configured as a worker type
+    const packageInfos = createPackageInfoWithScripts({
+      app: { deps: ["dep"], scripts: ["build"] },
+      dep: { deps: [], scripts: ["build"] },
+    });
+
+    const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false);
+    await builder.addTargetConfig("customTask", {
+      type: "worker",
+    });
+    await builder.addTargetConfig("build", {
+      dependsOn: ["^^customTask"],
+    });
+
+    const targetGraph = await builder.build(["build"], ["app"]);
+    const graph = getGraphFromTargets(targetGraph);
+
+    // app#build should depend on dep#customTask even though dep doesn't have it in scripts,
+    // because the target has an explicit non-npmScript type ("worker")
+    expect(graph).toContainEqual(["dep#customTask", "app#build"]);
+  });
 });
+
+function createPackageInfoWithScripts(packages: { [id: string]: { deps: string[]; scripts: string[] } }) {
+  const packageInfos: PackageInfos = {};
+  Object.keys(packages).forEach((id) => {
+    const { deps, scripts } = packages[id];
+    packageInfos[id] = {
+      packageJsonPath: `/path/to/${id}/package.json`,
+      name: id,
+      version: "1.0.0",
+      dependencies: deps.reduce((acc, dep) => ({ ...acc, [dep]: "*" }), {}),
+      devDependencies: {},
+      scripts: scripts.reduce((acc, script) => ({ ...acc, [script]: `echo ${script}` }), {}),
+    };
+  });
+  return packageInfos;
+}

packages/target-graph/src/expandDepSpecs.ts

@@ -1,11 +1,46 @@
 import type { Target } from "./types/Target.js";
 import type { DependencyMap } from "workspace-tools/lib/graph/createDependencyMap.js";
+import type { PackageInfos } from "workspace-tools";
 import { getPackageAndTask, getStartTargetId, getTargetId } from "./targetId.js";
 
+/**
+ * Checks whether a target represents a "phantom" target — one created for a package that
+ * doesn't actually define the script for an npmScript target. Phantom targets should be excluded from `^` and `^^`
+ * (topological/transitive) dependency expansion to avoid unwanted cross-package dependency chains.
+ *
+ * When phantom targets are later removed by `removeNodes` (because `shouldRun` returns false),
+ * their same-package dependencies get reconnected to their cross-package dependents, creating
+ * unnecessary work.
+ *
+ * Returns true if the target should be EXCLUDED from dependency expansion.
+ */
+function isPhantomTarget(targetId: string, task: string, targets: Map<string, Target>, packageInfos?: PackageInfos): boolean {
+  if (!packageInfos) return false;
+
+  const target = targets.get(targetId);
+  if (!target?.packageName) return false;
+
+  // Only npmScript targets can be phantom — other types (worker, noop, etc.)
+  // are real targets regardless of whether the package defines the script.
+  if (target.type !== "npmScript") {
+    return false;
+  }
+
+  const pkgScripts = packageInfos[target.packageName]?.scripts;
+  // If the package has a scripts section but doesn't include this task, it's a phantom target.
+  // If the package has no scripts section at all (e.g., in unit tests), we include the target
+  // for backward compatibility.
+  return !!pkgScripts && !pkgScripts[task];
+}
+
 /**
  * Expands the dependency graph by adding all transitive dependencies of the given targets.
  */
-export function expandDepSpecs(targets: Map<string, Target>, dependencyMap: DependencyMap): [string, string][] {
+export function expandDepSpecs(
+  targets: Map<string, Target>,
+  dependencyMap: DependencyMap,
+  packageInfos?: PackageInfos
+): [string, string][] {
   const dependencies: [string, string][] = [];
 
   /**
@@ -77,6 +112,8 @@ export function expandDepSpecs(targets: Map<string, Target>, dependencyMap: Depe
         const targetDependencies = [...(getTransitiveGraphDependencies(packageName, dependencyMap) ?? [])];
         const dependencyTargetIds = findDependenciesByTask(depTask, targetDependencies);
         for (const from of dependencyTargetIds) {
+          // Skip phantom targets: packages that don't define this task as a real npm script.
+          if (isPhantomTarget(from, depTask, targets, packageInfos)) continue;
           addDependency(from, to);
         }
       } else if (dependencyTargetId.startsWith("^") && packageName) {
@@ -85,6 +122,8 @@ export function expandDepSpecs(targets: Map<string, Target>, dependencyMap: Depe
         const targetDependencies = [...(dependencyMap.dependencies.get(packageName) ?? [])];
         const dependencyTargetIds = findDependenciesByTask(depTask, targetDependencies);
         for (const from of dependencyTargetIds) {
+          // Skip phantom targets: packages that don't define this task as a real npm script.
+          if (isPhantomTarget(from, depTask, targets, packageInfos)) continue;
           addDependency(from, to);
         }
       } else if (packageName) {