From d9d708d3049be487dd6077abfb95898a79f4ec13 Mon Sep 17 00:00:00 2001 From: Christian Gonzalez <1581488+christiango@users.noreply.github.com> Date: Tue, 3 Mar 2026 11:51:27 -0500 Subject: [PATCH 1/7] Copilot wip --- .../e2e-tests/src/transitiveTaskDeps.test.ts | 156 +++++++ packages/target-graph/src/TargetFactory.ts | 17 +- .../src/WorkspaceTargetGraphBuilder.ts | 4 +- .../WorkspaceTargetGraphBuilder.test.ts | 417 ++++++++++++++++++ packages/target-graph/src/expandDepSpecs.ts | 31 +- 5 files changed, 617 insertions(+), 8 deletions(-) diff --git a/packages/e2e-tests/src/transitiveTaskDeps.test.ts b/packages/e2e-tests/src/transitiveTaskDeps.test.ts index a8859aeb2..f7db092e8 100644 --- a/packages/e2e-tests/src/transitiveTaskDeps.test.ts +++ b/packages/e2e-tests/src/transitiveTaskDeps.test.ts @@ -207,4 +207,160 @@ describe("transitive task deps test", () => { await repo.cleanup(); }); + + it("isolated declarations: typecheck only blocks on ^^emitDeclarations from packages that have it", async () => { + // Models the office-bohemia isolated declarations pipeline exactly: + // transpile: [] + // emitDeclarations: ["typecheck"] – emitDeclarations depends on same-package typecheck + // typecheck: ["^^emitDeclarations", "transpile", "^^transpile"] – typecheck depends on transitive emitDeclarations + own & transitive transpile + // + // Topology: + // app → [iso-lib, iso-lib3, non-iso-lib] + // iso-lib → [non-iso-lib] + // iso-lib3 → [iso-lib2] + // iso-lib2 → [] (leaf, isolated) + // + // Only non-iso-lib defines emitDeclarations. All iso-lib* and app are "isolated" (no emitDeclarations script). + // Key behaviors: + // - ^^emitDeclarations resolves ONLY to non-iso-lib#emitDeclarations for downstream packages + // - iso-lib* and app do NOT get an emitDeclarations task at all + // - There is no ^^typecheck dependency, so typecheck tasks don't block on each other across packages + // - iso-lib3#typecheck blocks on iso-lib2#transpile (via ^^transpile), NOT on iso-lib2#typecheck + + const repo = new Monorepo("transitiveDeps-isolatedDecl"); + + await repo.init(); + await repo.setLageConfig(`module.exports = { + pipeline: { + transpile: [], + emitDeclarations: ["typecheck"], + typecheck: ["^^emitDeclarations", "transpile", "^^transpile"] + }, + }`); + + // non-iso-lib: non-isolated, has emitDeclarations (d.ts generated via tsc) + await repo.addPackage("non-iso-lib", [], { + transpile: "echo non-iso-lib:transpile", + typecheck: "echo non-iso-lib:typecheck", + emitDeclarations: "echo non-iso-lib:emitDeclarations", + }); + + // iso-lib: isolated, NO emitDeclarations (d.ts generated during transpile via OXC) + await repo.addPackage("iso-lib", ["non-iso-lib"], { + transpile: "echo iso-lib:transpile", + typecheck: "echo iso-lib:typecheck", + }); + + // iso-lib2: isolated leaf package, NO emitDeclarations + // Uses a slow typecheck (3s) so we can verify iso-lib3#typecheck starts before iso-lib2#typecheck finishes + await repo.addPackage("iso-lib2", [], { + transpile: "echo iso-lib2:transpile", + typecheck: "node slow-typecheck.js", + }); + + // Write a slow script file to avoid shell escaping issues on Windows + await repo.commitFiles({ + "packages/iso-lib2/slow-typecheck.js": "setTimeout(() => { console.log('iso-lib2:typecheck done'); process.exit(0); }, 3000);", + }); + + // iso-lib3: isolated, depends on iso-lib2, NO emitDeclarations + await repo.addPackage("iso-lib3", ["iso-lib2"], { + transpile: "echo iso-lib3:transpile", + typecheck: "echo iso-lib3:typecheck", + }); + + // app: isolated, NO emitDeclarations + await repo.addPackage("app", ["iso-lib", "iso-lib3", "non-iso-lib"], { + transpile: "echo app:transpile", + typecheck: "echo app:typecheck", + }); + + await repo.install(); + + // Use --concurrency 4 to ensure enough worker slots for parallel task execution + const results = await repo.run("lage", ["typecheck", "--reporter", "json", "--log-level", "silly", "--concurrency", "4"]); + + const output = results.stdout + results.stderr; + const jsonOutput = parseNdJson(output); + + const successIndices: { [taskId: string]: number } = {}; + const runningIndices: { [taskId: string]: number } = {}; + + for (const pkg of ["app", "iso-lib", "iso-lib2", "iso-lib3", "non-iso-lib"]) { + for (const task of ["transpile", "typecheck", "emitDeclarations"]) { + const successIndex = jsonOutput.findIndex((e) => filterEntry(e.data, pkg, task, "success")); + if (successIndex > -1) { + successIndices[getTargetId(pkg, task)] = successIndex; + } + const runningIndex = jsonOutput.findIndex((e) => filterEntry(e.data, pkg, task, "running")); + if (runningIndex > -1) { + runningIndices[getTargetId(pkg, task)] = runningIndex; + } + } + } + + // --- Existence assertions --- + + // 1. transpile runs for ALL packages + expect(successIndices[getTargetId("app", "transpile")]).toBeDefined(); + expect(successIndices[getTargetId("iso-lib", "transpile")]).toBeDefined(); + expect(successIndices[getTargetId("iso-lib2", "transpile")]).toBeDefined(); + expect(successIndices[getTargetId("iso-lib3", "transpile")]).toBeDefined(); + expect(successIndices[getTargetId("non-iso-lib", "transpile")]).toBeDefined(); + + // 2. emitDeclarations runs ONLY for non-iso-lib + expect(successIndices[getTargetId("non-iso-lib", "emitDeclarations")]).toBeDefined(); + expect(successIndices[getTargetId("iso-lib", "emitDeclarations")]).toBeUndefined(); + expect(successIndices[getTargetId("iso-lib2", "emitDeclarations")]).toBeUndefined(); + expect(successIndices[getTargetId("iso-lib3", "emitDeclarations")]).toBeUndefined(); + expect(successIndices[getTargetId("app", "emitDeclarations")]).toBeUndefined(); + + // 3. typecheck runs for ALL packages + expect(successIndices[getTargetId("app", "typecheck")]).toBeDefined(); + expect(successIndices[getTargetId("iso-lib", "typecheck")]).toBeDefined(); + expect(successIndices[getTargetId("iso-lib2", "typecheck")]).toBeDefined(); + expect(successIndices[getTargetId("iso-lib3", "typecheck")]).toBeDefined(); + expect(successIndices[getTargetId("non-iso-lib", "typecheck")]).toBeDefined(); + + // --- Ordering assertions (using success/completion indices) --- + + // 4. non-iso-lib#typecheck before non-iso-lib#emitDeclarations (emitDeclarations depends on same-package typecheck) + expect(successIndices[getTargetId("non-iso-lib", "typecheck")]).toBeLessThan(successIndices[getTargetId("non-iso-lib", "emitDeclarations")]); + + // 5. non-iso-lib#emitDeclarations before app#typecheck (app → ^^emitDeclarations → non-iso-lib#emitDeclarations) + expect(successIndices[getTargetId("non-iso-lib", "emitDeclarations")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); + + // 6. non-iso-lib#emitDeclarations before iso-lib#typecheck (iso-lib → ^^emitDeclarations → non-iso-lib#emitDeclarations) + expect(successIndices[getTargetId("non-iso-lib", "emitDeclarations")]).toBeLessThan(successIndices[getTargetId("iso-lib", "typecheck")]); + + // 7. Transitive transpile deps complete before downstream typecheck (^^transpile) + expect(successIndices[getTargetId("non-iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); + expect(successIndices[getTargetId("iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); + expect(successIndices[getTargetId("iso-lib3", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); + expect(successIndices[getTargetId("iso-lib2", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); + expect(successIndices[getTargetId("non-iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib", "typecheck")]); + + // 8. Own transpile before own typecheck (typecheck depends on "transpile" same-package) + expect(successIndices[getTargetId("app", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); + expect(successIndices[getTargetId("iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib", "typecheck")]); + expect(successIndices[getTargetId("iso-lib2", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib2", "typecheck")]); + expect(successIndices[getTargetId("iso-lib3", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib3", "typecheck")]); + expect(successIndices[getTargetId("non-iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("non-iso-lib", "typecheck")]); + + // --- Key assertion: isolated packages don't block typecheck on each other's typecheck --- + + // 9. iso-lib3#typecheck blocks on iso-lib2#transpile (via ^^transpile), NOT on iso-lib2#typecheck. + // Since there is no ^^typecheck in the pipeline, iso-lib2#typecheck is NOT a prerequisite + // for iso-lib3#typecheck. Only iso-lib2#transpile is (via ^^transpile). + expect(successIndices[getTargetId("iso-lib2", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib3", "typecheck")]); + + // 10. Verify non-blocking via "running" indices: iso-lib3#typecheck STARTS before + // iso-lib2#typecheck FINISHES. If iso-lib3 were blocked on iso-lib2's typecheck, + // iso-lib3 couldn't start running until iso-lib2's typecheck completed (success). + // By checking that iso-lib3#typecheck enters "running" before iso-lib2#typecheck + // enters "success", we prove there is no dependency between the two typecheck tasks. + expect(runningIndices[getTargetId("iso-lib3", "typecheck")]).toBeLessThan(successIndices[getTargetId("iso-lib2", "typecheck")]); + + await repo.cleanup(); + }); }); diff --git a/packages/target-graph/src/TargetFactory.ts b/packages/target-graph/src/TargetFactory.ts index 98d34e9fc..714ce3239 100644 --- a/packages/target-graph/src/TargetFactory.ts +++ b/packages/target-graph/src/TargetFactory.ts @@ -13,23 +13,28 @@ export interface TargetFactoryOptions { export class TargetFactory { private packageScripts: Set = new Set(); + private packageScriptsByPackage: Map> = new Map>(); constructor(private options: TargetFactoryOptions) { const { packageInfos } = options; for (const info of Object.values(packageInfos)) { + const scripts = new Set(); for (const scriptName of Object.keys(info.scripts ?? {})) { this.packageScripts.add(scriptName); + scripts.add(scriptName); } + this.packageScriptsByPackage.set(info.name, scripts); } } - private getTargetType(task: string, config: TargetConfig): string { + private getTargetType(task: string, config: TargetConfig, packageName?: string): string { if (!config.type) { - if (this.packageScripts.has(task)) { - return "npmScript"; - } else { - return "noop"; + // Per-package check: if we know the package, check if THIS package has the script. + // Falls back to global check for non-package targets or when packageName is not provided. + if (packageName) { + return this.packageScriptsByPackage.get(packageName)?.has(task) ? "npmScript" : "noop"; } + return this.packageScripts.has(task) ? "npmScript" : "noop"; } return config.type; @@ -43,7 +48,7 @@ export class TargetFactory { const { options, deps, dependsOn, cache, inputs, priority, maxWorkers, environmentGlob, weight } = config; const cwd = resolve(packageName); - const targetType = this.getTargetType(task, config); + const targetType = this.getTargetType(task, config, packageName); const target = { id: getTargetId(packageName, task), diff --git a/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts b/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts index 7c00c9c9a..06af9f33b 100644 --- a/packages/target-graph/src/WorkspaceTargetGraphBuilder.ts +++ b/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 { // 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); diff --git a/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts b/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts index 78dc9ecba..25863af04 100644 --- a/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts +++ b/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts @@ -402,4 +402,421 @@ describe("workspace target graph builder", () => { ] `); }); + + describe("^^transitive dependency only includes packages that have the task", () => { + /** + * These tests model the office-bohemia isolated declarations pipeline: + * + * - `typecheck` depends on `['^^emitDeclarations', 'transpile', '^^transpile']` + * - `emitDeclarations` depends on `['typecheck']` (same-package) + * - Only non-isolated packages define `emitDeclarations` + * - Isolated packages emit d.ts during `transpile` (via OXC), so they have no `emitDeclarations` + * + * The key behavior: `^^emitDeclarations` should only create edges to transitive deps + * that actually have the `emitDeclarations` task registered. Packages without it are skipped, + * meaning `typecheck` does NOT transitively block on `typecheck` of isolated dependencies. + */ + + it("should NOT create ^^emitDeclarations edges when no transitive deps have emitDeclarations", async () => { + // Setup: iso-app -> iso-libA (isolated) -> iso-libB (isolated) + // All packages are "isolated" (no emitDeclarations task). + // iso-app#typecheck should only depend on transpile tasks, not on any emitDeclarations. + // + // NOTE: Package names must be unique across all tests in this file because + // expandDepSpecs.ts caches transitive dependencies in a module-level Map keyed by package name. + const root = "/repos/isolated"; + + const packageInfos = createPackageInfo({ + "iso-app": ["iso-libA"], + "iso-libA": ["iso-libB"], + "iso-libB": [], + }); + + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); + + // transpile is defined for all packages + await builder.addTargetConfig("transpile"); + + // typecheck depends on ^^emitDeclarations + transpile + ^^transpile + await builder.addTargetConfig("typecheck", { + dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], + }); + + // emitDeclarations is NOT added for any package (all are isolated) + + const targetGraph = await builder.build(["typecheck"], ["iso-app"]); + const graph = getGraphFromTargets(targetGraph); + + // Verify: NO emitDeclarations edges exist at all + const emitEdges = graph.filter(([from]) => from.includes("emitDeclarations")); + expect(emitEdges).toEqual([]); + + // Verify: iso-app#typecheck depends on transpile of transitive deps + // and its own transpile (same-package) + expect(graph).toContainEqual(["iso-libA#transpile", "iso-app#typecheck"]); + expect(graph).toContainEqual(["iso-libB#transpile", "iso-app#typecheck"]); + expect(graph).toContainEqual(["iso-app#transpile", "iso-app#typecheck"]); + }); + + it("should create ^^emitDeclarations edges ONLY for transitive deps that have emitDeclarations", async () => { + // Setup: mix-app -> mix-libA (isolated) -> mix-libB (non-isolated, has emitDeclarations) + // mix-app -> mix-libC (non-isolated, has emitDeclarations) + // + // mix-app#typecheck should depend on: + // - mix-libB#emitDeclarations (transitive dep with emitDeclarations) + // - mix-libC#emitDeclarations (direct dep with emitDeclarations) + // - transpile tasks of all deps + // But NOT on mix-libA#emitDeclarations (mix-libA is isolated, has no emitDeclarations) + // or mix-libA#typecheck (mix-libA is isolated, so no typecheck->typecheck chain should form). + const root = "/repos/mixed"; + + const packageInfos = createPackageInfo({ + "mix-app": ["mix-libA", "mix-libC"], + "mix-libA": ["mix-libB"], + "mix-libB": [], + "mix-libC": [], + }); + + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); + + // transpile for all packages + await builder.addTargetConfig("transpile"); + + // typecheck for all packages + await builder.addTargetConfig("typecheck", { + dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], + }); + + // emitDeclarations ONLY for mix-libB and mix-libC (non-isolated packages) + await builder.addTargetConfig("mix-libB#emitDeclarations", { + dependsOn: ["typecheck"], + }); + await builder.addTargetConfig("mix-libC#emitDeclarations", { + dependsOn: ["typecheck"], + }); + + const targetGraph = await builder.build(["typecheck"], ["mix-app"]); + const graph = getGraphFromTargets(targetGraph); + + // mix-app#typecheck SHOULD depend on emitDeclarations of mix-libB and mix-libC + expect(graph).toContainEqual(["mix-libB#emitDeclarations", "mix-app#typecheck"]); + expect(graph).toContainEqual(["mix-libC#emitDeclarations", "mix-app#typecheck"]); + + // There should be NO edge from mix-libA#emitDeclarations (it doesn't exist) + const libAEmitEdges = graph.filter(([from]) => from === "mix-libA#emitDeclarations"); + expect(libAEmitEdges).toEqual([]); + + // mix-app#typecheck should also depend on transpile of all deps + expect(graph).toContainEqual(["mix-libA#transpile", "mix-app#typecheck"]); + expect(graph).toContainEqual(["mix-libB#transpile", "mix-app#typecheck"]); + expect(graph).toContainEqual(["mix-libC#transpile", "mix-app#typecheck"]); + expect(graph).toContainEqual(["mix-app#transpile", "mix-app#typecheck"]); + }); + + it("should not create cross-package typecheck dependencies for isolated packages", async () => { + // This is the key performance test: when all deps are isolated, + // typecheck tasks should be fully parallelizable (no cross-package typecheck->typecheck chain). + // + // Setup: par-app -> par-libA (isolated) -> par-libB (isolated) + // All are isolated, so no emitDeclarations task exists. + // + // Expected: par-app#typecheck depends on ^^transpile (par-libA, par-libB transpile) + own transpile. + // par-libA#typecheck depends on ^^transpile (par-libB transpile) + own transpile. + // There should be NO edge from par-libA#typecheck -> par-app#typecheck. + const root = "/repos/parallel"; + + const packageInfos = createPackageInfo({ + "par-app": ["par-libA"], + "par-libA": ["par-libB"], + "par-libB": [], + }); + + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); + + await builder.addTargetConfig("transpile"); + await builder.addTargetConfig("typecheck", { + dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], + }); + + // No emitDeclarations for any package (all isolated) + + const targetGraph = await builder.build(["typecheck"]); + const graph = getGraphFromTargets(targetGraph); + + // There should be NO edge from par-libA#typecheck to par-app#typecheck + // (that would mean sequential typechecking, defeating the purpose) + expect(graph).not.toContainEqual(["par-libA#typecheck", "par-app#typecheck"]); + expect(graph).not.toContainEqual(["par-libB#typecheck", "par-app#typecheck"]); + expect(graph).not.toContainEqual(["par-libB#typecheck", "par-libA#typecheck"]); + + // But transpile dependencies should exist + expect(graph).toContainEqual(["par-libA#transpile", "par-app#typecheck"]); + expect(graph).toContainEqual(["par-libB#transpile", "par-app#typecheck"]); + expect(graph).toContainEqual(["par-libB#transpile", "par-libA#typecheck"]); + }); + + it("should create typecheck chains through non-isolated packages via emitDeclarations", async () => { + // When a dep is non-isolated, its emitDeclarations depends on its typecheck, + // so the downstream typecheck transitively waits for that dep's typecheck. + // + // Setup: chain-app -> chain-libA (non-isolated, has emitDeclarations) -> chain-libB (isolated) + // + // Expected chain: chain-libB#transpile -> chain-libA#typecheck -> chain-libA#emitDeclarations -> chain-app#typecheck + // But NOT: chain-libB#typecheck -> chain-app#typecheck (no direct typecheck chain) + const root = "/repos/chain"; + + const packageInfos = createPackageInfo({ + "chain-app": ["chain-libA"], + "chain-libA": ["chain-libB"], + "chain-libB": [], + }); + + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); + + await builder.addTargetConfig("transpile"); + await builder.addTargetConfig("typecheck", { + dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], + }); + + // Only chain-libA has emitDeclarations (non-isolated) + await builder.addTargetConfig("chain-libA#emitDeclarations", { + dependsOn: ["typecheck"], + }); + + const targetGraph = await builder.build(["typecheck"]); + const graph = getGraphFromTargets(targetGraph); + + // chain-app#typecheck should depend on chain-libA#emitDeclarations (transitive dep with the task) + expect(graph).toContainEqual(["chain-libA#emitDeclarations", "chain-app#typecheck"]); + + // chain-libA#emitDeclarations depends on chain-libA#typecheck (same-package dep) + expect(graph).toContainEqual(["chain-libA#typecheck", "chain-libA#emitDeclarations"]); + + // This creates the chain: chain-libA#typecheck -> chain-libA#emitDeclarations -> chain-app#typecheck + // So chain-app#typecheck transitively waits for chain-libA#typecheck. Correct! + + // chain-libA#typecheck should depend on chain-libB#transpile (transitive dep) + expect(graph).toContainEqual(["chain-libB#transpile", "chain-libA#typecheck"]); + + // There should be NO chain-libB#emitDeclarations at all (chain-libB is isolated) + const libBEmitTargets = [...targetGraph.targets.keys()].filter((id) => id.includes("chain-libB#emitDeclarations")); + expect(libBEmitTargets).toEqual([]); + }); + + it("should not block typecheck on the typecheck of dependencies without emitDeclarations", async () => { + // This is the critical correctness assertion: when a dependency does NOT have + // emitDeclarations (i.e. it's isolated), there must be NO path from that dep's + // typecheck to the consumer's typecheck. The consumer only waits for transpile + // (which produces d.ts via OXC for isolated packages). + // + // Setup: nb-app -> nb-libIsolated (no emitDeclarations) -> nb-libNonIsolated (has emitDeclarations) + // + // Expected: + // - nb-app#typecheck does NOT depend on nb-libIsolated#typecheck (no emitDeclarations = no chain) + // - nb-app#typecheck DOES depend on nb-libNonIsolated#emitDeclarations + // (which depends on nb-libNonIsolated#typecheck, so there IS a transitive chain there) + // - nb-app#typecheck depends on nb-libIsolated#transpile (for the OXC-generated d.ts) + const root = "/repos/no-typecheck-block"; + + const packageInfos = createPackageInfo({ + "nb-app": ["nb-libIsolated"], + "nb-libIsolated": ["nb-libNonIsolated"], + "nb-libNonIsolated": [], + }); + + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); + + await builder.addTargetConfig("transpile"); + await builder.addTargetConfig("typecheck", { + dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], + }); + + // Only nb-libNonIsolated has emitDeclarations + await builder.addTargetConfig("nb-libNonIsolated#emitDeclarations", { + dependsOn: ["typecheck"], + }); + + const targetGraph = await builder.build(["typecheck"]); + const graph = getGraphFromTargets(targetGraph); + + // CRITICAL: nb-app#typecheck must NOT depend on nb-libIsolated#typecheck. + // nb-libIsolated has no emitDeclarations, so ^^emitDeclarations skips it entirely. + // The only way typecheck -> typecheck chains form is through emitDeclarations. + expect(graph).not.toContainEqual(["nb-libIsolated#typecheck", "nb-app#typecheck"]); + + // nb-app#typecheck DOES depend on nb-libNonIsolated#emitDeclarations + // (because nb-libNonIsolated is a transitive dep of nb-app that has emitDeclarations) + expect(graph).toContainEqual(["nb-libNonIsolated#emitDeclarations", "nb-app#typecheck"]); + + // nb-libNonIsolated#emitDeclarations depends on nb-libNonIsolated#typecheck (same-package) + expect(graph).toContainEqual(["nb-libNonIsolated#typecheck", "nb-libNonIsolated#emitDeclarations"]); + + // So nb-app#typecheck transitively waits for nb-libNonIsolated#typecheck via emitDeclarations. Correct. + // But it does NOT wait for nb-libIsolated#typecheck at all. That's the key. + + // nb-app#typecheck should still depend on transpile of both deps (for .js and d.ts files) + expect(graph).toContainEqual(["nb-libIsolated#transpile", "nb-app#typecheck"]); + expect(graph).toContainEqual(["nb-libNonIsolated#transpile", "nb-app#typecheck"]); + + // Verify nb-libIsolated#typecheck is completely independent of nb-app#typecheck. + // Collect all direct dependencies of nb-app#typecheck. + const appTypecheckDeps = graph + .filter(([, to]) => to === "nb-app#typecheck") + .map(([from]) => from); + + // None of them should be nb-libIsolated#typecheck + expect(appTypecheckDeps).not.toContain("nb-libIsolated#typecheck"); + + // And there should be no indirect path either: nb-libIsolated#typecheck + // should not appear as a dependency of any of nb-app#typecheck's deps + // (except via __start which is just a scheduling root). + // Specifically, nb-libIsolated#emitDeclarations should not exist at all. + const libIsolatedEmitTargets = [...targetGraph.targets.keys()].filter( + (id) => id === "nb-libIsolated#emitDeclarations" + ); + expect(libIsolatedEmitTargets).toEqual([]); + }); + + it("iso-lib3#typecheck depends on iso-lib2#transpile but NOT iso-lib2#typecheck", async () => { + // Matches the e2e test topology: + // dep-app → [dep-iso-lib, dep-iso-lib3, dep-non-iso-lib] + // dep-iso-lib → [dep-non-iso-lib] + // dep-iso-lib3 → [dep-iso-lib2] + // dep-iso-lib2 → [] (leaf, isolated) + // + // Only dep-non-iso-lib has emitDeclarations. All iso-lib* are isolated. + // Key assertion: dep-iso-lib3#typecheck has a direct edge from dep-iso-lib2#transpile + // but NO edge (direct or indirect) from dep-iso-lib2#typecheck. + const root = "/repos/dep-graph"; + + const packageInfos = createPackageInfo({ + "dep-app": ["dep-iso-lib", "dep-iso-lib3", "dep-non-iso-lib"], + "dep-iso-lib": ["dep-non-iso-lib"], + "dep-iso-lib3": ["dep-iso-lib2"], + "dep-iso-lib2": [], + "dep-non-iso-lib": [], + }); + + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); + + await builder.addTargetConfig("transpile"); + await builder.addTargetConfig("typecheck", { + dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], + }); + + // Only dep-non-iso-lib has emitDeclarations + await builder.addTargetConfig("dep-non-iso-lib#emitDeclarations", { + dependsOn: ["typecheck"], + }); + + const targetGraph = await builder.build(["typecheck"]); + const graph = getGraphFromTargets(targetGraph); + + // dep-iso-lib3#typecheck DOES depend on dep-iso-lib2#transpile (via ^^transpile) + expect(graph).toContainEqual(["dep-iso-lib2#transpile", "dep-iso-lib3#typecheck"]); + + // dep-iso-lib3#typecheck does NOT depend on dep-iso-lib2#typecheck + expect(graph).not.toContainEqual(["dep-iso-lib2#typecheck", "dep-iso-lib3#typecheck"]); + + // dep-iso-lib2#emitDeclarations should not exist at all (iso-lib2 is isolated) + expect([...targetGraph.targets.keys()]).not.toContain("dep-iso-lib2#emitDeclarations"); + + // Verify the full set of direct deps for dep-iso-lib3#typecheck: + // should be exactly: own transpile + dep-iso-lib2#transpile (and __start scheduling root). + // Notably: NO dep-iso-lib2#typecheck and NO dep-non-iso-lib#emitDeclarations + // (dep-non-iso-lib is not in iso-lib3's dependency tree). + const isoLib3TypecheckDeps = graph + .filter(([, to]) => to === "dep-iso-lib3#typecheck") + .map(([from]) => from) + .filter((from) => from !== "__start") + .sort(); + + expect(isoLib3TypecheckDeps).toEqual([ + "dep-iso-lib2#transpile", // ^^transpile (transitive dep) + "dep-iso-lib3#transpile", // transpile (same-package) + ].sort()); + + // Also verify dep-app#typecheck depends on emitDeclarations from non-iso-lib + // but NOT on typecheck of any isolated package + const appTypecheckDeps = graph + .filter(([, to]) => to === "dep-app#typecheck") + .map(([from]) => from); + + expect(appTypecheckDeps).toContain("dep-non-iso-lib#emitDeclarations"); + expect(appTypecheckDeps).not.toContain("dep-iso-lib#typecheck"); + expect(appTypecheckDeps).not.toContain("dep-iso-lib2#typecheck"); + expect(appTypecheckDeps).not.toContain("dep-iso-lib3#typecheck"); + }); + + it("with real scripts: skips emitDeclarations targets for packages without the script", async () => { + // This test uses packageInfos WITH scripts defined, which exercises the + // skipPackagesWithoutScript logic in WorkspaceTargetGraphBuilder.addTargetConfig. + // Unlike the other tests that use createPackageInfo (no scripts), this verifies + // that phantom emitDeclarations targets are NOT created for isolated packages, + // preventing unwanted dependency chains after optimizeTargetGraph's removeNodes. + // + // Topology: rs-app → [rs-iso-lib, rs-non-iso-lib] + // rs-non-iso-lib has: transpile, typecheck, emitDeclarations + // rs-iso-lib has: transpile, typecheck (no emitDeclarations) + // rs-app has: transpile, typecheck (no emitDeclarations) + const root = "/repos/real-scripts"; + + const packageInfos: PackageInfos = { + "rs-app": { + packageJsonPath: "/path/to/rs-app/package.json", + name: "rs-app", + version: "1.0.0", + dependencies: { "rs-iso-lib": "*", "rs-non-iso-lib": "*" }, + scripts: { transpile: "echo transpile", typecheck: "echo typecheck" }, + }, + "rs-iso-lib": { + packageJsonPath: "/path/to/rs-iso-lib/package.json", + name: "rs-iso-lib", + version: "1.0.0", + dependencies: { "rs-non-iso-lib": "*" }, + scripts: { transpile: "echo transpile", typecheck: "echo typecheck" }, + }, + "rs-non-iso-lib": { + packageJsonPath: "/path/to/rs-non-iso-lib/package.json", + name: "rs-non-iso-lib", + version: "1.0.0", + dependencies: {}, + scripts: { transpile: "echo transpile", typecheck: "echo typecheck", emitDeclarations: "echo emitDeclarations" }, + }, + }; + + const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); + + // Global pipeline tasks — emitDeclarations should only create targets where the script exists + await builder.addTargetConfig("transpile"); + await builder.addTargetConfig("emitDeclarations", { dependsOn: ["typecheck"] }); + await builder.addTargetConfig("typecheck", { + dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], + }); + + const targetGraph = await builder.build(["typecheck"]); + const graph = getGraphFromTargets(targetGraph); + + // emitDeclarations target should ONLY exist for rs-non-iso-lib + expect(targetGraph.targets.has("rs-non-iso-lib#emitDeclarations")).toBe(true); + expect(targetGraph.targets.has("rs-iso-lib#emitDeclarations")).toBe(false); + expect(targetGraph.targets.has("rs-app#emitDeclarations")).toBe(false); + + // rs-app#typecheck should depend on rs-non-iso-lib#emitDeclarations + expect(graph).toContainEqual(["rs-non-iso-lib#emitDeclarations", "rs-app#typecheck"]); + + // rs-app#typecheck should NOT depend on rs-iso-lib#typecheck + // (no phantom emitDeclarations → no reconnected typecheck chain) + expect(graph).not.toContainEqual(["rs-iso-lib#typecheck", "rs-app#typecheck"]); + + // rs-iso-lib#typecheck should depend on rs-non-iso-lib#emitDeclarations + expect(graph).toContainEqual(["rs-non-iso-lib#emitDeclarations", "rs-iso-lib#typecheck"]); + + // Transpile edges still work correctly + expect(graph).toContainEqual(["rs-iso-lib#transpile", "rs-app#typecheck"]); + expect(graph).toContainEqual(["rs-non-iso-lib#transpile", "rs-app#typecheck"]); + expect(graph).toContainEqual(["rs-app#transpile", "rs-app#typecheck"]); + expect(graph).toContainEqual(["rs-non-iso-lib#transpile", "rs-iso-lib#typecheck"]); + }); + }); }); diff --git a/packages/target-graph/src/expandDepSpecs.ts b/packages/target-graph/src/expandDepSpecs.ts index 2992832c5..7e4590097 100644 --- a/packages/target-graph/src/expandDepSpecs.ts +++ b/packages/target-graph/src/expandDepSpecs.ts @@ -1,11 +1,36 @@ 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. 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 + * e.g. typecheck→typecheck chains through ghost emitDeclarations nodes. + * + * Returns true if the target should be EXCLUDED from dependency expansion. + */ +function isPhantomTarget(targetId: string, task: string, targets: Map, packageInfos?: PackageInfos): boolean { + if (!packageInfos) return false; + + const target = targets.get(targetId); + if (!target?.packageName) 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, dependencyMap: DependencyMap): [string, string][] { +export function expandDepSpecs(targets: Map, dependencyMap: DependencyMap, packageInfos?: PackageInfos): [string, string][] { const dependencies: [string, string][] = []; /** @@ -77,6 +102,8 @@ export function expandDepSpecs(targets: Map, 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 +112,8 @@ export function expandDepSpecs(targets: Map, 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) { From ccc6ad7b4ba2fa95525cdabf02ffd17baaa8d385 Mon Sep 17 00:00:00 2001 From: Christian Gonzalez <1581488+christiango@users.noreply.github.com> Date: Tue, 3 Mar 2026 15:26:42 -0500 Subject: [PATCH 2/7] Add a simpler test --- .../e2e-tests/src/transitiveTaskDeps.test.ts | 147 +++--------------- 1 file changed, 19 insertions(+), 128 deletions(-) diff --git a/packages/e2e-tests/src/transitiveTaskDeps.test.ts b/packages/e2e-tests/src/transitiveTaskDeps.test.ts index f7db092e8..8185955f4 100644 --- a/packages/e2e-tests/src/transitiveTaskDeps.test.ts +++ b/packages/e2e-tests/src/transitiveTaskDeps.test.ts @@ -208,28 +208,15 @@ describe("transitive task deps test", () => { await repo.cleanup(); }); - it("isolated declarations: typecheck only blocks on ^^emitDeclarations from packages that have it", async () => { - // Models the office-bohemia isolated declarations pipeline exactly: - // transpile: [] - // emitDeclarations: ["typecheck"] – emitDeclarations depends on same-package typecheck - // typecheck: ["^^emitDeclarations", "transpile", "^^transpile"] – typecheck depends on transitive emitDeclarations + own & transitive transpile - // - // Topology: - // app → [iso-lib, iso-lib3, non-iso-lib] - // iso-lib → [non-iso-lib] - // iso-lib3 → [iso-lib2] - // iso-lib2 → [] (leaf, isolated) - // - // Only non-iso-lib defines emitDeclarations. All iso-lib* and app are "isolated" (no emitDeclarations script). - // Key behaviors: - // - ^^emitDeclarations resolves ONLY to non-iso-lib#emitDeclarations for downstream packages - // - iso-lib* and app do NOT get an emitDeclarations task at all - // - There is no ^^typecheck dependency, so typecheck tasks don't block on each other across packages - // - iso-lib3#typecheck blocks on iso-lib2#transpile (via ^^transpile), NOT on iso-lib2#typecheck - - const repo = new Monorepo("transitiveDeps-isolatedDecl"); + 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: [], @@ -238,128 +225,32 @@ describe("transitive task deps test", () => { }, }`); - // non-iso-lib: non-isolated, has emitDeclarations (d.ts generated via tsc) - await repo.addPackage("non-iso-lib", [], { - transpile: "echo non-iso-lib:transpile", - typecheck: "echo non-iso-lib:typecheck", - emitDeclarations: "echo non-iso-lib:emitDeclarations", - }); - - // iso-lib: isolated, NO emitDeclarations (d.ts generated during transpile via OXC) - await repo.addPackage("iso-lib", ["non-iso-lib"], { - transpile: "echo iso-lib:transpile", - typecheck: "echo iso-lib:typecheck", - }); - - // iso-lib2: isolated leaf package, NO emitDeclarations - // Uses a slow typecheck (3s) so we can verify iso-lib3#typecheck starts before iso-lib2#typecheck finishes - await repo.addPackage("iso-lib2", [], { - transpile: "echo iso-lib2:transpile", - typecheck: "node slow-typecheck.js", - }); - - // Write a slow script file to avoid shell escaping issues on Windows - await repo.commitFiles({ - "packages/iso-lib2/slow-typecheck.js": "setTimeout(() => { console.log('iso-lib2:typecheck done'); process.exit(0); }, 3000);", + await repo.addPackage("dep", [], { + transpile: "echo dep:transpile", + typecheck: "echo dep:typecheck", }); - // iso-lib3: isolated, depends on iso-lib2, NO emitDeclarations - await repo.addPackage("iso-lib3", ["iso-lib2"], { - transpile: "echo iso-lib3:transpile", - typecheck: "echo iso-lib3:typecheck", - }); - - // app: isolated, NO emitDeclarations - await repo.addPackage("app", ["iso-lib", "iso-lib3", "non-iso-lib"], { + await repo.addPackage("app", ["dep"], { transpile: "echo app:transpile", typecheck: "echo app:typecheck", + emitDeclarations: "echo app:emitDeclarations", }); await repo.install(); - // Use --concurrency 4 to ensure enough worker slots for parallel task execution - const results = await repo.run("lage", ["typecheck", "--reporter", "json", "--log-level", "silly", "--concurrency", "4"]); + 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 successIndices: { [taskId: string]: number } = {}; - const runningIndices: { [taskId: string]: number } = {}; + const appTypecheckTask = packageTasks.find(({ id }: { id: string }) => id === "app#typecheck"); + expect(appTypecheckTask).toBeTruthy(); - for (const pkg of ["app", "iso-lib", "iso-lib2", "iso-lib3", "non-iso-lib"]) { - for (const task of ["transpile", "typecheck", "emitDeclarations"]) { - const successIndex = jsonOutput.findIndex((e) => filterEntry(e.data, pkg, task, "success")); - if (successIndex > -1) { - successIndices[getTargetId(pkg, task)] = successIndex; - } - const runningIndex = jsonOutput.findIndex((e) => filterEntry(e.data, pkg, task, "running")); - if (runningIndex > -1) { - runningIndices[getTargetId(pkg, task)] = runningIndex; - } - } - } + expect(appTypecheckTask.dependencies).toContain("app#transpile"); - // --- Existence assertions --- - - // 1. transpile runs for ALL packages - expect(successIndices[getTargetId("app", "transpile")]).toBeDefined(); - expect(successIndices[getTargetId("iso-lib", "transpile")]).toBeDefined(); - expect(successIndices[getTargetId("iso-lib2", "transpile")]).toBeDefined(); - expect(successIndices[getTargetId("iso-lib3", "transpile")]).toBeDefined(); - expect(successIndices[getTargetId("non-iso-lib", "transpile")]).toBeDefined(); - - // 2. emitDeclarations runs ONLY for non-iso-lib - expect(successIndices[getTargetId("non-iso-lib", "emitDeclarations")]).toBeDefined(); - expect(successIndices[getTargetId("iso-lib", "emitDeclarations")]).toBeUndefined(); - expect(successIndices[getTargetId("iso-lib2", "emitDeclarations")]).toBeUndefined(); - expect(successIndices[getTargetId("iso-lib3", "emitDeclarations")]).toBeUndefined(); - expect(successIndices[getTargetId("app", "emitDeclarations")]).toBeUndefined(); - - // 3. typecheck runs for ALL packages - expect(successIndices[getTargetId("app", "typecheck")]).toBeDefined(); - expect(successIndices[getTargetId("iso-lib", "typecheck")]).toBeDefined(); - expect(successIndices[getTargetId("iso-lib2", "typecheck")]).toBeDefined(); - expect(successIndices[getTargetId("iso-lib3", "typecheck")]).toBeDefined(); - expect(successIndices[getTargetId("non-iso-lib", "typecheck")]).toBeDefined(); - - // --- Ordering assertions (using success/completion indices) --- - - // 4. non-iso-lib#typecheck before non-iso-lib#emitDeclarations (emitDeclarations depends on same-package typecheck) - expect(successIndices[getTargetId("non-iso-lib", "typecheck")]).toBeLessThan(successIndices[getTargetId("non-iso-lib", "emitDeclarations")]); - - // 5. non-iso-lib#emitDeclarations before app#typecheck (app → ^^emitDeclarations → non-iso-lib#emitDeclarations) - expect(successIndices[getTargetId("non-iso-lib", "emitDeclarations")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); - - // 6. non-iso-lib#emitDeclarations before iso-lib#typecheck (iso-lib → ^^emitDeclarations → non-iso-lib#emitDeclarations) - expect(successIndices[getTargetId("non-iso-lib", "emitDeclarations")]).toBeLessThan(successIndices[getTargetId("iso-lib", "typecheck")]); - - // 7. Transitive transpile deps complete before downstream typecheck (^^transpile) - expect(successIndices[getTargetId("non-iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); - expect(successIndices[getTargetId("iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); - expect(successIndices[getTargetId("iso-lib3", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); - expect(successIndices[getTargetId("iso-lib2", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); - expect(successIndices[getTargetId("non-iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib", "typecheck")]); - - // 8. Own transpile before own typecheck (typecheck depends on "transpile" same-package) - expect(successIndices[getTargetId("app", "transpile")]).toBeLessThan(successIndices[getTargetId("app", "typecheck")]); - expect(successIndices[getTargetId("iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib", "typecheck")]); - expect(successIndices[getTargetId("iso-lib2", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib2", "typecheck")]); - expect(successIndices[getTargetId("iso-lib3", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib3", "typecheck")]); - expect(successIndices[getTargetId("non-iso-lib", "transpile")]).toBeLessThan(successIndices[getTargetId("non-iso-lib", "typecheck")]); - - // --- Key assertion: isolated packages don't block typecheck on each other's typecheck --- - - // 9. iso-lib3#typecheck blocks on iso-lib2#transpile (via ^^transpile), NOT on iso-lib2#typecheck. - // Since there is no ^^typecheck in the pipeline, iso-lib2#typecheck is NOT a prerequisite - // for iso-lib3#typecheck. Only iso-lib2#transpile is (via ^^transpile). - expect(successIndices[getTargetId("iso-lib2", "transpile")]).toBeLessThan(successIndices[getTargetId("iso-lib3", "typecheck")]); - - // 10. Verify non-blocking via "running" indices: iso-lib3#typecheck STARTS before - // iso-lib2#typecheck FINISHES. If iso-lib3 were blocked on iso-lib2's typecheck, - // iso-lib3 couldn't start running until iso-lib2's typecheck completed (success). - // By checking that iso-lib3#typecheck enters "running" before iso-lib2#typecheck - // enters "success", we prove there is no dependency between the two typecheck tasks. - expect(runningIndices[getTargetId("iso-lib3", "typecheck")]).toBeLessThan(successIndices[getTargetId("iso-lib2", "typecheck")]); + // 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(); }); From 894c1d0b830863e6e463b21ed1a3bf7ca2fa2a92 Mon Sep 17 00:00:00 2001 From: Christian Gonzalez <1581488+christiango@users.noreply.github.com> Date: Tue, 3 Mar 2026 16:34:52 -0500 Subject: [PATCH 3/7] Less changes --- packages/target-graph/src/TargetFactory.ts | 17 ++++++----------- packages/target-graph/src/expandDepSpecs.ts | 6 +++++- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/target-graph/src/TargetFactory.ts b/packages/target-graph/src/TargetFactory.ts index 714ce3239..98d34e9fc 100644 --- a/packages/target-graph/src/TargetFactory.ts +++ b/packages/target-graph/src/TargetFactory.ts @@ -13,28 +13,23 @@ export interface TargetFactoryOptions { export class TargetFactory { private packageScripts: Set = new Set(); - private packageScriptsByPackage: Map> = new Map>(); constructor(private options: TargetFactoryOptions) { const { packageInfos } = options; for (const info of Object.values(packageInfos)) { - const scripts = new Set(); for (const scriptName of Object.keys(info.scripts ?? {})) { this.packageScripts.add(scriptName); - scripts.add(scriptName); } - this.packageScriptsByPackage.set(info.name, scripts); } } - private getTargetType(task: string, config: TargetConfig, packageName?: string): string { + private getTargetType(task: string, config: TargetConfig): string { if (!config.type) { - // Per-package check: if we know the package, check if THIS package has the script. - // Falls back to global check for non-package targets or when packageName is not provided. - if (packageName) { - return this.packageScriptsByPackage.get(packageName)?.has(task) ? "npmScript" : "noop"; + if (this.packageScripts.has(task)) { + return "npmScript"; + } else { + return "noop"; } - return this.packageScripts.has(task) ? "npmScript" : "noop"; } return config.type; @@ -48,7 +43,7 @@ export class TargetFactory { const { options, deps, dependsOn, cache, inputs, priority, maxWorkers, environmentGlob, weight } = config; const cwd = resolve(packageName); - const targetType = this.getTargetType(task, config, packageName); + const targetType = this.getTargetType(task, config); const target = { id: getTargetId(packageName, task), diff --git a/packages/target-graph/src/expandDepSpecs.ts b/packages/target-graph/src/expandDepSpecs.ts index 7e4590097..30cf0a0ec 100644 --- a/packages/target-graph/src/expandDepSpecs.ts +++ b/packages/target-graph/src/expandDepSpecs.ts @@ -30,7 +30,11 @@ function isPhantomTarget(targetId: string, task: string, targets: Map, dependencyMap: DependencyMap, packageInfos?: PackageInfos): [string, string][] { +export function expandDepSpecs( + targets: Map, + dependencyMap: DependencyMap, + packageInfos?: PackageInfos +): [string, string][] { const dependencies: [string, string][] = []; /** From 7978d198a6443fc3f959bccb5433543f4e4f4492 Mon Sep 17 00:00:00 2001 From: Christian Gonzalez <1581488+christiango@users.noreply.github.com> Date: Tue, 3 Mar 2026 16:35:15 -0500 Subject: [PATCH 4/7] Another revert --- .../WorkspaceTargetGraphBuilder.test.ts | 417 ------------------ 1 file changed, 417 deletions(-) diff --git a/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts b/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts index 25863af04..78dc9ecba 100644 --- a/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts +++ b/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts @@ -402,421 +402,4 @@ describe("workspace target graph builder", () => { ] `); }); - - describe("^^transitive dependency only includes packages that have the task", () => { - /** - * These tests model the office-bohemia isolated declarations pipeline: - * - * - `typecheck` depends on `['^^emitDeclarations', 'transpile', '^^transpile']` - * - `emitDeclarations` depends on `['typecheck']` (same-package) - * - Only non-isolated packages define `emitDeclarations` - * - Isolated packages emit d.ts during `transpile` (via OXC), so they have no `emitDeclarations` - * - * The key behavior: `^^emitDeclarations` should only create edges to transitive deps - * that actually have the `emitDeclarations` task registered. Packages without it are skipped, - * meaning `typecheck` does NOT transitively block on `typecheck` of isolated dependencies. - */ - - it("should NOT create ^^emitDeclarations edges when no transitive deps have emitDeclarations", async () => { - // Setup: iso-app -> iso-libA (isolated) -> iso-libB (isolated) - // All packages are "isolated" (no emitDeclarations task). - // iso-app#typecheck should only depend on transpile tasks, not on any emitDeclarations. - // - // NOTE: Package names must be unique across all tests in this file because - // expandDepSpecs.ts caches transitive dependencies in a module-level Map keyed by package name. - const root = "/repos/isolated"; - - const packageInfos = createPackageInfo({ - "iso-app": ["iso-libA"], - "iso-libA": ["iso-libB"], - "iso-libB": [], - }); - - const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); - - // transpile is defined for all packages - await builder.addTargetConfig("transpile"); - - // typecheck depends on ^^emitDeclarations + transpile + ^^transpile - await builder.addTargetConfig("typecheck", { - dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], - }); - - // emitDeclarations is NOT added for any package (all are isolated) - - const targetGraph = await builder.build(["typecheck"], ["iso-app"]); - const graph = getGraphFromTargets(targetGraph); - - // Verify: NO emitDeclarations edges exist at all - const emitEdges = graph.filter(([from]) => from.includes("emitDeclarations")); - expect(emitEdges).toEqual([]); - - // Verify: iso-app#typecheck depends on transpile of transitive deps - // and its own transpile (same-package) - expect(graph).toContainEqual(["iso-libA#transpile", "iso-app#typecheck"]); - expect(graph).toContainEqual(["iso-libB#transpile", "iso-app#typecheck"]); - expect(graph).toContainEqual(["iso-app#transpile", "iso-app#typecheck"]); - }); - - it("should create ^^emitDeclarations edges ONLY for transitive deps that have emitDeclarations", async () => { - // Setup: mix-app -> mix-libA (isolated) -> mix-libB (non-isolated, has emitDeclarations) - // mix-app -> mix-libC (non-isolated, has emitDeclarations) - // - // mix-app#typecheck should depend on: - // - mix-libB#emitDeclarations (transitive dep with emitDeclarations) - // - mix-libC#emitDeclarations (direct dep with emitDeclarations) - // - transpile tasks of all deps - // But NOT on mix-libA#emitDeclarations (mix-libA is isolated, has no emitDeclarations) - // or mix-libA#typecheck (mix-libA is isolated, so no typecheck->typecheck chain should form). - const root = "/repos/mixed"; - - const packageInfos = createPackageInfo({ - "mix-app": ["mix-libA", "mix-libC"], - "mix-libA": ["mix-libB"], - "mix-libB": [], - "mix-libC": [], - }); - - const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); - - // transpile for all packages - await builder.addTargetConfig("transpile"); - - // typecheck for all packages - await builder.addTargetConfig("typecheck", { - dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], - }); - - // emitDeclarations ONLY for mix-libB and mix-libC (non-isolated packages) - await builder.addTargetConfig("mix-libB#emitDeclarations", { - dependsOn: ["typecheck"], - }); - await builder.addTargetConfig("mix-libC#emitDeclarations", { - dependsOn: ["typecheck"], - }); - - const targetGraph = await builder.build(["typecheck"], ["mix-app"]); - const graph = getGraphFromTargets(targetGraph); - - // mix-app#typecheck SHOULD depend on emitDeclarations of mix-libB and mix-libC - expect(graph).toContainEqual(["mix-libB#emitDeclarations", "mix-app#typecheck"]); - expect(graph).toContainEqual(["mix-libC#emitDeclarations", "mix-app#typecheck"]); - - // There should be NO edge from mix-libA#emitDeclarations (it doesn't exist) - const libAEmitEdges = graph.filter(([from]) => from === "mix-libA#emitDeclarations"); - expect(libAEmitEdges).toEqual([]); - - // mix-app#typecheck should also depend on transpile of all deps - expect(graph).toContainEqual(["mix-libA#transpile", "mix-app#typecheck"]); - expect(graph).toContainEqual(["mix-libB#transpile", "mix-app#typecheck"]); - expect(graph).toContainEqual(["mix-libC#transpile", "mix-app#typecheck"]); - expect(graph).toContainEqual(["mix-app#transpile", "mix-app#typecheck"]); - }); - - it("should not create cross-package typecheck dependencies for isolated packages", async () => { - // This is the key performance test: when all deps are isolated, - // typecheck tasks should be fully parallelizable (no cross-package typecheck->typecheck chain). - // - // Setup: par-app -> par-libA (isolated) -> par-libB (isolated) - // All are isolated, so no emitDeclarations task exists. - // - // Expected: par-app#typecheck depends on ^^transpile (par-libA, par-libB transpile) + own transpile. - // par-libA#typecheck depends on ^^transpile (par-libB transpile) + own transpile. - // There should be NO edge from par-libA#typecheck -> par-app#typecheck. - const root = "/repos/parallel"; - - const packageInfos = createPackageInfo({ - "par-app": ["par-libA"], - "par-libA": ["par-libB"], - "par-libB": [], - }); - - const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); - - await builder.addTargetConfig("transpile"); - await builder.addTargetConfig("typecheck", { - dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], - }); - - // No emitDeclarations for any package (all isolated) - - const targetGraph = await builder.build(["typecheck"]); - const graph = getGraphFromTargets(targetGraph); - - // There should be NO edge from par-libA#typecheck to par-app#typecheck - // (that would mean sequential typechecking, defeating the purpose) - expect(graph).not.toContainEqual(["par-libA#typecheck", "par-app#typecheck"]); - expect(graph).not.toContainEqual(["par-libB#typecheck", "par-app#typecheck"]); - expect(graph).not.toContainEqual(["par-libB#typecheck", "par-libA#typecheck"]); - - // But transpile dependencies should exist - expect(graph).toContainEqual(["par-libA#transpile", "par-app#typecheck"]); - expect(graph).toContainEqual(["par-libB#transpile", "par-app#typecheck"]); - expect(graph).toContainEqual(["par-libB#transpile", "par-libA#typecheck"]); - }); - - it("should create typecheck chains through non-isolated packages via emitDeclarations", async () => { - // When a dep is non-isolated, its emitDeclarations depends on its typecheck, - // so the downstream typecheck transitively waits for that dep's typecheck. - // - // Setup: chain-app -> chain-libA (non-isolated, has emitDeclarations) -> chain-libB (isolated) - // - // Expected chain: chain-libB#transpile -> chain-libA#typecheck -> chain-libA#emitDeclarations -> chain-app#typecheck - // But NOT: chain-libB#typecheck -> chain-app#typecheck (no direct typecheck chain) - const root = "/repos/chain"; - - const packageInfos = createPackageInfo({ - "chain-app": ["chain-libA"], - "chain-libA": ["chain-libB"], - "chain-libB": [], - }); - - const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); - - await builder.addTargetConfig("transpile"); - await builder.addTargetConfig("typecheck", { - dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], - }); - - // Only chain-libA has emitDeclarations (non-isolated) - await builder.addTargetConfig("chain-libA#emitDeclarations", { - dependsOn: ["typecheck"], - }); - - const targetGraph = await builder.build(["typecheck"]); - const graph = getGraphFromTargets(targetGraph); - - // chain-app#typecheck should depend on chain-libA#emitDeclarations (transitive dep with the task) - expect(graph).toContainEqual(["chain-libA#emitDeclarations", "chain-app#typecheck"]); - - // chain-libA#emitDeclarations depends on chain-libA#typecheck (same-package dep) - expect(graph).toContainEqual(["chain-libA#typecheck", "chain-libA#emitDeclarations"]); - - // This creates the chain: chain-libA#typecheck -> chain-libA#emitDeclarations -> chain-app#typecheck - // So chain-app#typecheck transitively waits for chain-libA#typecheck. Correct! - - // chain-libA#typecheck should depend on chain-libB#transpile (transitive dep) - expect(graph).toContainEqual(["chain-libB#transpile", "chain-libA#typecheck"]); - - // There should be NO chain-libB#emitDeclarations at all (chain-libB is isolated) - const libBEmitTargets = [...targetGraph.targets.keys()].filter((id) => id.includes("chain-libB#emitDeclarations")); - expect(libBEmitTargets).toEqual([]); - }); - - it("should not block typecheck on the typecheck of dependencies without emitDeclarations", async () => { - // This is the critical correctness assertion: when a dependency does NOT have - // emitDeclarations (i.e. it's isolated), there must be NO path from that dep's - // typecheck to the consumer's typecheck. The consumer only waits for transpile - // (which produces d.ts via OXC for isolated packages). - // - // Setup: nb-app -> nb-libIsolated (no emitDeclarations) -> nb-libNonIsolated (has emitDeclarations) - // - // Expected: - // - nb-app#typecheck does NOT depend on nb-libIsolated#typecheck (no emitDeclarations = no chain) - // - nb-app#typecheck DOES depend on nb-libNonIsolated#emitDeclarations - // (which depends on nb-libNonIsolated#typecheck, so there IS a transitive chain there) - // - nb-app#typecheck depends on nb-libIsolated#transpile (for the OXC-generated d.ts) - const root = "/repos/no-typecheck-block"; - - const packageInfos = createPackageInfo({ - "nb-app": ["nb-libIsolated"], - "nb-libIsolated": ["nb-libNonIsolated"], - "nb-libNonIsolated": [], - }); - - const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); - - await builder.addTargetConfig("transpile"); - await builder.addTargetConfig("typecheck", { - dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], - }); - - // Only nb-libNonIsolated has emitDeclarations - await builder.addTargetConfig("nb-libNonIsolated#emitDeclarations", { - dependsOn: ["typecheck"], - }); - - const targetGraph = await builder.build(["typecheck"]); - const graph = getGraphFromTargets(targetGraph); - - // CRITICAL: nb-app#typecheck must NOT depend on nb-libIsolated#typecheck. - // nb-libIsolated has no emitDeclarations, so ^^emitDeclarations skips it entirely. - // The only way typecheck -> typecheck chains form is through emitDeclarations. - expect(graph).not.toContainEqual(["nb-libIsolated#typecheck", "nb-app#typecheck"]); - - // nb-app#typecheck DOES depend on nb-libNonIsolated#emitDeclarations - // (because nb-libNonIsolated is a transitive dep of nb-app that has emitDeclarations) - expect(graph).toContainEqual(["nb-libNonIsolated#emitDeclarations", "nb-app#typecheck"]); - - // nb-libNonIsolated#emitDeclarations depends on nb-libNonIsolated#typecheck (same-package) - expect(graph).toContainEqual(["nb-libNonIsolated#typecheck", "nb-libNonIsolated#emitDeclarations"]); - - // So nb-app#typecheck transitively waits for nb-libNonIsolated#typecheck via emitDeclarations. Correct. - // But it does NOT wait for nb-libIsolated#typecheck at all. That's the key. - - // nb-app#typecheck should still depend on transpile of both deps (for .js and d.ts files) - expect(graph).toContainEqual(["nb-libIsolated#transpile", "nb-app#typecheck"]); - expect(graph).toContainEqual(["nb-libNonIsolated#transpile", "nb-app#typecheck"]); - - // Verify nb-libIsolated#typecheck is completely independent of nb-app#typecheck. - // Collect all direct dependencies of nb-app#typecheck. - const appTypecheckDeps = graph - .filter(([, to]) => to === "nb-app#typecheck") - .map(([from]) => from); - - // None of them should be nb-libIsolated#typecheck - expect(appTypecheckDeps).not.toContain("nb-libIsolated#typecheck"); - - // And there should be no indirect path either: nb-libIsolated#typecheck - // should not appear as a dependency of any of nb-app#typecheck's deps - // (except via __start which is just a scheduling root). - // Specifically, nb-libIsolated#emitDeclarations should not exist at all. - const libIsolatedEmitTargets = [...targetGraph.targets.keys()].filter( - (id) => id === "nb-libIsolated#emitDeclarations" - ); - expect(libIsolatedEmitTargets).toEqual([]); - }); - - it("iso-lib3#typecheck depends on iso-lib2#transpile but NOT iso-lib2#typecheck", async () => { - // Matches the e2e test topology: - // dep-app → [dep-iso-lib, dep-iso-lib3, dep-non-iso-lib] - // dep-iso-lib → [dep-non-iso-lib] - // dep-iso-lib3 → [dep-iso-lib2] - // dep-iso-lib2 → [] (leaf, isolated) - // - // Only dep-non-iso-lib has emitDeclarations. All iso-lib* are isolated. - // Key assertion: dep-iso-lib3#typecheck has a direct edge from dep-iso-lib2#transpile - // but NO edge (direct or indirect) from dep-iso-lib2#typecheck. - const root = "/repos/dep-graph"; - - const packageInfos = createPackageInfo({ - "dep-app": ["dep-iso-lib", "dep-iso-lib3", "dep-non-iso-lib"], - "dep-iso-lib": ["dep-non-iso-lib"], - "dep-iso-lib3": ["dep-iso-lib2"], - "dep-iso-lib2": [], - "dep-non-iso-lib": [], - }); - - const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); - - await builder.addTargetConfig("transpile"); - await builder.addTargetConfig("typecheck", { - dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], - }); - - // Only dep-non-iso-lib has emitDeclarations - await builder.addTargetConfig("dep-non-iso-lib#emitDeclarations", { - dependsOn: ["typecheck"], - }); - - const targetGraph = await builder.build(["typecheck"]); - const graph = getGraphFromTargets(targetGraph); - - // dep-iso-lib3#typecheck DOES depend on dep-iso-lib2#transpile (via ^^transpile) - expect(graph).toContainEqual(["dep-iso-lib2#transpile", "dep-iso-lib3#typecheck"]); - - // dep-iso-lib3#typecheck does NOT depend on dep-iso-lib2#typecheck - expect(graph).not.toContainEqual(["dep-iso-lib2#typecheck", "dep-iso-lib3#typecheck"]); - - // dep-iso-lib2#emitDeclarations should not exist at all (iso-lib2 is isolated) - expect([...targetGraph.targets.keys()]).not.toContain("dep-iso-lib2#emitDeclarations"); - - // Verify the full set of direct deps for dep-iso-lib3#typecheck: - // should be exactly: own transpile + dep-iso-lib2#transpile (and __start scheduling root). - // Notably: NO dep-iso-lib2#typecheck and NO dep-non-iso-lib#emitDeclarations - // (dep-non-iso-lib is not in iso-lib3's dependency tree). - const isoLib3TypecheckDeps = graph - .filter(([, to]) => to === "dep-iso-lib3#typecheck") - .map(([from]) => from) - .filter((from) => from !== "__start") - .sort(); - - expect(isoLib3TypecheckDeps).toEqual([ - "dep-iso-lib2#transpile", // ^^transpile (transitive dep) - "dep-iso-lib3#transpile", // transpile (same-package) - ].sort()); - - // Also verify dep-app#typecheck depends on emitDeclarations from non-iso-lib - // but NOT on typecheck of any isolated package - const appTypecheckDeps = graph - .filter(([, to]) => to === "dep-app#typecheck") - .map(([from]) => from); - - expect(appTypecheckDeps).toContain("dep-non-iso-lib#emitDeclarations"); - expect(appTypecheckDeps).not.toContain("dep-iso-lib#typecheck"); - expect(appTypecheckDeps).not.toContain("dep-iso-lib2#typecheck"); - expect(appTypecheckDeps).not.toContain("dep-iso-lib3#typecheck"); - }); - - it("with real scripts: skips emitDeclarations targets for packages without the script", async () => { - // This test uses packageInfos WITH scripts defined, which exercises the - // skipPackagesWithoutScript logic in WorkspaceTargetGraphBuilder.addTargetConfig. - // Unlike the other tests that use createPackageInfo (no scripts), this verifies - // that phantom emitDeclarations targets are NOT created for isolated packages, - // preventing unwanted dependency chains after optimizeTargetGraph's removeNodes. - // - // Topology: rs-app → [rs-iso-lib, rs-non-iso-lib] - // rs-non-iso-lib has: transpile, typecheck, emitDeclarations - // rs-iso-lib has: transpile, typecheck (no emitDeclarations) - // rs-app has: transpile, typecheck (no emitDeclarations) - const root = "/repos/real-scripts"; - - const packageInfos: PackageInfos = { - "rs-app": { - packageJsonPath: "/path/to/rs-app/package.json", - name: "rs-app", - version: "1.0.0", - dependencies: { "rs-iso-lib": "*", "rs-non-iso-lib": "*" }, - scripts: { transpile: "echo transpile", typecheck: "echo typecheck" }, - }, - "rs-iso-lib": { - packageJsonPath: "/path/to/rs-iso-lib/package.json", - name: "rs-iso-lib", - version: "1.0.0", - dependencies: { "rs-non-iso-lib": "*" }, - scripts: { transpile: "echo transpile", typecheck: "echo typecheck" }, - }, - "rs-non-iso-lib": { - packageJsonPath: "/path/to/rs-non-iso-lib/package.json", - name: "rs-non-iso-lib", - version: "1.0.0", - dependencies: {}, - scripts: { transpile: "echo transpile", typecheck: "echo typecheck", emitDeclarations: "echo emitDeclarations" }, - }, - }; - - const builder = new WorkspaceTargetGraphBuilder(root, packageInfos, false); - - // Global pipeline tasks — emitDeclarations should only create targets where the script exists - await builder.addTargetConfig("transpile"); - await builder.addTargetConfig("emitDeclarations", { dependsOn: ["typecheck"] }); - await builder.addTargetConfig("typecheck", { - dependsOn: ["^^emitDeclarations", "transpile", "^^transpile"], - }); - - const targetGraph = await builder.build(["typecheck"]); - const graph = getGraphFromTargets(targetGraph); - - // emitDeclarations target should ONLY exist for rs-non-iso-lib - expect(targetGraph.targets.has("rs-non-iso-lib#emitDeclarations")).toBe(true); - expect(targetGraph.targets.has("rs-iso-lib#emitDeclarations")).toBe(false); - expect(targetGraph.targets.has("rs-app#emitDeclarations")).toBe(false); - - // rs-app#typecheck should depend on rs-non-iso-lib#emitDeclarations - expect(graph).toContainEqual(["rs-non-iso-lib#emitDeclarations", "rs-app#typecheck"]); - - // rs-app#typecheck should NOT depend on rs-iso-lib#typecheck - // (no phantom emitDeclarations → no reconnected typecheck chain) - expect(graph).not.toContainEqual(["rs-iso-lib#typecheck", "rs-app#typecheck"]); - - // rs-iso-lib#typecheck should depend on rs-non-iso-lib#emitDeclarations - expect(graph).toContainEqual(["rs-non-iso-lib#emitDeclarations", "rs-iso-lib#typecheck"]); - - // Transpile edges still work correctly - expect(graph).toContainEqual(["rs-iso-lib#transpile", "rs-app#typecheck"]); - expect(graph).toContainEqual(["rs-non-iso-lib#transpile", "rs-app#typecheck"]); - expect(graph).toContainEqual(["rs-app#transpile", "rs-app#typecheck"]); - expect(graph).toContainEqual(["rs-non-iso-lib#transpile", "rs-iso-lib#typecheck"]); - }); - }); }); From 931425146b6594c6015a25b536d91eefc0537575 Mon Sep 17 00:00:00 2001 From: Christian Gonzalez <1581488+christiango@users.noreply.github.com> Date: Tue, 3 Mar 2026 16:37:02 -0500 Subject: [PATCH 5/7] Fix comment --- packages/target-graph/src/expandDepSpecs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/target-graph/src/expandDepSpecs.ts b/packages/target-graph/src/expandDepSpecs.ts index 30cf0a0ec..7fabf3efd 100644 --- a/packages/target-graph/src/expandDepSpecs.ts +++ b/packages/target-graph/src/expandDepSpecs.ts @@ -10,7 +10,7 @@ import { getPackageAndTask, getStartTargetId, getTargetId } from "./targetId.js" * * When phantom targets are later removed by `removeNodes` (because `shouldRun` returns false), * their same-package dependencies get reconnected to their cross-package dependents, creating - * e.g. typecheck→typecheck chains through ghost emitDeclarations nodes. + * unnecessary work. * * Returns true if the target should be EXCLUDED from dependency expansion. */ From a8c9e252cc07589d1c71a79fa3a09b43b579a106 Mon Sep 17 00:00:00 2001 From: Christian Gonzalez <1581488+christiango@users.noreply.github.com> Date: Tue, 3 Mar 2026 16:39:50 -0500 Subject: [PATCH 6/7] Change files --- .../change-2e852706-e348-4268-baee-efe7ed5475ca.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 change/change-2e852706-e348-4268-baee-efe7ed5475ca.json diff --git a/change/change-2e852706-e348-4268-baee-efe7ed5475ca.json b/change/change-2e852706-e348-4268-baee-efe7ed5475ca.json new file mode 100644 index 000000000..d7503fa01 --- /dev/null +++ b/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" + } + ] +} \ No newline at end of file From 524ff4085fb90ec8535c8dd735d4b2d6cfa2fc66 Mon Sep 17 00:00:00 2001 From: Christian Gonzalez <1581488+christiango@users.noreply.github.com> Date: Wed, 4 Mar 2026 08:04:13 -0500 Subject: [PATCH 7/7] Another attempt --- .../WorkspaceTargetGraphBuilder.test.ts | 73 +++++++++++++++++++ packages/target-graph/src/expandDepSpecs.ts | 8 +- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts b/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts index 78dc9ecba..82a1fca64 100644 --- a/packages/target-graph/src/__tests__/WorkspaceTargetGraphBuilder.test.ts +++ b/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; +} diff --git a/packages/target-graph/src/expandDepSpecs.ts b/packages/target-graph/src/expandDepSpecs.ts index 7fabf3efd..83a359c11 100644 --- a/packages/target-graph/src/expandDepSpecs.ts +++ b/packages/target-graph/src/expandDepSpecs.ts @@ -5,7 +5,7 @@ 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. Phantom targets should be excluded from `^` and `^^` + * 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), @@ -20,6 +20,12 @@ function isPhantomTarget(targetId: string, task: string, targets: Map