(WIP) refactor(@joint/layout-directed-graph): avoid indirect function calls in API

packages/joint-core/docs/src/joint/api/layout/DirectedGraph.html

@@ -145,14 +145,15 @@ <h3 id="layout.DirectedGraph.api">API</h3>
 <table>
     <tr><th>toGraphLib(graph, opt)</th>
         <td>Convert the provided JointJS <code>joint.dia.Graph</code> object to a Graphlib graph object.
-            <pre><code>import { DirectedGraph } from '@joint/layout-directed-graph';
+            <pre><code>import { dia, shapes } from '@joint/core';
+import { DirectedGraph } from '@joint/layout-directed-graph';
 import * as graphlib from '@dagrejs/graphlib';
 
-var graph = new joint.dia.Graph({}, { cellNamespace: joint.shapes });
+const graph = new dia.Graph({}, { cellNamespace: shapes });
 // ... populate the graph with elements connected with links
 
 // Get a Graphlib representation of the graph:
-var glGraph = DirectedGraph.toGraphLib(graph);
+const glGraph = DirectedGraph.toGraphLib(graph);
 
 // Use Graphlib algorithms:
 graphlib.alg.isAcyclic(glGraph); // true if the graph is acyclic</code></pre>
@@ -161,31 +162,48 @@ <h3 id="layout.DirectedGraph.api">API</h3>
     <tr><th>fromGraphLib(glGraph, opt)</th>
         <td>Convert the provided Graphlib graph object to a JointJS <code>joint.dia.Graph</code> object.
             <br/><br/>
-            The <code>opt.importNode</code> and <code>opt.importEdge</code> callbacks are provided with a Graphlib node / edge object, and are expected to return a corresponding JointJS element / link object.
-            <pre><code>import { DirectedGraph } from '@joint/layout-directed-graph';
+            Custom <code>opt.importNode</code> and <code>opt.importEdge</code> callbacks need to be provided in order for this method to work as expected - to return a JointJS Graph matching the structure of the provided Graphlib graph (<code>glGraph</code>). The callbacks are provided with the following attributes:
+            <ul>
+                <li><code>nodeId</code> / <code>edgeObj</code> - a <a href="https://github.com/dagrejs/graphlib/wiki/API-Reference#node-and-edge-representation">Graphlib item ID</a>,</li>
+                <li><code>glGraph</code> - the original Graphlib graph object,</li>
+                <li><code>graph</code> - the JointJS Graph object to be populated (a newly created <code>joint.dia.Graph</code> by default),</li>
+                <li><code>opt</code> - the options object provided to <code>fromGraphLib()</code>.</li>
+            </ul>
+            You can implement your own logic inside the two callbacks to support your use case.
+            <br/><br/>
+            The example below illustrates how Graphlib node/edge labels can be used to store additional information for each item. These labels are accessed in the callbacks as <code>nodeData</code> / <code>edgeData</code>, and the information within is used to specify the position, size, and label text of the Element / Link being created. (Note: In Graphlib, edge source id / edge target id are specified via <code>nodeId</code> of the respective node; we need to set the <code>id</code> of generated Elements as <code>nodeId</code> so that the generated Links can connect the generated Elements.) The callbacks work by adding the generated Element / Link to the <code>graph</code> in the end (e.g. via the <code>graph.addCell()</code> function):
+            <pre><code>import { shapes } from '@joint/core';
+import { DirectedGraph } from '@joint/layout-directed-graph';
 import * as graphlib from 'graphlib';
 
 // Create a graph in Graphlib:
-var glGraph = new graphlib.Graph();
-glGraph.setNode(1);
-glGraph.setNode(2);
-glGraph.setNode(3);
-glGraph.setEdge(1, 2);
-glGraph.setEdge(2, 3);
+const glGraph = new graphlib.Graph();
+glGraph.setNode(1, { x: 50, y: 50, width: 100, height: 50, label: 'A' });
+glGraph.setNode(2, { x: 50, y: 150, width: 100, height: 50, label: 'B' });
+glGraph.setNode(3, { x: 50, y: 250, width: 100, height: 50, label: 'C' });
+glGraph.setEdge(1, 2, { label: 'Hello' });
+glGraph.setEdge(2, 3, { label: 'World!' });
 
 // Get a JointJS representation of the Graphlib graph:
