From 67d3cddda15c64b5ffcb52d504aced90e93cfad4 Mon Sep 17 00:00:00 2001 From: Kunal Bhalla Date: Sat, 21 Sep 2013 23:00:13 -0700 Subject: [PATCH] [d3-sankey] Throw an error for graphs with cycles The current behaviour is to go into an infinite loop while trying to assign depths to nodes in case the graph has cycles. This commit changes it to explicitly throw an error instead till support for displaying cycles in sankeys is properly supported. Tested by modifying http://bost.ocks.org/mike/sankey/ data to have complex cycles and attempting to render. --- sankey/sankey.js | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/sankey/sankey.js b/sankey/sankey.js index c3bc59f..213a977 100644 --- a/sankey/sankey.js +++ b/sankey/sankey.js @@ -107,25 +107,45 @@ d3.sankey = function() { // Nodes are assigned the maximum breadth of incoming neighbors plus one; // nodes with no incoming links are assigned breadth zero, while // nodes with no outgoing links are assigned the maximum breadth. + // + // Throws an error in case the graph has any cycles by constantly checking + // the invariant that at every time a level is completed there are fewer + // nodes left to display. function computeNodeBreadths() { var remainingNodes = nodes, + previousLength = nodes.length, nextNodes, + enqueued, x = 0; while (remainingNodes.length) { nextNodes = []; + enqueued = {}; + remainingNodes.forEach(function(node) { node.x = x; node.dx = nodeWidth; - node.sourceLinks.forEach(function(link) { - nextNodes.push(link.target); - }); + node.sourceLinks + .filter(function(link) { + return !enqueued[link.target.name]; + }) + .forEach(function(link) { + enqueued[link.target.name] = true; + nextNodes.push(link.target); + }); }); + + newLength = nextNodes.length; + if (newLength >= previousLength) { + throw new Error("d3-sankey: cannot render sankey diagram because the graph has cycles."); + } + + previousLength = newLength; remainingNodes = nextNodes; + ++x; } - // moveSinksRight(x); scaleNodeBreadths((size[0] - nodeWidth) / (x - 1)); }