diff --git a/change/change-8df26c07-7241-49f9-ab2b-72ef0e59f43f.json b/change/change-8df26c07-7241-49f9-ab2b-72ef0e59f43f.json new file mode 100644 index 000000000..f0906a049 --- /dev/null +++ b/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" + } + ] +} \ No newline at end of file diff --git a/packages/e2e-tests/src/basic.test.ts b/packages/e2e-tests/src/basic.test.ts index ad6195ae1..f66069ea8 100644 --- a/packages/e2e-tests/src/basic.test.ts +++ b/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(); + }); }); diff --git a/packages/target-graph/src/removeNodes.ts b/packages/target-graph/src/removeNodes.ts index 799885413..af5db5c85 100644 --- a/packages/target-graph/src/removeNodes.ts +++ b/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): Promise { // Create a map for quick lookup of nodes by id const nodeMap = new Map(); @@ -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(); + + // 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