From 6a8cceb2605aa93acb9dcd8ceb95bc6b29fd256d Mon Sep 17 00:00:00 2001
From: danielcaldas <caldasjdaniel@gmail.com>
Date: Fri, 28 Sep 2018 21:58:10 +0200
Subject: [PATCH 1/8] Add deprecation note on componentWillReceiveProps

---
 src/components/graph/Graph.jsx | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/components/graph/Graph.jsx b/src/components/graph/Graph.jsx
index de85007d8..dcd8cab52 100644
--- a/src/components/graph/Graph.jsx
+++ b/src/components/graph/Graph.jsx
@@ -295,6 +295,16 @@ export default class Graph extends React.Component {
         this.state = graphHelper.initializeGraphState(this.props, this.state);
     }
 
+    /**
+     * @deprecated
+     * `componentWillReceiveProps` has a replacement method in react v16.3 onwards.
+     * that is getDerivedStateFromProps.
+     * But one needs to be aware that if an anti pattern of `componentWillReceiveProps` is
+     * in place for this implementation the migration might not be that easy.
+     * See {@link https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html}.
+     * @param {Object} nextProps - props.
+     * @returns {undefined}
+     */
     componentWillReceiveProps(nextProps) {
         const newGraphElements =
             nextProps.data.nodes.length !== this.state.nodesInputSnapshot.length ||

From c4478a88ed7a92a9f1b447159f8a32a216278f48 Mon Sep 17 00:00:00 2001
From: danielcaldas <caldasjdaniel@gmail.com>
Date: Fri, 28 Sep 2018 22:04:59 +0200
Subject: [PATCH 2/8] Add antiPick method to utils

---
 src/utils.js       | 13 +++++++++++++
 test/utils.test.js | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/src/utils.js b/src/utils.js
index f198b8618..8912ca4cd 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -127,6 +127,18 @@ function pick(o, props = []) {
     }, {});
 }
 
