diff --git a/change/p-graph-2021-09-23-16-29-58-update-cyclic-dep-err-msg.json b/change/p-graph-2021-09-23-16-29-58-update-cyclic-dep-err-msg.json new file mode 100644 index 0000000..4e97601 --- /dev/null +++ b/change/p-graph-2021-09-23-16-29-58-update-cyclic-dep-err-msg.json @@ -0,0 +1,8 @@ +{ + "type": "patch", + "comment": "update cyclic dependancy error to inlude the cycle", + "packageName": "p-graph", + "email": "cheruiyotbryan@gmail.com", + "dependentChangeType": "patch", + "date": "2021-09-23T13:29:58.510Z" +} diff --git a/src/PGraph.ts b/src/PGraph.ts index 5b2ca4a..a434797 100644 --- a/src/PGraph.ts +++ b/src/PGraph.ts @@ -37,11 +37,10 @@ export class PGraph { throw new Error("We could not find a node in the graph with no dependencies, this likely means there is a cycle including all nodes"); } - const hasCycles = graphHasCycles(this.pGraphDependencyMap); + const graph = graphHasCycles(this.pGraphDependencyMap); - if (hasCycles.hasCycle) { - const { nodeId, dependsOn, dependedOnBy } = hasCycles.details; - throw new Error(`The dependency graph has a cycle at ${nodeId} which depends on ${dependsOn} and is depended on by ${dependedOnBy}`); + if (graph.hasCycle) { + throw new Error(`A cycle has been detected including the following nodes:\n${graph.cycle.join("\n")}`); } } diff --git a/src/__tests__/index.tests.ts b/src/__tests__/index.tests.ts index 3cde890..821b468 100644 --- a/src/__tests__/index.tests.ts +++ b/src/__tests__/index.tests.ts @@ -202,7 +202,39 @@ describe("Public API", () => { ["C", "D"], ["D", "B"], ]; - const expectedErrorMessage = "The dependency graph has a cycle at B which depends on A,D and is depended on by C"; + const expectedErrorMessage = `A cycle has been detected including the following nodes: +B +C +D`; + expect(() => pGraph(nodeMap, dependencies)).toThrow(expectedErrorMessage); + }); + + it("throws an exception in the first instance of a cycle that has been detected when there are overlapped cycles", async () => { + // This is almost the same as the last test, except the root node is not a part of the cycle + const nodeMap: PGraphNodeMap = new Map([ + ["A", { run: () => Promise.resolve() }], + ["B", { run: () => Promise.resolve() }], + ["C", { run: () => Promise.resolve() }], + ["D", { run: () => Promise.resolve() }], + ["E", { run: () => Promise.resolve() }], + ["F", { run: () => Promise.resolve() }], + ]); + // B -> C -> E -> F -> D is the first cycle detected + const dependencies: DependencyList = [ + ["A", "B"], + ["B", "C"], + ["C", "D"], + ["D", "B"], + ["C", "E"], + ["E", "F"], + ["F", "D"], + ]; + const expectedErrorMessage = `A cycle has been detected including the following nodes: +B +C +E +F +D` expect(() => pGraph(nodeMap, dependencies)).toThrow(expectedErrorMessage); }); diff --git a/src/graphHasCycles.ts b/src/graphHasCycles.ts index c780930..baaa550 100644 --- a/src/graphHasCycles.ts +++ b/src/graphHasCycles.ts @@ -2,7 +2,7 @@ import { PGraphNodeWithCyclicDependency, PGraphNodeWithNoCyclicDependency, PGrap /** * Checks for any cycles in the dependency graph, returning `{ hasCycle: false }` if no cycles were detected. - * Otherwise it returns the details of where the cycle was detected. + * Otherwise it returns the chain of nodes where the cycle was detected. */ export function graphHasCycles(pGraphDependencyMap: Map): PGraphNodeWithCyclicDependency | PGraphNodeWithNoCyclicDependency { /** @@ -13,7 +13,7 @@ export function graphHasCycles(pGraphDependencyMap: Map(); - for (const [nodeId, nodes] of pGraphDependencyMap.entries()) { + for (const [nodeId] of pGraphDependencyMap.entries()) { /** * Test whether this node has already been visited or not. */ @@ -21,15 +21,9 @@ export function graphHasCycles(pGraphDependencyMap: Map, visitMap: Map, nodeId: string): boolean => { +const searchForCycleDFS = (graph: Map, visitMap: Map, nodeId: string): string[] => { const stack: StackElement[] = [{ node: nodeId, traversing: false }]; while (stack.length > 0) { const current = stack[stack.length - 1]; @@ -65,7 +59,8 @@ const hasCycleDFS = (graph: Map, visitMap: M * The current node has already been visited, * hence there is a cycle. */ - return true; + const listOfCycle = stack.filter(i => i.traversing).map(a => a.node); + return listOfCycle.slice(listOfCycle.indexOf(current.node)); } else { /** * The current node has already been fully traversed @@ -90,9 +85,9 @@ const hasCycleDFS = (graph: Map, visitMap: M } /** - * Add the current node's dependencies to the stack + * Add the current node's dependents to the stack */ - stack.push(...[...node.dependsOn].map((n) => ({ node: n, traversing: false }))); + stack.push(...[...node.dependedOnBy].map((n) => ({ node: n, traversing: false }))); } else { /** * The current node has now been fully traversed. @@ -101,5 +96,5 @@ const hasCycleDFS = (graph: Map, visitMap: M stack.pop(); } } - return false; + return []; }; diff --git a/src/types.ts b/src/types.ts index 941a81a..134bdb3 100644 --- a/src/types.ts +++ b/src/types.ts @@ -66,21 +66,7 @@ export interface PGraphNodeWithCyclicDependency { */ hasCycle: true; /** - * Details on where the cyclic dependency was detected. + * Chain of node where the cyclic dependency was detected. */ - details: { - /** - * The identifier of this node, where the cyclic dependency was detected - */ - nodeId: string, - /** - * The set of nodes that this node depends on. - */ - dependsOn: string[]; - - /** - * The set of nodes that depend on this node. - */ - dependedOnBy: string[]; - } + cycle: string[] }