Skip to content

Commit

Permalink
separate try-catch block for performance in old V8
Browse files Browse the repository at this point in the history
  • Loading branch information
zbynekstara committed Jan 6, 2025
1 parent 7564b92 commit 33b3875
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 10 deletions.
23 changes: 13 additions & 10 deletions packages/joint-layout-directed-graph/DirectedGraph.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
Expand Down
41 changes: 41 additions & 0 deletions packages/joint-layout-directed-graph/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
});
});

0 comments on commit 33b3875

Please sign in to comment.