separate try-catch block for performance in old V8

clientIO · zbynekstara · Dec 18, 2024 · Jan 6, 2025 · Jan 6, 2025 · Jan 7, 2025
commit 0e1d3abb6e7b4fb1e58d53f2f8f4482cb0987b4a
diff --git a/packages/joint-layout-directed-graph/DirectedGraph.mjs b/packages/joint-layout-directed-graph/DirectedGraph.mjs
@@ -105,6 +105,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;
@@ -180,20 +191,12 @@ export const DirectedGraph = {
         }
 
         // Executes the layout.
-        try {
-            dagreUtil.layout(glGraph, {
-                debugTiming: !!opt.debugTiming,
-                disableOptimalOrderHeuristic: !!opt.disableOptimalOrderHeuristic,
-                customOrder,
-            });
-        } 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,
+            disableOptimalOrderHeuristic: !!opt.disableOptimalOrderHeuristic,
+            customOrder,
+        });
 
         // 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
@@ -583,5 +583,46 @@ QUnit.module('DirectedGraph', function(hooks) {
                 }
             });
         });
+
+        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);
+        })
     });
 });