From 3a9d1fe6520172f4001cff2521bea572c8c2d340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20van=20Velzen=20=F0=9F=81=B4?= Date: Wed, 18 Dec 2024 14:34:40 -0800 Subject: [PATCH] Add merge logic for targetConfig #816 --- packages/cli/src/commands/info/action.ts | 1 + .../cli/src/commands/run/createTargetGraph.ts | 19 +++++++- packages/cli/src/commands/run/runAction.ts | 1 + packages/cli/src/commands/run/watchAction.ts | 1 + .../cli/src/commands/server/lageService.ts | 1 + .../createTargetGraph.test.ts.snap | 47 ++++++++++++++++-- packages/cli/tests/createTargetGraph.test.ts | 48 ++++++++++++++----- packages/config/src/getConfig.ts | 1 + packages/config/src/types/ConfigOptions.ts | 5 ++ .../src/__snapshots__/info.test.ts.snap | 14 ++++++ packages/target-graph/package.json | 3 +- .../src/WorkspaceTargetGraphBuilder.ts | 43 ++++++++++++++--- yarn.lock | 8 ++++ 13 files changed, 166 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/commands/info/action.ts b/packages/cli/src/commands/info/action.ts index 4c728d096..4ffa12e0c 100644 --- a/packages/cli/src/commands/info/action.ts +++ b/packages/cli/src/commands/info/action.ts @@ -110,6 +110,7 @@ export async function infoAction(options: InfoActionOptions, command: Command) { outputs: config.cacheOptions.outputGlob, tasks, packageInfos, + enableTargetConfigMerging: config.enableTargetConfigMerging, }); const scope = getFilteredPackages({ diff --git a/packages/cli/src/commands/run/createTargetGraph.ts b/packages/cli/src/commands/run/createTargetGraph.ts index a034be96b..99c246a0c 100644 --- a/packages/cli/src/commands/run/createTargetGraph.ts +++ b/packages/cli/src/commands/run/createTargetGraph.ts @@ -19,6 +19,7 @@ interface CreateTargetGraphOptions { outputs: string[]; tasks: string[]; packageInfos: PackageInfos; + enableTargetConfigMerging: boolean; } function getChangedFiles(since: string, cwd: string) { @@ -37,9 +38,23 @@ function getChangedFiles(since: string, cwd: string) { } export async function createTargetGraph(options: CreateTargetGraphOptions) { - const { logger, root, dependencies, dependents, since, scope, repoWideChanges, ignore, pipeline, outputs, tasks, packageInfos } = options; + const { + logger, + root, + dependencies, + dependents, + enableTargetConfigMerging, + since, + scope, + repoWideChanges, + ignore, + pipeline, + outputs, + tasks, + packageInfos, + } = options; - const builder = new WorkspaceTargetGraphBuilder(root, packageInfos); + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, enableTargetConfigMerging); const packages = getFilteredPackages({ root, diff --git a/packages/cli/src/commands/run/runAction.ts b/packages/cli/src/commands/run/runAction.ts index 925e838a7..6195dc048 100644 --- a/packages/cli/src/commands/run/runAction.ts +++ b/packages/cli/src/commands/run/runAction.ts @@ -63,6 +63,7 @@ export async function runAction(options: RunOptions, command: Command) { outputs: config.cacheOptions.outputGlob, tasks, packageInfos, + enableTargetConfigMerging: config.enableTargetConfigMerging, }); validateTargetGraph(targetGraph, allowNoTargetRuns); diff --git a/packages/cli/src/commands/run/watchAction.ts b/packages/cli/src/commands/run/watchAction.ts index ad3046248..9f5bb9c1a 100644 --- a/packages/cli/src/commands/run/watchAction.ts +++ b/packages/cli/src/commands/run/watchAction.ts @@ -61,6 +61,7 @@ export async function watchAction(options: RunOptions, command: Command) { outputs: config.cacheOptions.outputGlob, tasks, packageInfos, + enableTargetConfigMerging: config.enableTargetConfigMerging, }); // Make sure we do not attempt writeRemoteCache in watch mode diff --git a/packages/cli/src/commands/server/lageService.ts b/packages/cli/src/commands/server/lageService.ts index 28bbc4ee9..99c591cc8 100644 --- a/packages/cli/src/commands/server/lageService.ts +++ b/packages/cli/src/commands/server/lageService.ts @@ -73,6 +73,7 @@ async function createInitializedPromise({ cwd, logger, serverControls, nodeArg, outputs: config.cacheOptions.outputGlob, tasks, packageInfos, + enableTargetConfigMerging: config.enableTargetConfigMerging, }); const dependencyMap = createDependencyMap(packageInfos, { withDevDependencies: true, withPeerDependencies: false }); diff --git a/packages/cli/tests/__snapshots__/createTargetGraph.test.ts.snap b/packages/cli/tests/__snapshots__/createTargetGraph.test.ts.snap index b62642446..e0663ba4e 100644 --- a/packages/cli/tests/__snapshots__/createTargetGraph.test.ts.snap +++ b/packages/cli/tests/__snapshots__/createTargetGraph.test.ts.snap @@ -58,7 +58,7 @@ exports[`createTargetGraph Basic graph, spanning tasks 1`] = ` ]" `; -exports[`createTargetGraph PackageJsonOverrideForTargetDeps 1`] = ` +exports[`createTargetGraph Merging Dependencies in pipeline and package.json override 1`] = ` "[ { "id": "__start", @@ -68,7 +68,9 @@ exports[`createTargetGraph PackageJsonOverrideForTargetDeps 1`] = ` "id": "foo1#build", "dependencies": [ "__start", - "foo2#build" + "foo2#build", + "foo3#build", + "foo4#build" ] }, { @@ -78,16 +80,53 @@ exports[`createTargetGraph PackageJsonOverrideForTargetDeps 1`] = ` ] }, { - "id": "foo1#test", + "id": "foo3#build", "dependencies": [ "__start" ] }, { - "id": "foo2#test", + "id": "foo4#build", + "dependencies": [ + "__start" + ] + } +]" +`; + +exports[`createTargetGraph Merging inputs and outputs in pipeline and package.json override 1`] = ` +"[ + { + "id": "__start", + "dependencies": [] + }, + { + "id": "foo1#build", "dependencies": [ "__start", "foo2#build" + ], + "inputs": [ + "src/**", + "src/**", + "myTool.config", + "tsconfig.json" + ], + "outputs": [ + "lib/**", + "dist/**" + ] + }, + { + "id": "foo2#build", + "dependencies": [ + "__start" + ], + "inputs": [ + "src/**" + ], + "outputs": [ + "lib/**" ] } ]" diff --git a/packages/cli/tests/createTargetGraph.test.ts b/packages/cli/tests/createTargetGraph.test.ts index 83107ac23..2c8873995 100644 --- a/packages/cli/tests/createTargetGraph.test.ts +++ b/packages/cli/tests/createTargetGraph.test.ts @@ -9,12 +9,10 @@ import { getFilteredPackages } from "../src/filter/getFilteredPackages.js"; import { getBinPaths } from "../src/getBinPaths.js"; describe("createTargetGraph", () => { - const logger = new Logger(); - it("Basic graph, seperate nodes", async () => { const packageInfos: PackageInfos = { - foo1: stubPackage({ name: "foo1", deps: ["foo2"] }), - foo2: stubPackage({ name: "foo2" }), + foo1: stubPackage({ name: "foo1", scripts: ["build"], deps: ["foo2"] }), + foo2: stubPackage({ name: "foo2", scripts: ["build"] }), }; const pipeline: PipelineDefinition = { @@ -28,8 +26,8 @@ describe("createTargetGraph", () => { it("Basic graph, spanning tasks", async () => { const packageInfos: PackageInfos = { - foo1: stubPackage({ name: "foo1", deps: ["foo2"] }), - foo2: stubPackage({ name: "foo2" }), + foo1: stubPackage({ name: "foo1", scripts: ["build"], deps: ["foo2"] }), + foo2: stubPackage({ name: "foo2", scripts: ["build"] }), }; const pipeline: PipelineDefinition = { @@ -41,18 +39,43 @@ describe("createTargetGraph", () => { expect(result).toMatchSnapshot(); }); - it("PackageJsonOverrideForTargetDeps", async () => { + it("Merging Dependencies in pipeline and package.json override", async () => { const packageInfos: PackageInfos = { - foo1: stubPackage({ name: "foo1", deps: ["foo2"] }), - foo2: stubPackage({ name: "foo2", fields: { lage: { test: ["build"] } } }), + foo1: stubPackage({ name: "foo1", scripts: ["build"], deps: ["foo2"], fields: { lage: { build: ["foo4#build"] } } }), + foo2: stubPackage({ name: "foo2", scripts: ["build"] }), + foo3: stubPackage({ name: "foo3", scripts: ["build"] }), + foo4: stubPackage({ name: "foo4", scripts: ["build"] }), }; const pipeline: PipelineDefinition = { build: ["^build"], - test: [], + "foo1#build": ["foo3#build"], }; - const result = await createAndPrintPackageTasks(["build", "test"], packageInfos, pipeline, ["id", "dependencies"]); + const result = await createAndPrintPackageTasks(["build"], packageInfos, pipeline, ["id", "dependencies"]); + expect(result).toMatchSnapshot(); + }); + + it("Merging inputs and outputs in pipeline and package.json override", async () => { + const packageInfos: PackageInfos = { + foo1: stubPackage({ + name: "foo1", + deps: ["foo2"], + scripts: ["build"], + fields: { lage: { build: { inputs: ["tsconfig.json"], outputs: ["dist/**"] } } }, + }), + foo2: stubPackage({ name: "foo2", scripts: ["build"] }), + }; + + const pipeline: PipelineDefinition = { + build: { inputs: ["src/**"], outputs: ["lib/**"] }, + "foo1#build": { + inputs: ["src/**", "myTool.config"], + dependsOn: ["foo2#build"], + }, + }; + + const result = await createAndPrintPackageTasks(["build"], packageInfos, pipeline, ["id", "dependencies", "inputs", "outputs"]); expect(result).toMatchSnapshot(); }); }); @@ -64,7 +87,7 @@ async function createAndPrintPackageTasks( fields: string[] ): Promise { const packageTasks = await createPackageTasks(tasks, packageInfos, pipeline); - const expected = filterObjects(packageTasks, ["id", "dependencies"]); + const expected = filterObjects(packageTasks, fields); return JSON.stringify(expected, null, 2); } @@ -114,6 +137,7 @@ async function createPackageTasks(tasks: string[], packageInfos: PackageInfos, p outputs: config.cacheOptions.outputGlob, tasks, packageInfos, + enableTargetConfigMerging: true, }); const scope = getFilteredPackages({ diff --git a/packages/config/src/getConfig.ts b/packages/config/src/getConfig.ts index 0257931ff..bc7b0acae 100644 --- a/packages/config/src/getConfig.ts +++ b/packages/config/src/getConfig.ts @@ -28,5 +28,6 @@ export async function getConfig(cwd: string): Promise { workerIdleMemoryLimit: config?.workerIdleMemoryLimit ?? os.totalmem(), // 0 means no limit, concurrency: config?.concurrency ?? availableParallelism, allowNoTargetRuns: config?.allowNoTargetRuns ?? false, + enableTargetConfigMerging: config?.enableTargetConfigMerging ?? false, }; } diff --git a/packages/config/src/types/ConfigOptions.ts b/packages/config/src/types/ConfigOptions.ts index a6eda4f2b..712fad54b 100644 --- a/packages/config/src/types/ConfigOptions.ts +++ b/packages/config/src/types/ConfigOptions.ts @@ -65,4 +65,9 @@ export interface ConfigOptions { * Allows for no targets run */ allowNoTargetRuns: boolean; + + /** + * Enables the merging of target config files, rather than simply replace it when multiple matches are encoutered + */ + enableTargetConfigMerging: boolean; } diff --git a/packages/e2e-tests/src/__snapshots__/info.test.ts.snap b/packages/e2e-tests/src/__snapshots__/info.test.ts.snap index acf949b63..c4cf2e2f7 100644 --- a/packages/e2e-tests/src/__snapshots__/info.test.ts.snap +++ b/packages/e2e-tests/src/__snapshots__/info.test.ts.snap @@ -14,6 +14,7 @@ exports[`info command basic info test case 1`] = ` "id": "__start", "package": "", "task": "__start", + "weight": 1, "workingDirectory": "", }, { @@ -25,8 +26,10 @@ exports[`info command basic info test case 1`] = ` "a#build", ], "id": "a#test", + "options": {}, "package": "a", "task": "test", + "weight": 1, "workingDirectory": "packages/a", }, { @@ -38,8 +41,10 @@ exports[`info command basic info test case 1`] = ` "b#build", ], "id": "b#test", + "options": {}, "package": "b", "task": "test", + "weight": 1, "workingDirectory": "packages/b", }, { @@ -51,8 +56,10 @@ exports[`info command basic info test case 1`] = ` "b#build", ], "id": "a#build", + "options": {}, "package": "a", "task": "build", + "weight": 1, "workingDirectory": "packages/a", }, { @@ -64,8 +71,10 @@ exports[`info command basic info test case 1`] = ` "__start", ], "id": "b#build", + "options": {}, "package": "b", "task": "build", + "weight": 1, "workingDirectory": "packages/b", }, ], @@ -94,6 +103,7 @@ exports[`info command scoped info test case 1`] = ` "id": "__start", "package": "", "task": "__start", + "weight": 1, "workingDirectory": "", }, { @@ -105,8 +115,10 @@ exports[`info command scoped info test case 1`] = ` "b#build", ], "id": "b#test", + "options": {}, "package": "b", "task": "test", + "weight": 1, "workingDirectory": "packages/b", }, { @@ -118,8 +130,10 @@ exports[`info command scoped info test case 1`] = ` "__start", ], "id": "b#build", + "options": {}, "package": "b", "task": "build", + "weight": 1, "workingDirectory": "packages/b", }, ], diff --git a/packages/target-graph/package.json b/packages/target-graph/package.json index 7398341f5..d0a5f0d27 100644 --- a/packages/target-graph/package.json +++ b/packages/target-graph/package.json @@ -17,6 +17,7 @@ "lint": "monorepo-scripts lint" }, "dependencies": { + "mergician": "^2.0.2", "p-limit": "^3.1.0", "workspace-tools": "0.38.1" }, @@ -27,4 +28,4 @@ "publishConfig": { "access": "public" } -} +} \ No newline at end of file diff --git a/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts b/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts index 3c9c49bac..b2de998fb 100644 --- a/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts +++ b/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts @@ -12,6 +12,9 @@ import { TargetGraphBuilder } from "./TargetGraphBuilder.js"; import { TargetFactory } from "./TargetFactory.js"; import pLimit from "p-limit"; +//eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports -- mergician is a dual-mode library with CJS and ESM export but a single .d.ts file. Without type="module" on this packge.json typescript gets confused. See: https://github.com/microsoft/TypeScript/issues/50466 +const { mergician } = require("mergician"); + const DEFAULT_STAGED_TARGET_THRESHOLD = 50; /** * TargetGraphBuilder class provides a builder API for registering target configs. It exposes a method called `generateTargetGraph` to @@ -45,7 +48,7 @@ export class WorkspaceTargetGraphBuilder { * @param root the root directory of the workspace * @param packageInfos the package infos for the workspace */ - constructor(root: string, private packageInfos: PackageInfos) { + constructor(root: string, private packageInfos: PackageInfos, private enableTargetConfigMerging: boolean) { this.dependencyMap = createDependencyMap(packageInfos, { withDevDependencies: true, withPeerDependencies: false }); this.graphBuilder = new TargetGraphBuilder(); this.targetFactory = new TargetFactory({ @@ -70,32 +73,58 @@ export class WorkspaceTargetGraphBuilder { async addTargetConfig(id: string, config: TargetConfig = {}, changedFiles?: string[]) { // Generates a target definition from the target config if (id.startsWith("//") || id.startsWith("#")) { - const target = this.targetFactory.createGlobalTarget(id, config); + const targetConfig = this.determineFinalTargetConfig(id, config); + const target = this.targetFactory.createGlobalTarget(id, targetConfig); this.graphBuilder.addTarget(target); - this.targetConfigMap.set(id, config); this.hasRootTarget = true; this.processStagedConfig(target, config, changedFiles); } else if (id.includes("#")) { const { packageName, task } = getPackageAndTask(id); - const target = this.targetFactory.createPackageTarget(packageName!, task, config); + const targetConfig = this.determineFinalTargetConfig(id, config); + const target = this.targetFactory.createPackageTarget(packageName!, task, targetConfig); this.graphBuilder.addTarget(target); - this.targetConfigMap.set(id, config); this.processStagedConfig(target, config, changedFiles); } else { const packages = Object.keys(this.packageInfos); for (const packageName of packages) { const task = id; - const target = this.targetFactory.createPackageTarget(packageName!, task, config); + const targetConfig = this.determineFinalTargetConfig(getTargetId(packageName, task), config); + const target = this.targetFactory.createPackageTarget(packageName!, task, targetConfig); this.graphBuilder.addTarget(target); - this.targetConfigMap.set(id, config); this.processStagedConfig(target, config, changedFiles); } } } + // memoizes the merge function to avoid creating a new function for each target config + deepCloneTargetConfig = mergician({ + appendArrays: true, + onCircular: () => { + throw new Error(`Circular object reference detected in TargetConfig`); + }, + }); + + /** + * Given the config passed in and if merging is enabled, this logic will + * merge the current config object with the new config object and store it in the targetConfigMap. + * @param id The Id of the target to merge + * @param config The TargetConfig settings that will be merged if this target has already been seen before + * @returns The merged TargetConfig object. + */ + determineFinalTargetConfig(targetId: string, config: TargetConfig): TargetConfig { + let finalConfig = config; + if (this.enableTargetConfigMerging && this.targetConfigMap.has(targetId)) { + const existingConfig = this.targetConfigMap.get(targetId)!; + finalConfig = this.deepCloneTargetConfig(existingConfig, config); + } + + this.targetConfigMap.set(targetId, finalConfig); + return finalConfig; + } + /** * Side effects function on the passed in target * @param parentTarget diff --git a/yarn.lock b/yarn.lock index 152bde79c..e41229232 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1923,6 +1923,7 @@ __metadata: dependencies: "@lage-run/monorepo-scripts": "npm:*" jest-diff: "npm:^29.5.0" + mergician: "npm:^2.0.2" p-limit: "npm:^3.1.0" workspace-tools: "npm:0.38.1" languageName: unknown @@ -6398,6 +6399,13 @@ __metadata: languageName: node linkType: hard +"mergician@npm:^2.0.2": + version: 2.0.2 + resolution: "mergician@npm:2.0.2" + checksum: 10c0/ced960df3de520bef21caf219a6f7cc3efd8395719fb5cf5153361fd27913f9bbaf80ad301f2b6d43f7d0d4ff5d484fc223f3182cc12dc52b33b89063f83692e + languageName: node + linkType: hard + "micromatch@npm:4.0.8, micromatch@npm:~4.0.8": version: 4.0.8 resolution: "micromatch@npm:4.0.8"