From 4baa8dd3c4790faf1ecbe80504f03d7a457a76e7 Mon Sep 17 00:00:00 2001 From: Felipe Escoto Date: Wed, 1 Mar 2023 13:31:08 -0600 Subject: [PATCH 1/2] Don't add to graph scripts missing from package.json --- .../e2e-tests/src/transitiveTaskDeps.test.ts | 4 +- .../src/WorkspaceTargetGraphBuilder.ts | 12 ++- .../tests/WorkspaceTargetGraphBuilder.test.ts | 87 +++++++++++++++---- 3 files changed, 80 insertions(+), 23 deletions(-) diff --git a/packages/e2e-tests/src/transitiveTaskDeps.test.ts b/packages/e2e-tests/src/transitiveTaskDeps.test.ts index f20a3e65e..53187e499 100644 --- a/packages/e2e-tests/src/transitiveTaskDeps.test.ts +++ b/packages/e2e-tests/src/transitiveTaskDeps.test.ts @@ -5,7 +5,7 @@ import { filterEntry, parseNdJson } from "./parseNdJson.js"; describe("transitive task deps test", () => { // This test follows the model as documented here: // https://microsoft.github.io/lage/guide/levels.html - it("produces a build graph even when some scripts are missing in package.json", () => { + it("produces a the correct build graph when some scripts are missing in package.json", () => { const repo = new Monorepo("transitiveDeps"); repo.init(); @@ -44,7 +44,7 @@ describe("transitive task deps test", () => { expect(indices[getTargetId("a", "build")]).toBeLessThan(indices[getTargetId("a", "test")]); - expect(indices[getTargetId("b", "build")]).toBeLessThan(indices[getTargetId("a", "test")]); + expect(indices[getTargetId("b", "build")]).toBeUndefined(); repo.cleanup(); }); diff --git a/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts b/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts index f748ea087..721ded42a 100644 --- a/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts +++ b/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts @@ -97,11 +97,19 @@ export class WorkspaceTargetGraphBuilder { for (const task of tasks) { if (scope) { for (const packageName of scope) { - subGraphEntries.push(getTargetId(packageName, task)); + const packageInfo = this.packageInfos[packageName]; + // Do not add a package to the subgraph if the package does not have the task + if (packageInfo.scripts && packageInfo.scripts[task]) { + subGraphEntries.push(getTargetId(packageName, task)); + } } } else { for (const packageName of Object.keys(this.packageInfos)) { - subGraphEntries.push(getTargetId(packageName, task)); + const packageInfo = this.packageInfos[packageName]; + // Do not add a package to the subgraph if the package does not have the task + if (packageInfo.scripts && packageInfo.scripts[task]) { + subGraphEntries.push(getTargetId(packageName, task)); + } } } } diff --git a/packages/target-graph/tests/WorkspaceTargetGraphBuilder.test.ts b/packages/target-graph/tests/WorkspaceTargetGraphBuilder.test.ts index 0b7a65eb9..fa12e305f 100644 --- a/packages/target-graph/tests/WorkspaceTargetGraphBuilder.test.ts +++ b/packages/target-graph/tests/WorkspaceTargetGraphBuilder.test.ts @@ -2,14 +2,17 @@ import type { PackageInfos } from "workspace-tools"; import { WorkspaceTargetGraphBuilder } from "../src/WorkspaceTargetGraphBuilder"; import type { TargetGraph } from "../src/types/TargetGraph"; -function createPackageInfo(packages: { [id: string]: string[] }) { +function createPackageInfo(packages: { [id: string]: { tasks: string[]; dependencies: string[] } }) { const packageInfos: PackageInfos = {}; Object.keys(packages).forEach((id) => { packageInfos[id] = { packageJsonPath: `/path/to/${id}/package.json`, name: id, version: "1.0.0", - dependencies: packages[id].reduce((acc, dep) => { + scripts: packages[id].tasks.reduce((acc, task) => { + return { ...acc, [task]: "noop" }; + }, {}), + dependencies: packages[id].dependencies.reduce((acc, dep) => { return { ...acc, [dep]: "*" }; }, {}), }; @@ -34,8 +37,8 @@ describe("workspace target graph builder", () => { const root = "/repos/a"; const packageInfos = createPackageInfo({ - a: ["b"], - b: [], + a: { dependencies: ["b"], tasks: ["build"] }, + b: { dependencies: [], tasks: ["build"] }, }); const builder = new WorkspaceTargetGraphBuilder(root, packageInfos); @@ -69,8 +72,8 @@ describe("workspace target graph builder", () => { it("should generate target graphs for tasks that do not depend on each other", () => { const root = "/repos/a"; const packageInfos = createPackageInfo({ - a: ["b"], - b: [], + a: { dependencies: ["b"], tasks: ["test", "lint"] }, + b: { dependencies: [], tasks: ["test", "lint"] }, }); const builder = new WorkspaceTargetGraphBuilder(root, packageInfos); @@ -107,9 +110,9 @@ describe("workspace target graph builder", () => { const root = "/repos/a"; const packageInfos = createPackageInfo({ - a: ["b"], - b: [], - c: ["b"], + a: { dependencies: ["b"], tasks: ["build"] }, + b: { dependencies: [], tasks: ["build"] }, + c: { dependencies: ["b"], tasks: ["build"] }, }); const builder = new WorkspaceTargetGraphBuilder(root, packageInfos); @@ -149,9 +152,9 @@ describe("workspace target graph builder", () => { const root = "/repos/a"; const packageInfos = createPackageInfo({ - a: ["b"], - b: [], - c: ["b"], + a: { dependencies: ["b"], tasks: ["build"] }, + b: { dependencies: [], tasks: ["build"] }, + c: { dependencies: ["b"], tasks: ["build"] }, }); const builder = new WorkspaceTargetGraphBuilder(root, packageInfos); @@ -179,13 +182,59 @@ describe("workspace target graph builder", () => { `); }); + it("should generate targetGraph without dependencies not needed to run the target", () => { + const root = "/repos/a"; + + const packageInfos = createPackageInfo({ + a: { dependencies: ["b"], tasks: ["build"] }, + b: { dependencies: [], tasks: ["build"] }, + c: { dependencies: ["b"], tasks: ["build", "test"] }, + }); + + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos); + + builder.addTargetConfig("build", { + dependsOn: ["^build"], + }); + + builder.addTargetConfig("test", { + dependsOn: ["build"], + }); + + const targetGraph = builder.build(["test"], ["a", "b", "c"]); + expect(getGraphFromTargets(targetGraph)).toMatchInlineSnapshot(` + [ + [ + "__start", + "c#test", + ], + [ + "c#build", + "c#test", + ], + [ + "__start", + "c#build", + ], + [ + "b#build", + "c#build", + ], + [ + "__start", + "b#build", + ], + ] + `); + }); + it("should generate targetGraph with transitive dependencies", () => { const root = "/repos/a"; const packageInfos = createPackageInfo({ - a: ["b"], - b: ["c"], - c: [], + a: { dependencies: ["b"], tasks: ["bundle", "transpile"] }, + b: { dependencies: ["c"], tasks: ["bundle", "transpile"] }, + c: { dependencies: [], tasks: ["bundle", "transpile"] }, }); const builder = new WorkspaceTargetGraphBuilder(root, packageInfos); @@ -227,10 +276,10 @@ describe("workspace target graph builder", () => { const root = "/repos/a"; const packageInfos = createPackageInfo({ - a: [], - b: [], - c: [], - common: [], + a: { dependencies: [], tasks: ["build"] }, + b: { dependencies: [], tasks: ["build"] }, + c: { dependencies: [], tasks: ["build"] }, + common: { dependencies: [], tasks: ["build", "copy"] }, }); const builder = new WorkspaceTargetGraphBuilder(root, packageInfos); From e476a8ddbb0c0f77029e77fdf6bcae5be8e3e67a Mon Sep 17 00:00:00 2001 From: Felipe Escoto Date: Wed, 1 Mar 2023 13:31:21 -0600 Subject: [PATCH 2/2] Change files --- ...-target-graph-fdec4d4b-405d-4ebe-825a-430305b9ac86.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@lage-run-target-graph-fdec4d4b-405d-4ebe-825a-430305b9ac86.json diff --git a/change/@lage-run-target-graph-fdec4d4b-405d-4ebe-825a-430305b9ac86.json b/change/@lage-run-target-graph-fdec4d4b-405d-4ebe-825a-430305b9ac86.json new file mode 100644 index 000000000..37e1379cf --- /dev/null +++ b/change/@lage-run-target-graph-fdec4d4b-405d-4ebe-825a-430305b9ac86.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Don't add to graph scripts missing from package.json", + "packageName": "@lage-run/target-graph", + "email": "felescoto95@hotmail.com", + "dependentChangeType": "patch" +}