+/**
+ * Picks all props except the ones passed in the props array.
+ * @param {Object} o - the object to pick props from.
+ * @param {Array.<string>} props - list of props that we DON'T want to pick from o.
+ * @returns {Object} the object resultant from the anti picking operation.
+ */
+function antiPick(o, props = []) {
+    const wanted = Object.keys(o).filter(k => !props.includes(k));
+
+    return pick(o, wanted);
+}
+
 /**
  * Helper function for customized error logging.
  * @param  {string} component - the name of the component where the error is to be thrown.
@@ -145,5 +157,6 @@ export default {
     isObjectEmpty,
     merge,
     pick,
+    antiPick,
     throwErr
 };
diff --git a/test/utils.test.js b/test/utils.test.js
index c62b16066..a8a417c4d 100644
--- a/test/utils.test.js
+++ b/test/utils.test.js
@@ -216,6 +216,38 @@ describe('Utils', () => {
         });
     });
 
+    describe('#antiPick', () => {
+        let that = {};
+
+        beforeEach(() => {
+            that.o = {
+                a: 1,
+                b: {
+                    j: {
+                        k: null,
+                        l: 'test'
+                    }
+                },
+                c: 'test',
+                f: 0
+            };
+        });
+
+        test('should pick given props and return expected object', () => {
+            const result = utils.antiPick(that.o, ['a', 'f', 'not a o prop']);
+
+            expect(result).toEqual({
+                b: {
+                    j: {
+                        k: null,
+                        l: 'test'
+                    }
+                },
+                c: 'test'
+            });
+        });
+    });
+
     describe('#throwErr', () => {
         test('should throw error', () => {
             const c = 'some component';

From 48f29d00a7d1a489062af5b44c388d58e909e01f Mon Sep 17 00:00:00 2001
From: danielcaldas <caldasjdaniel@gmail.com>
Date: Fri, 28 Sep 2018 22:19:33 +0200
Subject: [PATCH 3/8] Add check:light task in package.json

---
 package.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package.json b/package.json
index 7ed364ec0..70959c66c 100644
--- a/package.json
+++ b/package.json
@@ -6,6 +6,7 @@
     "license": "MIT",
     "scripts": {
         "check": "npm run docs:lint && npm run lint && npm run test && npm run functional",
+        "check:light": "npm run lint && npm run test",
         "dev": "NODE_ENV=dev webpack-dev-server --mode=development --content-base sandbox --config webpack.config.js --inline --hot --port 3002",
         "dist:rd3g": "rm -rf dist/ && webpack --config webpack.config.dist.js -p --display-modules --optimize-minimize",
         "dist:sandbox": "webpack --config webpack.config.js -p",

From 8cb868d6c4a101a96ecc2fc359a4eb74a18545a6 Mon Sep 17 00:00:00 2001
From: danielcaldas <caldasjdaniel@gmail.com>
Date: Sat, 29 Sep 2018 14:03:38 +0200
Subject: [PATCH 4/8] Initialize links in initializeGraphState

---
 src/components/graph/graph.helper.js      | 9 ++++-----
 test/component/graph/graph.helper.test.js | 2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/components/graph/graph.helper.js b/src/components/graph/graph.helper.js
index bb446c177..004db738b 100644
--- a/src/components/graph/graph.helper.js
+++ b/src/components/graph/graph.helper.js
@@ -340,7 +340,7 @@ function initializeGraphState({ data, id, config }, state) {
     const nodesInputSnapshot = data.nodes.map(n => Object.assign({}, n));
     const linksInputSnapshot = data.links.map(l => Object.assign({}, l));
 
-    if (state && state.nodes && state.links) {
+    if (state && state.nodes) {
         // absorb existent positioning
         graph = {
             nodes: data.nodes.map(
@@ -349,17 +349,15 @@ function initializeGraphState({ data, id, config }, state) {
                         ? Object.assign({}, n, utils.pick(state.nodes[n.id], NODE_PROPS_WHITELIST))
                         : Object.assign({}, n)
             ),
-            links: {}
+            links: (state && state.d3Links) || data.links.map(l => Object.assign({}, l))
         };
     } else {
         graph = {
             nodes: data.nodes.map(n => Object.assign({}, n)),
-            links: {}
+            links: data.links.map(l => Object.assign({}, l))
         };
     }
 
-    graph.links = data.links.map(l => Object.assign({}, l));
-
     let newConfig = Object.assign({}, utils.merge(DEFAULT_CONFIG, config || {}));
     let nodes = _initializeNodes(graph.nodes);
     let links = _initializeLinks(graph.links); // matrix of graph connections
@@ -513,6 +511,7 @@ function getNodeCardinality(nodeId, linksMatrix) {
 }
 
 export {
+    NODE_PROPS_WHITELIST,
     buildLinkProps,
     buildNodeProps,
     disconnectLeafNodeConnections,
diff --git a/test/component/graph/graph.helper.test.js b/test/component/graph/graph.helper.test.js
index dc5b124b2..409c4e8e7 100644
--- a/test/component/graph/graph.helper.test.js
+++ b/test/component/graph/graph.helper.test.js
@@ -245,7 +245,7 @@ describe('Graph Helper', () => {
                             A: { x: 20, y: 40 },
                             B: { x: 40, y: 60 }
                         },
-                        links: 'links',
+                        links: [],
                         nodeIndexMapping: 'nodeIndexMapping'
                     };
 

From c577ed1d6ab166e1f2a5c0c6b6e86942a6f7a8e4 Mon Sep 17 00:00:00 2001
From: danielcaldas <caldasjdaniel@gmail.com>
Date: Sat, 29 Sep 2018 18:04:41 +0200
Subject: [PATCH 5/8] Implement checkForGraphElementsChanges in graph.helper

---
 src/components/graph/Graph.jsx       | 16 +++++-------
 src/components/graph/graph.helper.js | 37 ++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/src/components/graph/Graph.jsx b/src/components/graph/Graph.jsx
index dcd8cab52..d2642c1b5 100644
--- a/src/components/graph/Graph.jsx
+++ b/src/components/graph/Graph.jsx
@@ -306,15 +306,11 @@ export default class Graph extends React.Component {
      * @returns {undefined}
      */
     componentWillReceiveProps(nextProps) {
-        const newGraphElements =
-            nextProps.data.nodes.length !== this.state.nodesInputSnapshot.length ||
-            nextProps.data.links.length !== this.state.linksInputSnapshot.length ||
-            !utils.isDeepEqual(nextProps.data, {
-                nodes: this.state.nodesInputSnapshot,
-                links: this.state.linksInputSnapshot
-            });
-        const state = newGraphElements ? graphHelper.initializeGraphState(nextProps, this.state) : this.state;
-
+        const { graphElementsUpdated, newGraphElements } = graphHelper.checkForGraphElementsChanges(
+            nextProps,
+            this.state
+        );
+        const state = graphElementsUpdated ? graphHelper.initializeGraphState(nextProps, this.state) : this.state;
         const newConfig = nextProps.config || {};
         const configUpdated =
             newConfig && !utils.isObjectEmpty(newConfig) && !utils.isDeepEqual(newConfig, this.state.config);
@@ -328,8 +324,8 @@ export default class Graph extends React.Component {
         this.setState({
             ...state,
             config,
-            newGraphElements,
             configUpdated,
+            newGraphElements,
             transform
         });
     }
diff --git a/src/components/graph/graph.helper.js b/src/components/graph/graph.helper.js
index 004db738b..9aa9ab8bb 100644
--- a/src/components/graph/graph.helper.js
+++ b/src/components/graph/graph.helper.js
@@ -322,6 +322,42 @@ function buildNodeProps(node, config, nodeCallbacks = {}, highlightedNode, highl
     };
 }
 
