From 7564b92852b044c85b4181ac3efb17fb8319c867 Mon Sep 17 00:00:00 2001 From: zbynekstara Date: Wed, 18 Dec 2024 16:39:11 +0100 Subject: [PATCH 1/8] fix(layout.DirectedGraph): better error message when dagre fails to connect to container --- .../joint-layout-directed-graph/DirectedGraph.mjs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/joint-layout-directed-graph/DirectedGraph.mjs b/packages/joint-layout-directed-graph/DirectedGraph.mjs index fef1b73cf..730c63931 100644 --- a/packages/joint-layout-directed-graph/DirectedGraph.mjs +++ b/packages/joint-layout-directed-graph/DirectedGraph.mjs @@ -160,7 +160,16 @@ export const DirectedGraph = { glGraph.setGraph(glLabel); // Executes the layout. - dagreUtil.layout(glGraph, { debugTiming: !!opt.debugTiming }); + try { + dagreUtil.layout(glGraph, { debugTiming: !!opt.debugTiming }); + } catch (err) { + if (err instanceof TypeError) { + // see https://github.com/clientIO/joint/issues/455 + throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); + } else { + throw err; + } + } // Wrap all graph changes into a batch. graph.startBatch('layout'); From 33b3875bb4c74eb7875cee1497966e1a3f0111b2 Mon Sep 17 00:00:00 2001 From: zbynekstara Date: Mon, 6 Jan 2025 12:18:11 +0100 Subject: [PATCH 2/8] separate try-catch block for performance in old V8 --- .../DirectedGraph.mjs | 23 ++++++----- .../joint-layout-directed-graph/test/index.js | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/joint-layout-directed-graph/DirectedGraph.mjs b/packages/joint-layout-directed-graph/DirectedGraph.mjs index 730c63931..5761e0be9 100644 --- a/packages/joint-layout-directed-graph/DirectedGraph.mjs +++ b/packages/joint-layout-directed-graph/DirectedGraph.mjs @@ -94,6 +94,17 @@ export const DirectedGraph = { } }, + tryLayout: function(glGraph, opt) { + try { + dagreUtil.layout(glGraph, opt); + } catch (err) { + // ASSUMPTION: Only one error is relevant here: + // - `Uncaught TypeError: Cannot set property 'rank' of undefined` + // - See https://github.com/clientIO/joint/issues/455 + throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); + } + }, + layout: function(graphOrCells, opt) { var graph; @@ -160,16 +171,8 @@ export const DirectedGraph = { glGraph.setGraph(glLabel); // Executes the layout. - try { - dagreUtil.layout(glGraph, { debugTiming: !!opt.debugTiming }); - } catch (err) { - if (err instanceof TypeError) { - // see https://github.com/clientIO/joint/issues/455 - throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); - } else { - throw err; - } - } + // - See https://stackoverflow.com/a/19728876/2263595 + this.tryLayout(glGraph, { debugTiming: !!opt.debugTiming }); // Wrap all graph changes into a batch. graph.startBatch('layout'); diff --git a/packages/joint-layout-directed-graph/test/index.js b/packages/joint-layout-directed-graph/test/index.js index 11d659296..25d1d71c2 100644 --- a/packages/joint-layout-directed-graph/test/index.js +++ b/packages/joint-layout-directed-graph/test/index.js @@ -317,5 +317,46 @@ QUnit.module('DirectedGraph', function(hooks) { assert.deepEqual(bbox.toJSON(), graph.getBBox().toJSON()); }); + + QUnit.test('should throw an understandable error when trying to connect a child to a container', function(assert) { + const elements = [ + new joint.shapes.standard.Rectangle({ position: { x: 50, y: 50 }, size: { width: 300, height: 300 } }), + new joint.shapes.standard.Rectangle({ position: { x: 175, y: 175 }, size: {width: 50, height: 50 } }), + new joint.shapes.standard.Rectangle({ position: { x: 400, y: 150 }, size: { width: 100, height: 100 } }), + new joint.shapes.standard.Rectangle({ position: { x: 150, y: 400 }, size: { width: 100, height: 100 } }) + ]; + + elements[0].embed(elements[1]); + + const links = [ + new joint.shapes.standard.Link({ source: { id: elements[0].id }, target: { id: elements[1].id }}), // container -> its child + new joint.shapes.standard.Link({ source: { id: elements[1].id }, target: { id: elements[0].id }}), // child -> its container + new joint.shapes.standard.Link({ source: { id: elements[0].id }, target: { id: elements[2].id }}) // container -> unrelated element + // these are ok: + //new joint.shapes.standard.Link({ source: { id: elements[1].id }, target: { id: elements[2].id }}), // child -> unrelated element + //new joint.shapes.standard.Link({ source: { id: elements[2].id }, target: { id: elements[3].id }}) // unrelated element -> unrelated element + ]; + + let cells; + const error = new Error('DirectedGraph: It is not possible to connect a child to a container.'); + + cells = elements.concat([links[0]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph); + }, error); + + cells = elements.concat([links[1]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph); + }, error); + + cells = elements.concat([links[2]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph); + }, error); + }) }); }); From 2d7c3d73a69c0f935c62fec4ebc4126503170045 Mon Sep 17 00:00:00 2001 From: zbynekstara Date: Mon, 6 Jan 2025 17:28:51 +0100 Subject: [PATCH 3/8] check graph instead of try-catch --- .../DirectedGraph.mjs | 35 ++++++------ .../joint-layout-directed-graph/test/index.js | 53 ++++++++++++++++--- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/packages/joint-layout-directed-graph/DirectedGraph.mjs b/packages/joint-layout-directed-graph/DirectedGraph.mjs index 5761e0be9..50e9d3e56 100644 --- a/packages/joint-layout-directed-graph/DirectedGraph.mjs +++ b/packages/joint-layout-directed-graph/DirectedGraph.mjs @@ -94,17 +94,6 @@ export const DirectedGraph = { } }, - tryLayout: function(glGraph, opt) { - try { - dagreUtil.layout(glGraph, opt); - } catch (err) { - // ASSUMPTION: Only one error is relevant here: - // - `Uncaught TypeError: Cannot set property 'rank' of undefined` - // - See https://github.com/clientIO/joint/issues/455 - throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); - } - }, - layout: function(graphOrCells, opt) { var graph; @@ -120,6 +109,22 @@ export const DirectedGraph = { // This is not needed anymore. graphOrCells = null; + // Check that we are not trying to connect a child to a container: + // - child to a container + // - container to a child + // - container to a container + graph.getLinks().forEach((link) => { + const source = link.getSourceElement(); + const target = link.getTargetElement(); + // is container = is element && has at least one embedded element + const isSourceContainer = source && (source.getEmbeddedCells().filter((cell) => cell.isElement()).length !== 0); + const isTargetContainer = target && (target.getEmbeddedCells().filter((cell) => cell.isElement()).length !== 0); + if ((isSourceContainer && target) || (source && isTargetContainer)) { + // see https://github.com/clientIO/joint/issues/455 + throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); + } + }); + opt = util.defaults(opt || {}, { resizeClusters: true, clusterPadding: 10, @@ -127,8 +132,7 @@ export const DirectedGraph = { exportLink: this.exportLink }); - // create a graphlib.Graph that represents the joint.dia.Graph - // var glGraph = graph.toGraphLib({ + // Create a graphlib.Graph that represents the joint.dia.Graph var glGraph = DirectedGraph.toGraphLib(graph, { directed: true, // We are about to use edge naming feature. @@ -170,9 +174,8 @@ export const DirectedGraph = { // Set the option object for the graph label. glGraph.setGraph(glLabel); - // Executes the layout. - // - See https://stackoverflow.com/a/19728876/2263595 - this.tryLayout(glGraph, { debugTiming: !!opt.debugTiming }); + // Execute the layout. + dagreUtil.layout(glGraph, { debugTiming: !!opt.debugTiming }); // Wrap all graph changes into a batch. graph.startBatch('layout'); diff --git a/packages/joint-layout-directed-graph/test/index.js b/packages/joint-layout-directed-graph/test/index.js index 25d1d71c2..db1f3143e 100644 --- a/packages/joint-layout-directed-graph/test/index.js +++ b/packages/joint-layout-directed-graph/test/index.js @@ -322,19 +322,26 @@ QUnit.module('DirectedGraph', function(hooks) { const elements = [ new joint.shapes.standard.Rectangle({ position: { x: 50, y: 50 }, size: { width: 300, height: 300 } }), new joint.shapes.standard.Rectangle({ position: { x: 175, y: 175 }, size: {width: 50, height: 50 } }), - new joint.shapes.standard.Rectangle({ position: { x: 400, y: 150 }, size: { width: 100, height: 100 } }), - new joint.shapes.standard.Rectangle({ position: { x: 150, y: 400 }, size: { width: 100, height: 100 } }) + new joint.shapes.standard.Rectangle({ position: { x: 400, y: 50 }, size: { width: 300, height: 300 } }), + new joint.shapes.standard.Rectangle({ position: { x: 525, y: 175 }, size: { width: 50, height: 50 } }), ]; elements[0].embed(elements[1]); + elements[2].embed(elements[3]); const links = [ - new joint.shapes.standard.Link({ source: { id: elements[0].id }, target: { id: elements[1].id }}), // container -> its child + // this throws error: new joint.shapes.standard.Link({ source: { id: elements[1].id }, target: { id: elements[0].id }}), // child -> its container - new joint.shapes.standard.Link({ source: { id: elements[0].id }, target: { id: elements[2].id }}) // container -> unrelated element - // these are ok: - //new joint.shapes.standard.Link({ source: { id: elements[1].id }, target: { id: elements[2].id }}), // child -> unrelated element - //new joint.shapes.standard.Link({ source: { id: elements[2].id }, target: { id: elements[3].id }}) // unrelated element -> unrelated element + new joint.shapes.standard.Link({ source: { id: elements[1].id }, target: { id: elements[2].id }}), // child -> unrelated container + new joint.shapes.standard.Link({ source: { id: elements[0].id }, target: { id: elements[1].id }}), // container -> its child + new joint.shapes.standard.Link({ source: { id: elements[0].id }, target: { id: elements[3].id }}), // container -> unrelated child + new joint.shapes.standard.Link({ source: { id: elements[0].id }, target: { id: elements[2].id }}), // container -> unrelated container + // this is ok: + new joint.shapes.standard.Link({ source: { id: elements[1].id }, target: { id: elements[3].id }}), // child -> unrelated child + new joint.shapes.standard.Link({ source: { id: elements[1].id }, target: { x: 0, y: 0 }}), // child -> point + new joint.shapes.standard.Link({ source: { x: 0, y: 0 }, target: { id: elements[1].id }}), // point -> child + new joint.shapes.standard.Link({ source: { id: elements[0].id }, target: { x: 0, y: 0 }}), // container -> point + new joint.shapes.standard.Link({ source: { x: 0, y: 0 }, target: { id: elements[0].id }}), // point -> container ]; let cells; @@ -357,6 +364,38 @@ QUnit.module('DirectedGraph', function(hooks) { assert.throws(() => { DirectedGraph.layout(graph); }, error); + + cells = elements.concat([links[3]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph); + }, error); + + cells = elements.concat([links[4]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph); + }, error); + + cells = elements.concat([links[5]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph) instanceof g.Rect); + + cells = elements.concat([links[6]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph) instanceof g.Rect); + + cells = elements.concat([links[7]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph) instanceof g.Rect); + + cells = elements.concat([links[8]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph) instanceof g.Rect); + + cells = elements.concat([links[9]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph) instanceof g.Rect); }) }); }); From d9cde9e12b5fc54b3d0ea1982c4f4e82925eaf8d Mon Sep 17 00:00:00 2001 From: zbynekstara Date: Tue, 7 Jan 2025 11:51:22 +0100 Subject: [PATCH 4/8] add checkContainerConnections option, use Array.some() --- .../DirectedGraph.mjs | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/joint-layout-directed-graph/DirectedGraph.mjs b/packages/joint-layout-directed-graph/DirectedGraph.mjs index 50e9d3e56..2be0b97a2 100644 --- a/packages/joint-layout-directed-graph/DirectedGraph.mjs +++ b/packages/joint-layout-directed-graph/DirectedGraph.mjs @@ -109,29 +109,32 @@ export const DirectedGraph = { // This is not needed anymore. graphOrCells = null; - // Check that we are not trying to connect a child to a container: - // - child to a container - // - container to a child - // - container to a container - graph.getLinks().forEach((link) => { - const source = link.getSourceElement(); - const target = link.getTargetElement(); - // is container = is element && has at least one embedded element - const isSourceContainer = source && (source.getEmbeddedCells().filter((cell) => cell.isElement()).length !== 0); - const isTargetContainer = target && (target.getEmbeddedCells().filter((cell) => cell.isElement()).length !== 0); - if ((isSourceContainer && target) || (source && isTargetContainer)) { - // see https://github.com/clientIO/joint/issues/455 - throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); - } - }); - opt = util.defaults(opt || {}, { + checkContainerConnections: true, resizeClusters: true, clusterPadding: 10, exportElement: this.exportElement, exportLink: this.exportLink }); + // Check that we are not trying to connect a child to a container: + // - child to a container + // - container to a child + // - container to a container + if (opt.checkContainerConnections) { + graph.getLinks().forEach((link) => { + const source = link.getSourceElement(); + const target = link.getTargetElement(); + // is container = is element && has at least one embedded element + const isSourceContainer = source && source.getEmbeddedCells().some((cell) => cell.isElement()); + const isTargetContainer = target && target.getEmbeddedCells().some((cell) => cell.isElement()); + if ((isSourceContainer && target) || (source && isTargetContainer)) { + // see https://github.com/clientIO/joint/issues/455 + throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); + } + }); + } + // Create a graphlib.Graph that represents the joint.dia.Graph var glGraph = DirectedGraph.toGraphLib(graph, { directed: true, From d74c60e33309efdb8b92324302e6214c4ff19c32 Mon Sep 17 00:00:00 2001 From: zbynekstara Date: Tue, 7 Jan 2025 11:59:09 +0100 Subject: [PATCH 5/8] add tests for checkContainerConnections option --- .../joint-layout-directed-graph/test/index.js | 61 ++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/packages/joint-layout-directed-graph/test/index.js b/packages/joint-layout-directed-graph/test/index.js index db1f3143e..98efd8924 100644 --- a/packages/joint-layout-directed-graph/test/index.js +++ b/packages/joint-layout-directed-graph/test/index.js @@ -344,8 +344,11 @@ QUnit.module('DirectedGraph', function(hooks) { new joint.shapes.standard.Link({ source: { x: 0, y: 0 }, target: { id: elements[0].id }}), // point -> container ]; - let cells; - const error = new Error('DirectedGraph: It is not possible to connect a child to a container.'); + let cells, error; + + // Using `checkContainerConnections` option (default): + + error = new Error('DirectedGraph: It is not possible to connect a child to a container.'); cells = elements.concat([links[0]]); graph.resetCells(cells); @@ -396,6 +399,60 @@ QUnit.module('DirectedGraph', function(hooks) { cells = elements.concat([links[9]]); graph.resetCells(cells); assert.ok(DirectedGraph.layout(graph) instanceof g.Rect); + + // Disabling `checkContainerConnections` option: + + error = new TypeError(`Cannot set properties of undefined (setting 'rank')`); + + cells = elements.concat([links[0]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph, { checkContainerConnections: false }); + }, error); + + cells = elements.concat([links[1]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph, { checkContainerConnections: false }); + }, error); + + cells = elements.concat([links[2]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph, { checkContainerConnections: false }); + }, error); + + cells = elements.concat([links[3]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph, { checkContainerConnections: false }); + }, error); + + cells = elements.concat([links[4]]); + graph.resetCells(cells); + assert.throws(() => { + DirectedGraph.layout(graph, { checkContainerConnections: false }); + }, error); + + cells = elements.concat([links[5]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + + cells = elements.concat([links[6]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + + cells = elements.concat([links[7]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + + cells = elements.concat([links[8]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + + cells = elements.concat([links[9]]); + graph.resetCells(cells); + assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); }) }); }); From 8f75075305da3d4bfdd42dafdf4a315fd44be14e Mon Sep 17 00:00:00 2001 From: zbynekstara Date: Tue, 7 Jan 2025 12:06:13 +0100 Subject: [PATCH 6/8] add checkContainerConnections option to types --- packages/joint-layout-directed-graph/DirectedGraph.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/joint-layout-directed-graph/DirectedGraph.d.ts b/packages/joint-layout-directed-graph/DirectedGraph.d.ts index c6472823b..cc043671a 100644 --- a/packages/joint-layout-directed-graph/DirectedGraph.d.ts +++ b/packages/joint-layout-directed-graph/DirectedGraph.d.ts @@ -25,6 +25,7 @@ export namespace DirectedGraph { rankSep?: number; marginX?: number; marginY?: number; + checkContainerConnections: boolean; resizeClusters?: boolean; clusterPadding?: dia.Padding; debugTiming?: boolean; From f6cee55123f08d98488bb61f23d5ed98d6d23a31 Mon Sep 17 00:00:00 2001 From: zbynekstara Date: Tue, 7 Jan 2025 12:19:35 +0100 Subject: [PATCH 7/8] rename option to validateGraph --- .../DirectedGraph.d.ts | 2 +- .../DirectedGraph.mjs | 4 ++-- .../joint-layout-directed-graph/test/index.js | 24 +++++++++---------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/joint-layout-directed-graph/DirectedGraph.d.ts b/packages/joint-layout-directed-graph/DirectedGraph.d.ts index cc043671a..8901d3b61 100644 --- a/packages/joint-layout-directed-graph/DirectedGraph.d.ts +++ b/packages/joint-layout-directed-graph/DirectedGraph.d.ts @@ -25,7 +25,7 @@ export namespace DirectedGraph { rankSep?: number; marginX?: number; marginY?: number; - checkContainerConnections: boolean; + validateGraph?: boolean; resizeClusters?: boolean; clusterPadding?: dia.Padding; debugTiming?: boolean; diff --git a/packages/joint-layout-directed-graph/DirectedGraph.mjs b/packages/joint-layout-directed-graph/DirectedGraph.mjs index 2be0b97a2..82e8864f0 100644 --- a/packages/joint-layout-directed-graph/DirectedGraph.mjs +++ b/packages/joint-layout-directed-graph/DirectedGraph.mjs @@ -110,7 +110,7 @@ export const DirectedGraph = { graphOrCells = null; opt = util.defaults(opt || {}, { - checkContainerConnections: true, + validateGraph: true, resizeClusters: true, clusterPadding: 10, exportElement: this.exportElement, @@ -121,7 +121,7 @@ export const DirectedGraph = { // - child to a container // - container to a child // - container to a container - if (opt.checkContainerConnections) { + if (opt.validateGraph) { graph.getLinks().forEach((link) => { const source = link.getSourceElement(); const target = link.getTargetElement(); diff --git a/packages/joint-layout-directed-graph/test/index.js b/packages/joint-layout-directed-graph/test/index.js index 98efd8924..d5b8938eb 100644 --- a/packages/joint-layout-directed-graph/test/index.js +++ b/packages/joint-layout-directed-graph/test/index.js @@ -346,7 +346,7 @@ QUnit.module('DirectedGraph', function(hooks) { let cells, error; - // Using `checkContainerConnections` option (default): + // Using `validateGraph` option (default): error = new Error('DirectedGraph: It is not possible to connect a child to a container.'); @@ -400,59 +400,59 @@ QUnit.module('DirectedGraph', function(hooks) { graph.resetCells(cells); assert.ok(DirectedGraph.layout(graph) instanceof g.Rect); - // Disabling `checkContainerConnections` option: + // Disabling `validateGraph` option: error = new TypeError(`Cannot set properties of undefined (setting 'rank')`); cells = elements.concat([links[0]]); graph.resetCells(cells); assert.throws(() => { - DirectedGraph.layout(graph, { checkContainerConnections: false }); + DirectedGraph.layout(graph, { validateGraph: false }); }, error); cells = elements.concat([links[1]]); graph.resetCells(cells); assert.throws(() => { - DirectedGraph.layout(graph, { checkContainerConnections: false }); + DirectedGraph.layout(graph, { validateGraph: false }); }, error); cells = elements.concat([links[2]]); graph.resetCells(cells); assert.throws(() => { - DirectedGraph.layout(graph, { checkContainerConnections: false }); + DirectedGraph.layout(graph, { validateGraph: false }); }, error); cells = elements.concat([links[3]]); graph.resetCells(cells); assert.throws(() => { - DirectedGraph.layout(graph, { checkContainerConnections: false }); + DirectedGraph.layout(graph, { validateGraph: false }); }, error); cells = elements.concat([links[4]]); graph.resetCells(cells); assert.throws(() => { - DirectedGraph.layout(graph, { checkContainerConnections: false }); + DirectedGraph.layout(graph, { validateGraph: false }); }, error); cells = elements.concat([links[5]]); graph.resetCells(cells); - assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + assert.ok(DirectedGraph.layout(graph, { validateGraph: false }) instanceof g.Rect); cells = elements.concat([links[6]]); graph.resetCells(cells); - assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + assert.ok(DirectedGraph.layout(graph, { validateGraph: false }) instanceof g.Rect); cells = elements.concat([links[7]]); graph.resetCells(cells); - assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + assert.ok(DirectedGraph.layout(graph, { validateGraph: false }) instanceof g.Rect); cells = elements.concat([links[8]]); graph.resetCells(cells); - assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + assert.ok(DirectedGraph.layout(graph, { validateGraph: false }) instanceof g.Rect); cells = elements.concat([links[9]]); graph.resetCells(cells); - assert.ok(DirectedGraph.layout(graph, { checkContainerConnections: false }) instanceof g.Rect); + assert.ok(DirectedGraph.layout(graph, { validateGraph: false }) instanceof g.Rect); }) }); }); From ee6df9619747ad29d2a57a9a711a0f036ca1d597 Mon Sep 17 00:00:00 2001 From: zbynekstara Date: Wed, 8 Jan 2025 11:23:49 +0100 Subject: [PATCH 8/8] move validateGraph logic to a separate function --- .../DirectedGraph.mjs | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/packages/joint-layout-directed-graph/DirectedGraph.mjs b/packages/joint-layout-directed-graph/DirectedGraph.mjs index 82e8864f0..e25e0bdd7 100644 --- a/packages/joint-layout-directed-graph/DirectedGraph.mjs +++ b/packages/joint-layout-directed-graph/DirectedGraph.mjs @@ -94,6 +94,26 @@ export const DirectedGraph = { } }, + // Validate that the graph does not have an arrangement of cells that Dagre considers invalid: + // - child connected to a container + // - container connected to a child + // - container connected to a container + // Throw an understandable error if one of the above situations is the case. + validateGraph: function(graph) { + + graph.getLinks().forEach((link) => { + const source = link.getSourceElement(); + const target = link.getTargetElement(); + // is container = is element && has at least one embedded element + const isSourceContainer = source && source.getEmbeddedCells().some((cell) => cell.isElement()); + const isTargetContainer = target && target.getEmbeddedCells().some((cell) => cell.isElement()); + if ((isSourceContainer && target) || (source && isTargetContainer)) { + // see https://github.com/clientIO/joint/issues/455 + throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); + } + }); + }, + layout: function(graphOrCells, opt) { var graph; @@ -117,23 +137,8 @@ export const DirectedGraph = { exportLink: this.exportLink }); - // Check that we are not trying to connect a child to a container: - // - child to a container - // - container to a child - // - container to a container - if (opt.validateGraph) { - graph.getLinks().forEach((link) => { - const source = link.getSourceElement(); - const target = link.getTargetElement(); - // is container = is element && has at least one embedded element - const isSourceContainer = source && source.getEmbeddedCells().some((cell) => cell.isElement()); - const isTargetContainer = target && target.getEmbeddedCells().some((cell) => cell.isElement()); - if ((isSourceContainer && target) || (source && isTargetContainer)) { - // see https://github.com/clientIO/joint/issues/455 - throw new Error('DirectedGraph: It is not possible to connect a child to a container.'); - } - }); - } + // Check that we are not trying to connect a child to a container. + if (opt.validateGraph) this.validateGraph(graph); // Create a graphlib.Graph that represents the joint.dia.Graph var glGraph = DirectedGraph.toGraphLib(graph, {