Fix issue with noop tasks that depend on other noop tasks

change/change-8df26c07-7241-49f9-ab2b-72ef0e59f43f.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "type": "patch",
+      "comment": "Fix issue with noop tasks that depend on other noop tasks",
+      "packageName": "@lage-run/target-graph",
+      "email": "dobes@formative.com",
+      "dependentChangeType": "patch"
+    }
+  ]
+}

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

@@ -1,5 +1,6 @@
 import { Monorepo } from "./mock/monorepo.js";
 import { filterEntry, parseNdJson } from "./parseNdJson.js";
+import { getTargetId } from "@lage-run/target-graph";
 
 describe("basics", () => {
   let repo: Monorepo | undefined;
@@ -130,4 +131,35 @@ describe("basics", () => {
     expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "test", "success"))).toBeTruthy();
     expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "lint", "success"))).toBeFalsy();
   });
+
+  it("handles multiple levels of noop tasks", async () => {
+    const repo = new Monorepo("noop-task-layers");
+
+    repo.init();
+    repo.setLageConfig(`module.exports = {
+      "pipeline": {
+        "build": [],
+        "_build": { "type": "noop", "dependsOn": ["build"] },
+        "__build": { "type": "noop", "dependsOn": ["_build", "^__build"] },
+        "test": ["__build"]         
+      },
+      npmClient: 'yarn'
+    }`);
+
+    repo.addPackage("a", ["b"]);
+    repo.addPackage("b");
+    repo.install();
+
+    const results = repo.run("test");
+    const output = results.stdout + results.stderr;
+
+    const jsonOutput = parseNdJson(output);
+
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "b", "build", "success"))).toBeTruthy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "b", "test", "success"))).toBeTruthy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "build", "success"))).toBeTruthy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "test", "success"))).toBeTruthy();
+
+    await repo.cleanup();
+  });
 });

packages/target-graph/src/removeNodes.ts

@@ -1,5 +1,14 @@
 import type { Target } from "./types/Target.js";
 
+/**
+ * Remove nodes from the graph that are not runnable.
+ *
+ * When a node is removed, dependencies on that node are replaced with dependencies on the node's own
+ * dependencies.
+ *
+ * @param dag Node graph to process
+ * @param shouldDelete Predicate function identifying nodes to remove
+ */
 export async function removeNodes(dag: Target[], shouldDelete: (node: Target) => boolean | Promise<boolean>): Promise<Target[]> {
   // Create a map for quick lookup of nodes by id
   const nodeMap = new Map<string, Target>();
@@ -24,13 +33,20 @@ export async function removeNodes(dag: Target[], shouldDelete: (node: Target) =>
 
   // Update dependencies of remaining nodes
   for (const node of nodeMap.values()) {
-    const newDependencies = new Set(node.dependencies);
-    for (const depId of node.dependencies) {
-      if (additionalDependencies.has(depId)) {
-        additionalDependencies.get(depId)!.forEach((subDepId) => newDependencies.add(subDepId));
+    const newDependencies = new Set<string>();
+
+    // Inherit the dependencies of removed nodes we depended on
+    const visitDependency = (depId: string) => {
+      if (nodeMap.has(depId)) {
+        // It's still a valid node, keep in dependencies list
+        newDependencies.add(depId);
+      } else {
+        // Node was removed, propagate dependencies from the removed node
+        additionalDependencies.get(depId)?.forEach(visitDependency);
       }
-    }
-    node.dependencies = Array.from(newDependencies).filter((depId) => nodeMap.has(depId));
+    };
+    node.dependencies.forEach(visitDependency);
+    node.dependencies = Array.from(newDependencies);
   }
 
   // Convert the map back to an array