+// list of properties that are of no interest when it comes to nodes and links comparison
+const NODE_PROPERTIES_DISCARD_TO_COMPARE = ['x', 'y', 'vx', 'vy', 'index'];
+
+/**
+ * This function checks for graph elements (nodes and links) changes, in two different
+ * levels of significance, updated elements (whether some property has changed in some
+ * node or link) and new elements (whether some new elements or added/removed from the graph).
+ * @param {Object} nextProps - nextProps that graph will receive.
+ * @param {Object} currentState - the current state of the graph.
+ * @returns {Object.<string, boolean>} returns object containing update check flags:
+ * - newGraphElements - flag that indicates whether new graph elements were added.
+ * - graphElementsUpdated - flag that indicates whether some graph elements have
+ * updated (some property that is not in NODE_PROPERTIES_DISCARD_TO_COMPARE was added to
+ * some node or link or was updated).
+ */
+function checkForGraphElementsChanges(nextProps, currentState) {
+    const nextNodes = nextProps.data.nodes.map(n => utils.antiPick(n, NODE_PROPERTIES_DISCARD_TO_COMPARE));
+    const nextLinks = nextProps.data.links;
+    const stateD3Nodes = currentState.d3Nodes.map(n => utils.antiPick(n, NODE_PROPERTIES_DISCARD_TO_COMPARE));
+    const stateD3Links = currentState.d3Links.map(l => ({
+        // FIXME: solve this source data inconsistency later
+        source: l.source.id || l.source,
+        target: l.target.id || l.target
+    }));
+    const graphElementsUpdated = !(
+        utils.isDeepEqual(nextNodes, stateD3Nodes) && utils.isDeepEqual(nextLinks, stateD3Links)
+    );
+    const newGraphElements =
+        nextNodes.length !== stateD3Nodes.length ||
+        nextLinks.length !== stateD3Links.length ||
+        !utils.isDeepEqual(nextNodes.map(({ id }) => ({ id })), stateD3Nodes.map(({ id }) => ({ id }))) ||
+        !utils.isDeepEqual(nextLinks, stateD3Links.map(({ source, target }) => ({ source, target })));
+
+    return { graphElementsUpdated, newGraphElements };
+}
+
 /**
  * Encapsulates common procedures to initialize graph.
  * @param {Object} props - Graph component props, object that holds data, id and config.
@@ -514,6 +550,7 @@ export {
     NODE_PROPS_WHITELIST,
     buildLinkProps,
     buildNodeProps,
+    checkForGraphElementsChanges,
     disconnectLeafNodeConnections,
     getLeafNodeConnections,
     getNodeCardinality,

From a84c65f5350b5614756d583e01df4989143c637a Mon Sep 17 00:00:00 2001
From: danielcaldas <caldasjdaniel@gmail.com>
Date: Sat, 29 Sep 2018 18:24:02 +0200
Subject: [PATCH 6/8] e2e test collapse improvements

---
 cypress/integration/graph.e2e.js | 15 +++++++++++++++
 cypress/integration/link.e2e.js  |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/cypress/integration/graph.e2e.js b/cypress/integration/graph.e2e.js
index 06271d84f..6ff834461 100644
--- a/cypress/integration/graph.e2e.js
+++ b/cypress/integration/graph.e2e.js
@@ -125,6 +125,8 @@ describe('[rd3g-graph] graph tests', function() {
 
             this.node1PO = new NodePO(1);
             this.node2PO = new NodePO(2);
+            this.node3PO = new NodePO(3);
+            this.node4PO = new NodePO(4);
             this.link12PO = new LinkPO(0);
         });
 
@@ -141,6 +143,19 @@ describe('[rd3g-graph] graph tests', function() {
             // Check the leaf node & link is no longer visible
             this.node2PO.getPath().should('not.be.visible');
             line.should('not.be.visible');
+
+            // Check if other nodes and links are still visible
+            this.node1PO.getPath().should('be.visible');
+            this.node3PO.getPath().should('be.visible');
+            this.node4PO.getPath().should('be.visible');
+
+            const link13PO = new LinkPO(1);
+            const link14PO = new LinkPO(2);
+            const link34PO = new LinkPO(3);
+
+            link13PO.getLine().should('be.visible');
+            link14PO.getLine().should('be.visible');
+            link34PO.getLine().should('be.visible');
         });
     });
 });
diff --git a/cypress/integration/link.e2e.js b/cypress/integration/link.e2e.js
index ab913a575..c30eea105 100644
--- a/cypress/integration/link.e2e.js
+++ b/cypress/integration/link.e2e.js
@@ -6,7 +6,7 @@ const NodePO = require('../page-objects/node.po');
 const SandboxPO = require('../page-objects/sandbox.po');
 let nodes = require('./../../sandbox/data/small/small.data').nodes.map(({ id }) => id);
 
-describe('[rd3g-graph] link tests', function() {
+describe('[rd3g-link] link tests', function() {
     before(function() {
         this.sandboxPO = new SandboxPO();
         // visit sandbox

From 19a77598b6fb08d4e87b37c3f2e5015497beb71f Mon Sep 17 00:00:00 2001
From: danielcaldas <caldasjdaniel@gmail.com>
Date: Sat, 29 Sep 2018 18:52:12 +0200
Subject: [PATCH 7/8] Small check on number of links graph e2e

---
 cypress/integration/graph.e2e.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cypress/integration/graph.e2e.js b/cypress/integration/graph.e2e.js
index 6ff834461..966b06755 100644
--- a/cypress/integration/graph.e2e.js
+++ b/cypress/integration/graph.e2e.js
@@ -65,6 +65,7 @@ describe('[rd3g-graph] graph tests', function() {
                 it('nodes props modifications should be reflected in the graph', function() {
                     cy.get('text').should('have.length', 14);
                     cy.get('path[class="link"]').should('be.visible');
+                    cy.get('path[class="link"]').should('have.length', 23);
 
                     this.sandboxPO.addNode();
                     this.sandboxPO.addNode();
@@ -73,6 +74,9 @@ describe('[rd3g-graph] graph tests', function() {
 
                     cy.get('text').should('have.length', 18);
 
+                    // should now have more than 23 links
+                    cy.get('path[class="link"]').should('not.have.length', 23);
+
                     // click (+) add prop to 1st node
                     this.sandboxPO.addJsonTreeFirstNodeProp();
                     // prop name be color

From 2164eed47d0acc2a121710184af4f685875dd1be Mon Sep 17 00:00:00 2001
From: danielcaldas <caldasjdaniel@gmail.com>
Date: Sun, 30 Sep 2018 00:07:00 +0200
Subject: [PATCH 8/8] Proper link initialization by mapping input to d3 link

---
 src/components/graph/graph.helper.js      | 40 ++++++++++--
 test/component/graph/graph.helper.test.js | 74 ++++++++++++++++-------
 2 files changed, 87 insertions(+), 27 deletions(-)

diff --git a/src/components/graph/graph.helper.js b/src/components/graph/graph.helper.js
index 9aa9ab8bb..d1989939d 100644
--- a/src/components/graph/graph.helper.js
+++ b/src/components/graph/graph.helper.js
@@ -1,3 +1,4 @@
+/*eslint-disable max-lines*/
 /**
  * @module Graph/helper
  * @description
@@ -148,6 +149,39 @@ function _initializeNodes(graphNodes) {
     return nodes;
 }
 
+/**
+ * Maps an input link (with format `{ source: 'sourceId', target: 'targetId' }`) to a d3Link
+ * (with format `{ source: { id: 'sourceId' }, target: { id: 'targetId' } }`). If d3Link with
+ * given index exists already that same d3Link is returned.
+ * @param {Object} link - input link.
+ * @param {number} index - index of the input link.
+ * @param {Array.<Object>} d3Links - all d3Links.
+ * @returns {Object} a d3Link.
+ */
+function _mapDataLinkToD3Link(link, index, d3Links = []) {
+    const d3Link = d3Links[index];
+
+    if (d3Link) {
+        return d3Link;
+    }
+
+    const highlighted = false;
+    const source = {
+        id: link.source,
+        highlighted
+    };
+    const target = {
+        id: link.target,
+        highlighted
+    };
+
+    return {
+        index,
+        source,
+        target
+    };
+}
+
 /**
  * Some integrity validations on links and nodes structure. If some validation fails the function will
  * throw an error.
@@ -369,15 +403,13 @@ function checkForGraphElementsChanges(nextProps, currentState) {
  * @memberof Graph/helper
  */
 function initializeGraphState({ data, id, config }, state) {
-    let graph;
-
     _validateGraphData(data);
 
+    let graph;
     const nodesInputSnapshot = data.nodes.map(n => Object.assign({}, n));
     const linksInputSnapshot = data.links.map(l => Object.assign({}, l));
 
     if (state && state.nodes) {
-        // absorb existent positioning
         graph = {
             nodes: data.nodes.map(
                 n =>
@@ -385,7 +417,7 @@ function initializeGraphState({ data, id, config }, state) {
                         ? Object.assign({}, n, utils.pick(state.nodes[n.id], NODE_PROPS_WHITELIST))
                         : Object.assign({}, n)
             ),
-            links: (state && state.d3Links) || data.links.map(l => Object.assign({}, l))
+            links: data.links.map((l, index) => _mapDataLinkToD3Link(l, index, state && state.d3Links))
         };
     } else {
         graph = {
diff --git a/test/component/graph/graph.helper.test.js b/test/component/graph/graph.helper.test.js
index 409c4e8e7..0f7ce953a 100644
--- a/test/component/graph/graph.helper.test.js
+++ b/test/component/graph/graph.helper.test.js
@@ -235,7 +235,7 @@ describe('Graph Helper', () => {
             });
 
             describe('and received state was already initialized', () => {
-                test('should create graph structure absorbing stored nodes behavior in state obj', () => {
+                test('should create graph structure absorbing stored nodes and links behavior', () => {
                     const data = {
                         nodes: [{ id: 'A' }, { id: 'B' }, { id: 'C' }],
                         links: [{ source: 'A', target: 'B' }, { source: 'C', target: 'A' }]
@@ -273,12 +273,26 @@ describe('Graph Helper', () => {
                     ]);
                     expect(newState.d3Links).toEqual([
                         {
-                            source: 'A',
-                            target: 'B'
+                            index: 0,
+                            source: {
+                                highlighted: false,
+                                id: 'A'
+                            },
+                            target: {
+                                highlighted: false,
+                                id: 'B'
+                            }
                         },
                         {
-                            source: 'C',
-                            target: 'A'
+                            index: 1,
+                            source: {
+                                highlighted: false,
+                                id: 'C'
+                            },
+                            target: {
+                                highlighted: false,
+                                id: 'A'
+                            }
                         }
                     ]);
                 });
@@ -364,12 +378,26 @@ describe('Graph Helper', () => {
                     configUpdated: false,
                     d3Links: [
                         {
-                            source: 'A',
-                            target: 'B'
+                            index: 0,
+                            source: {
+                                highlighted: false,
+                                id: 'A'
+                            },
+                            target: {
+                                highlighted: false,
+                                id: 'B'
+                            }
                         },
                         {
-                            source: 'C',
-                            target: 'A'
+                            index: 1,
+                            source: {
+                                highlighted: false,
+                                id: 'C'
+                            },
+                            target: {
+                                highlighted: false,
+                                id: 'A'
+                            }
                         }
                     ],
                     d3Nodes: [
@@ -406,6 +434,16 @@ describe('Graph Helper', () => {
                             A: 1
                         }
                     },
+                    linksInputSnapshot: [
+                        {
+                            source: 'A',
+                            target: 'B'
+                        },
+                        {
+                            source: 'C',
+                            target: 'A'
+                        }
+                    ],
                     newGraphElements: false,
                     nodes: {
                         A: {
@@ -427,10 +465,6 @@ describe('Graph Helper', () => {
                             y: 0
                         }
                     },
-                    simulation: {
-                        force: forceStub
-                    },
-                    transform: 1,
                     nodesInputSnapshot: [
                         {
                             id: 'A'
@@ -442,16 +476,10 @@ describe('Graph Helper', () => {
                             id: 'C'
                         }
                     ],
-                    linksInputSnapshot: [
-                        {
-                            source: 'A',
-                            target: 'B'
-                        },
-                        {
-                            source: 'C',
-                            target: 'A'
-                        }
-                    ]
+                    simulation: {
+                        force: forceStub
+                    },
+                    transform: 1
                 });
             });
         });