From 1cab740586f6a421908306a6f34f30def8269dcd Mon Sep 17 00:00:00 2001 From: mpsanchis Date: Mon, 10 Jun 2024 17:09:54 +0200 Subject: [PATCH 1/3] fix(js): filter project dependencies when calculating topological ordering For each project being sorted, only its dependent projects must be processed. --- .../utils/sort-projects-topologically.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/js/src/generators/release-version/utils/sort-projects-topologically.ts b/packages/js/src/generators/release-version/utils/sort-projects-topologically.ts index b89f583a4f276..d78d3fd071a8c 100644 --- a/packages/js/src/generators/release-version/utils/sort-projects-topologically.ts +++ b/packages/js/src/generators/release-version/utils/sort-projects-topologically.ts @@ -39,14 +39,16 @@ export function sortProjectsTopologically( sortedProjects.push(node); // Process each project that depends on the current node - filteredDependencies.forEach((dep) => { - const dependentNode = projectGraph.nodes[dep.source]; - const count = edges.get(dependentNode) - 1; - edges.set(dependentNode, count); - if (count === 0) { - processQueue.push(dependentNode); - } - }); + filteredDependencies + .filter((dep) => dep.target === node.name) + .forEach((dep) => { + const dependentNode = projectGraph.nodes[dep.source]; + const count = edges.get(dependentNode) - 1; + edges.set(dependentNode, count); + if (count === 0) { + processQueue.push(dependentNode); + } + }); } /** From d48cb41b3636bcc863ba2eb3f8189464a04e9705 Mon Sep 17 00:00:00 2001 From: mpsanchis Date: Tue, 11 Jun 2024 10:45:19 +0200 Subject: [PATCH 2/3] chore(js): Enhance unit test of topological sorting of project nodes --- .../utils/sort-projects-topologically.spec.ts | 319 ++++++++++++------ 1 file changed, 224 insertions(+), 95 deletions(-) diff --git a/packages/js/src/generators/release-version/utils/sort-projects-topologically.spec.ts b/packages/js/src/generators/release-version/utils/sort-projects-topologically.spec.ts index 7dbcf27cfb36d..bbad72af1e938 100644 --- a/packages/js/src/generators/release-version/utils/sort-projects-topologically.spec.ts +++ b/packages/js/src/generators/release-version/utils/sort-projects-topologically.spec.ts @@ -1,114 +1,243 @@ import { sortProjectsTopologically } from './sort-projects-topologically'; +import { DependencyType, StaticDependency } from '@nx/devkit'; describe('sortProjectsTopologically', () => { - it('should return empty array if no projects are provided', () => { - const projectGraph = { - dependencies: {}, - nodes: {}, - }; - const projectNodes = []; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual([]); - }); + describe('edge cases', () => { + it('should return empty array if no projects are provided', () => { + const projectGraph = { + dependencies: {}, + nodes: {}, + }; + const projectNodes = []; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([]); + }); - it('should return a single project if only one project is provided', () => { - const projectGraph = { - dependencies: {}, - nodes: { - project1: { - name: 'project1', - data: { - root: '', + it('should return a single project if only one project is provided', () => { + const projectGraph = { + dependencies: {}, + nodes: { + project1: { + name: 'project1', + data: { + root: '', + }, + type: 'app' as const, }, - type: 'app' as const, }, - }, - }; - const projectNodes = [projectGraph.nodes.project1]; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual([projectGraph.nodes.project1]); - }); + }; + const projectNodes = [projectGraph.nodes.project1]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([projectGraph.nodes.project1]); + }); - it('should return projects in the correct order', () => { - const projectGraph = { - dependencies: { - project1: [ - { - source: 'project1', - target: 'project2', - type: 'static', - }, - ], - project2: [], - }, - nodes: { - project1: { - name: 'project1', - data: { - root: '', - }, - type: 'app' as const, + it('should return the original list of nodes if a circular dependency is present', () => { + const projectGraph = { + dependencies: { + project1: [ + { + source: 'project1', + target: 'project2', + type: 'static', + }, + ], + project2: [ + { + source: 'project2', + target: 'project1', + type: 'static', + }, + ], }, - project2: { - name: 'project2', - data: { - root: '', + nodes: { + project1: { + name: 'project1', + data: { + root: '', + }, + type: 'app' as const, + }, + project2: { + name: 'project2', + data: { + root: '', + }, + type: 'app' as const, }, - type: 'app' as const, }, - }, - }; - const projectNodes = [ - projectGraph.nodes.project1, - projectGraph.nodes.project2, - ]; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual([ - projectGraph.nodes.project2, - projectGraph.nodes.project1, - ]); + }; + const projectNodes = [ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + ]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual(projectNodes); + }); }); - it('should return the original list of nodes if a circular dependency is present', () => { - const projectGraph = { - dependencies: { - project1: [ - { - source: 'project1', - target: 'project2', - type: 'static', - }, - ], - project2: [ - { - source: 'project2', - target: 'project1', - type: 'static', + describe('complex sorting cases', () => { + it('should return [B,A] if A depends on B', () => { + const projectGraph = { + dependencies: { + project1: [ + { + source: 'project1', + target: 'project2', + type: 'static', + }, + ], + project2: [], + }, + nodes: { + project1: { + name: 'project1', + data: { + root: '', + }, + type: 'app' as const, }, - ], - }, - nodes: { - project1: { - name: 'project1', - data: { - root: '', + project2: { + name: 'project2', + data: { + root: '', + }, + type: 'app' as const, }, - type: 'app' as const, }, - project2: { - name: 'project2', - data: { - root: '', + }; + const projectNodes = [ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + ]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([ + projectGraph.nodes.project2, + projectGraph.nodes.project1, + ]); + }); + it('should return [C,B,A] if A depends on B and B depends on C', () => { + const projectGraph = { + dependencies: { + project1: [ + { + source: 'project1', + target: 'project2', + type: 'static', + }, + ], + project2: [ + { + source: 'project2', + target: 'project3', + type: 'static', + }, + ], + }, + nodes: { + project1: { + name: 'project1', + data: { + root: '', + }, + type: 'app' as const, + }, + project2: { + name: 'project2', + data: { + root: '', + }, + type: 'app' as const, }, - type: 'app' as const, + project3: { + name: 'project3', + data: { + root: '', + }, + type: 'app' as const, + }, + }, + }; + const projectNodes = [ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + projectGraph.nodes.project3, + ]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([ + projectGraph.nodes.project3, + projectGraph.nodes.project2, + projectGraph.nodes.project1, + ]); + }); + + it('should return [A,B,C,D] if A has 0 dependencies, B has 1, C has 2, and D has 3', () => { + const graphNodes = Object.fromEntries( + [1, 2, 3, 4].map((n) => { + return [ + `project${n}`, + { + name: `project${n}`, + data: { root: '' }, + type: 'app' as const, + }, + ]; + }) + ); + const projectGraph = { + dependencies: { + project1: [], + project2: [ + { + source: 'project2', + target: 'project1', + type: 'static', + }, + ], + project3: [ + { + source: 'project3', + target: 'project1', + type: 'static', + }, + { + source: 'project3', + target: 'project2', + type: 'static', + }, + ], + project4: [ + { + source: 'project4', + target: 'project3', + type: 'static', + }, + { + source: 'project4', + target: 'project2', + type: 'static', + }, + { + source: 'project4', + target: 'project1', + type: 'static', + }, + ], }, - }, - }; - const projectNodes = [ - projectGraph.nodes.project1, - projectGraph.nodes.project2, - ]; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual(projectNodes); + nodes: graphNodes, + }; + const projectNodes = [ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + projectGraph.nodes.project3, + projectGraph.nodes.project4, + ]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + projectGraph.nodes.project3, + projectGraph.nodes.project4, + ]); + }); }); }); From 18df41b399e82054bb6774c08087db1c5ad36341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CJamesHenry=E2=80=9D?= Date: Tue, 11 Jun 2024 16:26:08 +0400 Subject: [PATCH 3/3] chore(js): less churn in existing tests --- .../utils/sort-projects-topologically.spec.ts | 455 +++++++++--------- 1 file changed, 234 insertions(+), 221 deletions(-) diff --git a/packages/js/src/generators/release-version/utils/sort-projects-topologically.spec.ts b/packages/js/src/generators/release-version/utils/sort-projects-topologically.spec.ts index bbad72af1e938..da9b8d82988a0 100644 --- a/packages/js/src/generators/release-version/utils/sort-projects-topologically.spec.ts +++ b/packages/js/src/generators/release-version/utils/sort-projects-topologically.spec.ts @@ -1,243 +1,256 @@ import { sortProjectsTopologically } from './sort-projects-topologically'; -import { DependencyType, StaticDependency } from '@nx/devkit'; describe('sortProjectsTopologically', () => { - describe('edge cases', () => { - it('should return empty array if no projects are provided', () => { - const projectGraph = { - dependencies: {}, - nodes: {}, - }; - const projectNodes = []; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual([]); - }); + it('should return empty array if no projects are provided', () => { + const projectGraph = { + dependencies: {}, + nodes: {}, + }; + const projectNodes = []; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([]); + }); - it('should return a single project if only one project is provided', () => { - const projectGraph = { - dependencies: {}, - nodes: { - project1: { - name: 'project1', - data: { - root: '', - }, - type: 'app' as const, + it('should return a single project if only one project is provided', () => { + const projectGraph = { + dependencies: {}, + nodes: { + project1: { + name: 'project1', + data: { + root: '', }, + type: 'app' as const, }, - }; - const projectNodes = [projectGraph.nodes.project1]; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual([projectGraph.nodes.project1]); - }); + }, + }; + const projectNodes = [projectGraph.nodes.project1]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([projectGraph.nodes.project1]); + }); - it('should return the original list of nodes if a circular dependency is present', () => { - const projectGraph = { - dependencies: { - project1: [ - { - source: 'project1', - target: 'project2', - type: 'static', - }, - ], - project2: [ - { - source: 'project2', - target: 'project1', - type: 'static', - }, - ], + it('should return [2,1] if 1 depends on 2', () => { + const projectGraph = { + dependencies: { + project1: [ + { + source: 'project1', + target: 'project2', + type: 'static', + }, + ], + project2: [], + }, + nodes: { + project1: { + name: 'project1', + data: { + root: '', + }, + type: 'app' as const, }, - nodes: { - project1: { - name: 'project1', - data: { - root: '', - }, - type: 'app' as const, - }, - project2: { - name: 'project2', - data: { - root: '', - }, - type: 'app' as const, + project2: { + name: 'project2', + data: { + root: '', }, + type: 'app' as const, }, - }; - const projectNodes = [ - projectGraph.nodes.project1, - projectGraph.nodes.project2, - ]; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual(projectNodes); - }); + }, + }; + const projectNodes = [ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + ]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([ + projectGraph.nodes.project2, + projectGraph.nodes.project1, + ]); }); - describe('complex sorting cases', () => { - it('should return [B,A] if A depends on B', () => { - const projectGraph = { - dependencies: { - project1: [ - { - source: 'project1', - target: 'project2', - type: 'static', - }, - ], - project2: [], + it('should return the original list of nodes if a circular dependency is present', () => { + const projectGraph = { + dependencies: { + project1: [ + { + source: 'project1', + target: 'project2', + type: 'static', + }, + ], + project2: [ + { + source: 'project2', + target: 'project1', + type: 'static', + }, + ], + }, + nodes: { + project1: { + name: 'project1', + data: { + root: '', + }, + type: 'app' as const, }, - nodes: { - project1: { - name: 'project1', - data: { - root: '', - }, - type: 'app' as const, - }, - project2: { - name: 'project2', - data: { - root: '', - }, - type: 'app' as const, + project2: { + name: 'project2', + data: { + root: '', }, + type: 'app' as const, }, - }; - const projectNodes = [ - projectGraph.nodes.project1, - projectGraph.nodes.project2, - ]; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual([ - projectGraph.nodes.project2, - projectGraph.nodes.project1, - ]); - }); - it('should return [C,B,A] if A depends on B and B depends on C', () => { - const projectGraph = { - dependencies: { - project1: [ - { - source: 'project1', - target: 'project2', - type: 'static', - }, - ], - project2: [ - { - source: 'project2', - target: 'project3', - type: 'static', - }, - ], + }, + }; + const projectNodes = [ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + ]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual(projectNodes); + }); + + it('should return [3,2,1] if 1 depends on 2 and 2 depends on 3', () => { + const projectGraph = { + dependencies: { + project1: [ + { + source: 'project1', + target: 'project2', + type: 'static', + }, + ], + project2: [ + { + source: 'project2', + target: 'project3', + type: 'static', + }, + ], + }, + nodes: { + project1: { + name: 'project1', + data: { + root: '', + }, + type: 'app' as const, + }, + project2: { + name: 'project2', + data: { + root: '', + }, + type: 'app' as const, }, - nodes: { - project1: { - name: 'project1', - data: { - root: '', - }, - type: 'app' as const, - }, - project2: { - name: 'project2', - data: { - root: '', - }, - type: 'app' as const, - }, - project3: { - name: 'project3', - data: { - root: '', - }, - type: 'app' as const, + project3: { + name: 'project3', + data: { + root: '', }, + type: 'app' as const, }, - }; - const projectNodes = [ - projectGraph.nodes.project1, - projectGraph.nodes.project2, - projectGraph.nodes.project3, - ]; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual([ - projectGraph.nodes.project3, - projectGraph.nodes.project2, - projectGraph.nodes.project1, - ]); - }); + }, + }; + const projectNodes = [ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + projectGraph.nodes.project3, + ]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([ + projectGraph.nodes.project3, + projectGraph.nodes.project2, + projectGraph.nodes.project1, + ]); + }); - it('should return [A,B,C,D] if A has 0 dependencies, B has 1, C has 2, and D has 3', () => { - const graphNodes = Object.fromEntries( - [1, 2, 3, 4].map((n) => { - return [ - `project${n}`, - { - name: `project${n}`, - data: { root: '' }, - type: 'app' as const, - }, - ]; - }) - ); - const projectGraph = { - dependencies: { - project1: [], - project2: [ - { - source: 'project2', - target: 'project1', - type: 'static', - }, - ], - project3: [ - { - source: 'project3', - target: 'project1', - type: 'static', - }, - { - source: 'project3', - target: 'project2', - type: 'static', - }, - ], - project4: [ - { - source: 'project4', - target: 'project3', - type: 'static', - }, - { - source: 'project4', - target: 'project2', - type: 'static', - }, - { - source: 'project4', - target: 'project1', - type: 'static', - }, - ], + it('should return [1,2,3,4] if 1 has zero dependencies, 2 has one, 3 has two, and 4 has three', () => { + const projectGraph = { + dependencies: { + project1: [], + project2: [ + { + source: 'project2', + target: 'project1', + type: 'static', + }, + ], + project3: [ + { + source: 'project3', + target: 'project1', + type: 'static', + }, + { + source: 'project3', + target: 'project2', + type: 'static', + }, + ], + project4: [ + { + source: 'project4', + target: 'project3', + type: 'static', + }, + { + source: 'project4', + target: 'project2', + type: 'static', + }, + { + source: 'project4', + target: 'project1', + type: 'static', + }, + ], + }, + nodes: { + project1: { + name: 'project1', + data: { + root: '', + }, + type: 'app' as const, + }, + project2: { + name: 'project2', + data: { + root: '', + }, + type: 'app' as const, + }, + project3: { + name: 'project3', + data: { + root: '', + }, + type: 'app' as const, + }, + project4: { + name: 'project4', + data: { + root: '', + }, + type: 'app' as const, }, - nodes: graphNodes, - }; - const projectNodes = [ - projectGraph.nodes.project1, - projectGraph.nodes.project2, - projectGraph.nodes.project3, - projectGraph.nodes.project4, - ]; - const result = sortProjectsTopologically(projectGraph, projectNodes); - expect(result).toEqual([ - projectGraph.nodes.project1, - projectGraph.nodes.project2, - projectGraph.nodes.project3, - projectGraph.nodes.project4, - ]); - }); + }, + }; + const projectNodes = [ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + projectGraph.nodes.project3, + projectGraph.nodes.project4, + ]; + const result = sortProjectsTopologically(projectGraph, projectNodes); + expect(result).toEqual([ + projectGraph.nodes.project1, + projectGraph.nodes.project2, + projectGraph.nodes.project3, + projectGraph.nodes.project4, + ]); }); });