diff --git a/packages/nx/src/tasks-runner/__snapshots__/task-graph-creator.spec.ts.snap b/packages/nx/src/tasks-runner/__snapshots__/task-graph-creator.spec.ts.snap index 003f18585e680..cf291ed6584e2 100644 --- a/packages/nx/src/tasks-runner/__snapshots__/task-graph-creator.spec.ts.snap +++ b/packages/nx/src/tasks-runner/__snapshots__/task-graph-creator.spec.ts.snap @@ -36,42 +36,6 @@ Object { } `; -exports[`TaskGraphCreator (tasks with dependency configurations) should create a task graph (builds depend on builds of dependencies with intermediate projects and circular dependencies between projects) 1`] = ` -Object { - "dependencies": Object { - "app1:build": Array [ - "common2:build", - ], - "common2:build": Array [], - }, - "roots": Array [ - "common2:build", - ], - "tasks": Object { - "app1:build": Object { - "id": "app1:build", - "overrides": Object {}, - "projectRoot": "app1-root", - "target": Object { - "configuration": undefined, - "project": "app1", - "target": "build", - }, - }, - "common2:build": Object { - "id": "common2:build", - "overrides": Object {}, - "projectRoot": "common2-root", - "target": Object { - "configuration": undefined, - "project": "common2", - "target": "build", - }, - }, - }, -} -`; - exports[`TaskGraphCreator (tasks with dependency configurations) should create a task graph (builds depend on builds of dependencies with intermediate projects and circular dependencies between projects) 2 1`] = ` Object { "dependencies": Object { diff --git a/packages/nx/src/tasks-runner/run-command.spec.ts b/packages/nx/src/tasks-runner/run-command.spec.ts index efdffcb0d1668..44fa05968cd11 100644 --- a/packages/nx/src/tasks-runner/run-command.spec.ts +++ b/packages/nx/src/tasks-runner/run-command.spec.ts @@ -458,7 +458,8 @@ describe('createTasksForProjectToRun', () => { }); }); - it('should handle circular dependencies between projects', () => { + it('should handle APP1 <=> LIB1(no build) => LIB2', () => { + // APP1 <=> LIB1 => LIB2 // App 1 depends on builds of its dependencies projectGraph.nodes.app1.data.targets.build.dependsOn = [ { @@ -501,13 +502,7 @@ describe('createTasksForProjectToRun', () => { }, }, }; - projectGraph.dependencies.lib2 = [ - { - type: DependencyType.static, - source: 'lib2', - target: 'lib1', - }, - ]; + projectGraph.dependencies.lib2 = []; const tasks = createTasksForProjectToRun( [projectGraph.nodes.app1], @@ -534,8 +529,28 @@ describe('createTasksForProjectToRun', () => { }); }); - it('should handle circular dependencies between projects with no tasks', () => { - // App 1 depends on builds of its dependencies + it('should error when APP1 <=> LIB1(no build) <=> LIB2', () => { + jest.spyOn(process, 'exit').mockImplementation(() => { + throw new Error(); + }); + + // Define Lib 2 with a build + projectGraph.nodes.lib2 = { + name: 'lib2', + type: 'lib', + data: { + root: 'lib2-root', + files: [], + targets: { + build: {}, + }, + }, + }; + + // Lib 1 does not have "build" + delete projectGraph.nodes.lib1.data.targets.build; + + // APP1 <=> LIB1 <=> LIB2 projectGraph.nodes.app1.data.targets.build.dependsOn = [ { target: 'build', @@ -543,6 +558,13 @@ describe('createTasksForProjectToRun', () => { }, ]; + projectGraph.nodes.lib2.data.targets.build.dependsOn = [ + { + target: 'build', + projects: 'dependencies', + }, + ]; + // App 1 depends on Lib 1 projectGraph.dependencies.app1.push({ type: DependencyType.static, @@ -550,8 +572,6 @@ describe('createTasksForProjectToRun', () => { target: 'lib1', }); - // Lib 1 does not have build but depends on Lib 2 - delete projectGraph.nodes.lib1.data.targets.build; projectGraph.dependencies.lib1.push( { type: DependencyType.static, @@ -565,45 +585,36 @@ describe('createTasksForProjectToRun', () => { } ); - // Lib 2 has a build - projectGraph.nodes.lib2 = { - name: 'lib2', - type: 'lib', - data: { - root: 'lib2-root', - files: [], - targets: { - build: {}, - }, - }, - }; - projectGraph.dependencies.lib2 = []; - - const tasks = createTasksForProjectToRun( - [projectGraph.nodes.app1], + projectGraph.dependencies.lib2 = [ { - target: 'build', - configuration: undefined, - overrides: {}, + type: DependencyType.static, + source: 'lib2', + target: 'lib1', }, - projectGraph, - projectGraph.nodes.app1.name - ); + ]; - expect(tasks).toContainEqual({ - id: 'app1:build', - target: { project: 'app1', target: 'build' }, - projectRoot: 'app1-root', - overrides: {}, - }); - expect(tasks).toContainEqual({ - id: 'lib2:build', - target: { project: 'lib2', target: 'build' }, - projectRoot: 'lib2-root', - overrides: {}, - }); + try { + createTasksForProjectToRun( + [projectGraph.nodes.app1], + { + target: 'build', + configuration: undefined, + overrides: {}, + }, + projectGraph, + projectGraph.nodes.app1.name + ); + fail(); + } catch (e) { + expect(process.exit).toHaveBeenCalledWith(1); + } }); + // Technically, this could work but it creates a lot of problems in the implementation, + // so instead we error saying that the circular dependency cannot be handled. + // xit('should handle APP1 => LIB1(no build) <=> LIB2', () => { + // }); + it('should throw an error for an invalid target', () => { jest.spyOn(process, 'exit').mockImplementation(() => { throw new Error(); diff --git a/packages/nx/src/tasks-runner/run-command.ts b/packages/nx/src/tasks-runner/run-command.ts index 5aa0a1aa3502f..6abb619a30777 100644 --- a/packages/nx/src/tasks-runner/run-command.ts +++ b/packages/nx/src/tasks-runner/run-command.ts @@ -258,7 +258,7 @@ function addTasksForProjectTarget( projectGraph: ProjectGraph, originalTargetExecutor: string, tasksMap: Map, - path: string[], + path: { targetIdentifier: string; hasTarget: boolean }[], seenSet: Set ) { const task = createTask({ @@ -356,7 +356,7 @@ function addTasksForProjectDependencyConfig( projectGraph: ProjectGraph, originalTargetExecutor: string, tasksMap: Map, - path: string[], + path: { targetIdentifier: string; hasTarget: boolean }[], seenSet: Set ) { const targetIdentifier = getId({ @@ -364,15 +364,14 @@ function addTasksForProjectDependencyConfig( target, configuration, }); - seenSet.add(project.name); - if (path.includes(targetIdentifier)) { - output.error({ - title: `Could not execute ${path[0]} because it has a circular dependency`, - bodyLines: [`${[...path, targetIdentifier].join(' --> ')}`], - }); - process.exit(1); - } + const pathFragment = { + targetIdentifier, + hasTarget: projectHasTarget(project, target), + }; + + const newPath = [...path, pathFragment]; + seenSet.add(project.name); if (tasksMap.has(targetIdentifier)) { return; @@ -385,10 +384,21 @@ function addTasksForProjectDependencyConfig( const depProject = projectGraph.nodes[ dep.target ] as ProjectGraphProjectNode; + if ( depProject && projectHasTarget(depProject, dependencyConfig.target) ) { + const depTargetId = getId({ + project: depProject.name, + target, + configuration: configuration, + }); + exitOnCircularDep(newPath, depTargetId); + if (seenSet.has(dep.target)) { + continue; + } + addTasksForProjectTarget( { project: depProject, @@ -401,17 +411,26 @@ function addTasksForProjectDependencyConfig( projectGraph, originalTargetExecutor, tasksMap, - [...path, targetIdentifier], + newPath, seenSet ); } else { - if (seenSet.has(dep.target)) { - continue; - } if (!depProject) { seenSet.add(dep.target); continue; } + const depTargetId = getId({ + project: depProject.name, + target, + configuration: configuration, + }); + + exitOnCircularDep(newPath, depTargetId); + + if (seenSet.has(dep.target)) { + continue; + } + addTasksForProjectDependencyConfig( depProject, { target, configuration, overrides }, @@ -420,7 +439,7 @@ function addTasksForProjectDependencyConfig( projectGraph, originalTargetExecutor, tasksMap, - path, + newPath, seenSet ); } @@ -439,12 +458,30 @@ function addTasksForProjectDependencyConfig( projectGraph, originalTargetExecutor, tasksMap, - [...path, targetIdentifier], + newPath, seenSet ); } } +function exitOnCircularDep( + path: { targetIdentifier: string; hasTarget: boolean }[], + targetIdentifier: string +) { + if ( + path.length > 0 && + path[path.length - 1].hasTarget && + path.filter((p) => p.targetIdentifier === targetIdentifier).length > 0 + ) { + const identifiers = path.map((p) => p.targetIdentifier); + output.error({ + title: `Could not execute ${identifiers[0]} because it has a circular dependency`, + bodyLines: [`${[...identifiers, targetIdentifier].join(' --> ')}`], + }); + process.exit(1); + } +} + function getId({ project, target, diff --git a/packages/nx/src/tasks-runner/task-graph-creator.spec.ts b/packages/nx/src/tasks-runner/task-graph-creator.spec.ts index ceb59edbbfe26..1f707ac5a6380 100644 --- a/packages/nx/src/tasks-runner/task-graph-creator.spec.ts +++ b/packages/nx/src/tasks-runner/task-graph-creator.spec.ts @@ -320,38 +320,6 @@ describe('TaskGraphCreator', () => { expect(taskGraph).toMatchSnapshot(); }); - it('should create a task graph (builds depend on builds of dependencies with intermediate projects and circular dependencies between projects)', () => { - delete projectGraph.nodes.common1.data.targets.build; - projectGraph.dependencies.common1.push({ - type: DependencyType.static, - source: 'common1', - target: 'common2', - }); - - projectGraph.dependencies.common2.push({ - type: DependencyType.static, - source: 'common2', - target: 'common1', - }); - - const tasks = createTasksForProjectToRun( - [projectGraph.nodes.app1], - { - target: 'build', - configuration: undefined, - overrides: {}, - }, - projectGraph, - null - ); - - const taskGraph = new TaskGraphCreator(projectGraph, {}).createTaskGraph( - tasks - ); - - expect(taskGraph).toMatchSnapshot(); - }); - it('should create a task graph (builds depend on builds of dependencies with intermediate projects and circular dependencies between projects) 2', () => { delete projectGraph.nodes.common1.data.targets.build; projectGraph.dependencies.common1.push({ diff --git a/packages/nx/src/utils/project-graph-utils.ts b/packages/nx/src/utils/project-graph-utils.ts index 41c3063a86318..ab83ec404ef38 100644 --- a/packages/nx/src/utils/project-graph-utils.ts +++ b/packages/nx/src/utils/project-graph-utils.ts @@ -10,7 +10,11 @@ export function projectHasTarget( project: ProjectGraphProjectNode, target: string ) { - return project.data && project.data.targets && project.data.targets[target]; + return !!( + project.data && + project.data.targets && + project.data.targets[target] + ); } export function projectHasTargetAndConfiguration(