-var graph = DirectedGraph.fromGraphLib(glGraph, {
-    importNode: function(node) {
-        return new joint.shapes.standard.Rectangle({
-            position: { x: node.x, y: node.y },
-            size: { width: node.width, height: node.height }
+const graph = DirectedGraph.fromGraphLib(glGraph, {
+    importNode: (nodeId, glGraph, graph, opt) => {
+        const nodeData = glGraph.node(nodeId);
+        const element = new shapes.standard.Rectangle({
+            id: nodeId,
+            position: { x: nodeData.x, y: nodeData.y },
+            size: { width: nodeData.width, height: nodeData.height },
+            attrs: { label: { text: nodeData.label }}
         });
+        graph.addCell(element);
     },
-    importEdge: function(edge) {
-        return new joint.shapes.standard.Link({
-            source: { id: edge.v },
-            target: { id: edge.w }
+    importEdge: (edgeObj, glGraph, graph, opt) => {
+        const edgeData = glGraph.edge(edgeObj);
+        const link =  new shapes.standard.Link({
+            source: { id: edgeObj.v },
+            target: { id: edgeObj.w },
+            labels: [{ attrs: { text: { text: edgeData.label }}}]
         });
+        graph.addCell(link);
     }
 });</code></pre>
         </td>

packages/joint-layout-directed-graph/DirectedGraph.d.ts

@@ -41,10 +41,14 @@ export namespace DirectedGraph {
         [key: string]: any;
     }
 
+    interface fromGraphLibOptions {
+        graph?: dia.Graph;
+        [key: string]: any;
+    }
+
     export function layout(graph: dia.Graph | dia.Cell[], opt?: LayoutOptions): g.Rect;
 
     export function toGraphLib(graph: dia.Graph, opt?: toGraphLibOptions): any;
 
-    export function fromGraphLib(glGraph: any, opt?: { [key: string]: any }): dia.Graph;
-    export function fromGraphLib(this: dia.Graph, glGraph: any, opt?: { [key: string]: any }): dia.Graph;
+    export function fromGraphLib(glGraph: any, opt?: fromGraphLibOptions): dia.Graph;
 }

packages/joint-layout-directed-graph/DirectedGraph.mjs

@@ -34,28 +34,34 @@ export const DirectedGraph = {
         return edge;
     },
 
-    importElement: function(opt, v, gl) {
+    /**
+     * @private
+     */
+    importElement: function(nodeId, glGraph, graph, opt) {
 
-        var element = this.getCell(v);
-        var glNode = gl.node(v);
+        const element = graph.getCell(nodeId);
+        const nodeData = glGraph.node(nodeId);
 
         if (opt.setPosition) {
-            opt.setPosition(element, glNode);
+            opt.setPosition(element, nodeData);
         } else {
             element.set('position', {
-                x: glNode.x - glNode.width / 2,
-                y: glNode.y - glNode.height / 2
+                x: nodeData.x - (nodeData.width / 2),
+                y: nodeData.y - (nodeData.height / 2)
             });
         }
     },
 
-    importLink: function(opt, edgeObj, gl) {
+    /**
+     * @private
+     */
+    importLink: function(edgeObj, glGraph, graph, opt) {
 
         const SIMPLIFY_THRESHOLD = 0.001;
 
-        const link = this.getCell(edgeObj.name);
-        const glEdge = gl.edge(edgeObj);
-        const points = glEdge.points || [];
+        const link = graph.getCell(edgeObj.name);
+        const edgeData = glGraph.edge(edgeObj);
+        const points = edgeData.points || [];
         const polyline = new g.Polyline(points);
 
         // check the `setLinkVertices` here for backwards compatibility
@@ -68,13 +74,13 @@ export const DirectedGraph = {
                 const polylinePoints = polyline.points.map((point) => (point.toJSON())); // JSON of points after simplification
                 const numPolylinePoints = polylinePoints.length; // number of points after simplification
                 // set simplified polyline points as link vertices
-                // remove first and last polyline points (= source/target sonnectionPoints)
+                // remove first and last polyline points (= source/target connectionPoints)
                 link.set('vertices', polylinePoints.slice(1, numPolylinePoints - 1));
             }
         }
 
-        if (opt.setLabels && ('x' in glEdge) && ('y' in glEdge)) {
-            const labelPosition = { x: glEdge.x, y: glEdge.y };
+        if (opt.setLabels && ('x' in edgeData) && ('y' in edgeData)) {
+            const labelPosition = { x: edgeData.x, y: edgeData.y };
             if (util.isFunction(opt.setLabels)) {
                 opt.setLabels(link, labelPosition, points);
             } else {
@@ -113,11 +119,12 @@ export const DirectedGraph = {
             resizeClusters: true,
             clusterPadding: 10,
             exportElement: this.exportElement,
-            exportLink: this.exportLink
+            exportLink: this.exportLink,
+            importNode: this.importElement,
+            importEdge: this.importLink
         });
 
         // create a graphlib.Graph that represents the joint.dia.Graph
-        // var glGraph = graph.toGraphLib({
         var glGraph = DirectedGraph.toGraphLib(graph, {
             directed: true,
             // We are about to use edge naming feature.
@@ -127,7 +134,7 @@ export const DirectedGraph = {
             setNodeLabel: opt.exportElement,
             setEdgeLabel: opt.exportLink,
             setEdgeName: function(link) {
-                // Graphlib edges have no ids. We use edge name property
+                // Graphlib edges have no ids. We use `edgeObj.name` property
                 // to store and retrieve ids instead.
                 return link.id;
             }
@@ -166,16 +173,15 @@ export const DirectedGraph = {
         graph.startBatch('layout');
 
         DirectedGraph.fromGraphLib(glGraph, {
-            importNode: this.importElement.bind(graph, opt),
-            importEdge: this.importLink.bind(graph, opt)
+            graph,
+            importNode: opt.importNode,
+            importEdge: opt.importEdge,
+            setPosition: opt.setPosition,
+            setVertices: opt.setVertices,
+            setLinkVertices: opt.setLinkVertices, // deprecated
+            setLabels: opt.setLabels
         });
 
-        // // Update the graph.
-        // graph.fromGraphLib(glGraph, {
-        //     importNode: this.importElement.bind(graph, opt),
-        //     importEdge: this.importLink.bind(graph, opt)
-        // });
-
         if (opt.resizeClusters) {
             // Resize and reposition cluster elements (parents of other elements)
             // to fit their children.
@@ -212,17 +218,13 @@ export const DirectedGraph = {
 
         var importNode = opt.importNode || util.noop;
         var importEdge = opt.importEdge || util.noop;
-        var graph = (this instanceof dia.Graph) ? this : new dia.Graph();
+        var graph = opt.graph || new dia.Graph();
 
         // Import all nodes.
-        glGraph.nodes().forEach(function(node) {
-            importNode.call(graph, node, glGraph, graph, opt);
-        });
+        glGraph.nodes().forEach((nodeId) => importNode(nodeId, glGraph, graph, opt));
 
         // Import all edges.
-        glGraph.edges().forEach(function(edge) {
-            importEdge.call(graph, edge, glGraph, graph, opt);
-        });
+        glGraph.edges().forEach((edgeObj) => importEdge(edgeObj, glGraph, graph, opt));
 
         return graph;
     },

packages/joint-layout-directed-graph/test/index.js

@@ -15,6 +15,89 @@ QUnit.module('DirectedGraph', function(hooks) {
 
             assert.equal(typeof DirectedGraph.fromGraphLib, 'function');
         });
+
+        QUnit.test('should correctly convert a graphlib graph into JointJS graph', function(assert) {
+
+            const glGraph = new graphlib.Graph();
+            glGraph.setNode(1, { x: 50, y: 50, width: 100, height: 50, label: 'A' });
+            glGraph.setNode(2, { x: 50, y: 150, width: 100, height: 50, label: 'B' });
+            glGraph.setNode(3, { x: 50, y: 250, width: 100, height: 50, label: 'C' });
+            glGraph.setEdge(1, 2, { label: 'Hello' });
+            glGraph.setEdge(2, 3, { label: 'World!' });
+
+            const graph = DirectedGraph.fromGraphLib(glGraph, {
+                importNode: (nodeId, glGraph, graph, _opt) => {
+                    const nodeData = glGraph.node(nodeId);
+                    const element = new joint.shapes.standard.Rectangle({
+                        id: nodeId,
+                        position: { x: nodeData.x, y: nodeData.y },
+                        size: { width: nodeData.width, height: nodeData.height },
+                        attrs: { label: { text: nodeData.label }}
+                    });
+                    graph.addCell(element);
+                },
+                importEdge: (edgeObj, glGraph, graph, _opt) => {
+                    const edgeData = glGraph.edge(edgeObj);
+                    const link =  new joint.shapes.standard.Link({
+                        source: { id: edgeObj.v },
+                        target: { id: edgeObj.w },
+                        labels: [{ attrs: { text: { text: edgeData.label }}}]
+                    });
+                    graph.addCell(link);
+                }
+            });
+
+            // elements
+            const elements = graph.getElements();
+            assert.equal(elements.length, 3);
+            let id, x, y, width, height, elementLabel;
+
+            (id = elements[0].id);
+            assert.equal(id, '1');
+            ({ x, y } = elements[0].position());
+            assert.deepEqual({ x, y }, { x: 50, y: 50 });
+            ({ width, height} = elements[0].size());
+            assert.deepEqual({ width, height }, {width: 100, height: 50 });
+            (elementLabel = elements[0].attr('label/text'));
+            assert.equal(elementLabel, 'A');
+
+            (id = elements[1].id);
+            assert.equal(id, '2');
+            ({ x, y } = elements[1].position());
+            assert.deepEqual({ x, y }, { x: 50, y: 150 });
+            ({ width, height} = elements[1].size());
+            assert.deepEqual({ width, height }, {width: 100, height: 50 });
+            (elementLabel = elements[1].attr('label/text'));
+            assert.equal(elementLabel, 'B');
+
+            (id = elements[2].id);
+            assert.equal(id, '3');
+            ({ x, y } = elements[2].position());
+            assert.deepEqual({ x, y }, { x: 50, y: 250 });
+            ({ width, height} = elements[2].size());
+            assert.deepEqual({ width, height }, {width: 100, height: 50 });
+            (elementLabel = elements[2].attr('label/text'));
+            assert.equal(elementLabel, 'C');
+
+            // links
+            const links = graph.getLinks();
+            assert.equal(links.length, 2);
+            let source, target, linkLabel;
+
+            (source = links[0].source().id);
+            assert.equal(source, '1');
+            (target = links[0].target().id);
+            assert.equal(target, '2');
+            (linkLabel = links[0].label(0).attrs.text.text);
+            assert.equal(linkLabel, 'Hello');
+
+            (source = links[1].source().id);
+            assert.equal(source, '2');
+            (target = links[1].target().id);
+            assert.equal(target, '3');
+            (linkLabel = links[1].label(0).attrs.text.text);
+            assert.equal(linkLabel, 'World!');
+        });
     });
 
     QUnit.module('toGraphLib(jointGraph[, opt])', function(hooks) {
@@ -187,7 +270,6 @@ QUnit.module('DirectedGraph', function(hooks) {
             assert.deepEqual({ x, y }, { x: 5, y: 210 });
         });
 
-
         QUnit.test('should return a rectangle representing the graph bounding box', function(assert) {
 
             var bbox;