react-transition-group is no longer used when transitions are disabled (#92)

* react-transition-group is no longer used when transitions are disabled, resulting in perf boost (#91)

* eliminated unnecessary state changes in NodeWrapper

* Ensures zoom listeners are rebound if NodeWrapper switched DOM nodes

Author: sspanak

src/Tree/NodeWrapper.js

@@ -0,0 +1,52 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { TransitionGroup } from 'react-transition-group';
+
+export default class NodeWrapper extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {
+      enableTransitions: props.transitionDuration > 0,
+    };
+  }
+
+  componentWillReceiveProps(nextProps) {
+    if (nextProps.transitionDuration !== this.props.transitionDuration) {
+      this.setState({
+        enableTransitions: nextProps.transitionDuration > 0,
+      });
+    }
+  }
+
+  render() {
+    if (this.state.enableTransitions) {
+      return (
+        <TransitionGroup
+          component={this.props.component}
+          className={this.props.className}
+          transform={this.props.transform}
+        >
+          {this.props.children}
+        </TransitionGroup>
+      );
+    }
+
+    return (
+      <g className={this.props.className} transform={this.props.transform}>
+        {this.props.children}
+      </g>
+    );
+  }
+}
+
+NodeWrapper.defaultProps = {
+  component: 'g',
+};
+
+NodeWrapper.propTypes = {
+  transitionDuration: PropTypes.number.isRequired,
+  component: PropTypes.string,
+  className: PropTypes.string.isRequired,
+  transform: PropTypes.string.isRequired,
+  children: PropTypes.array.isRequired,
+};

src/Tree/index.js

@@ -1,11 +1,11 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { TransitionGroup } from 'react-transition-group';
 import { layout, select, behavior, event } from 'd3';
 import clone from 'clone';
 import deepEqual from 'deep-equal';
 import uuid from 'uuid';
 
+import NodeWrapper from './NodeWrapper';
 import Node from '../Node';
 import Link from '../Link';
 import './style.css';
@@ -41,7 +41,12 @@ export default class Tree extends React.Component {
     this.internalState.initialRender = false;
   }
 
-  componentDidUpdate() {
+  componentDidUpdate(prevProps) {
+    // Rebind zoom listeners to new DOM nodes in case NodeWrapper switched <TransitionGroup> <-> <g>
+    if (prevProps.transitionDuration !== this.props.transitionDuration) {
+      this.bindZoomListener(this.props);
+    }
+
     if (typeof this.props.onUpdate === 'function') {
       this.props.onUpdate({
         node: this.internalState.targetNode ? clone(this.internalState.targetNode) : null,
@@ -366,7 +371,8 @@ export default class Tree extends React.Component {
     return (
       <div className={`rd3t-tree-container ${zoomable ? 'rd3t-grabbable' : undefined}`}>
         <svg className="rd3t-svg" width="100%" height="100%">
-          <TransitionGroup
+          <NodeWrapper
+            transitionDuration={transitionDuration}
             component="g"
             className="rd3t-g"
             transform={`translate(${translate.x},${translate.y}) scale(${scale})`}
@@ -403,7 +409,7 @@ export default class Tree extends React.Component {
                 styles={styles.nodes}
               />
             ))}
-          </TransitionGroup>
+          </NodeWrapper>
         </svg>
       </div>
     );

src/Tree/tests/NodeWrapper.test.js

@@ -0,0 +1,55 @@
+import React from 'react';
+import { shallow, mount } from 'enzyme';
+import { TransitionGroup } from 'react-transition-group';
+
+import NodeWrapper from '../NodeWrapper';
+
+describe('<NodeWrapper />', () => {
+  jest.spyOn(NodeWrapper.prototype, 'setState');
+
+  // Clear method spies on prototype after each test
+  afterEach(() => jest.clearAllMocks());
+
+  it('Renders a <g> when transitions are disabled', () => {
+    const fixture = {
+      transform: 't',
+      className: 'cls',
+      transitionDuration: 0,
+    };
+
+    const renderedComponent = shallow(<NodeWrapper {...fixture}>{[]}</NodeWrapper>);
+    expect(renderedComponent.find('g').prop('transform')).toContain(fixture.transform);
+    expect(renderedComponent.find('g').prop('className')).toContain(fixture.className);
+  });
+
+  it('Renders a <TransitionGroup> when transitions are enabled', () => {
+    const fixture = {
+      component: 'g',
+      transform: 't',
+      className: 'cls',
+      transitionDuration: 10,
+    };
+
+    const renderedComponent = shallow(<NodeWrapper {...fixture}>{[]}</NodeWrapper>);
+    expect(renderedComponent.find(TransitionGroup).prop('transform')).toContain(fixture.transform);
+    expect(renderedComponent.find(TransitionGroup).prop('className')).toContain(fixture.className);
+    expect(renderedComponent.find(TransitionGroup).prop('component')).toContain(fixture.component);
+  });
+
+  it('does not do useless state updates unless transitionDuration has changed', () => {
+    const fixture = {
+      transform: 't',
+      className: 'cls',
+      transitionDuration: 1,
+    };
+
+    const renderedComponent = mount(<NodeWrapper {...fixture}>{[]}</NodeWrapper>);
+    renderedComponent.setProps(fixture);
+    expect(renderedComponent.instance().setState).toHaveBeenCalledTimes(0);
+    renderedComponent.setProps({
+      ...fixture,
+      transitionDuration: 2,
+    });
+    expect(renderedComponent.instance().setState).toHaveBeenCalledTimes(1);
+  });
+});

src/Tree/tests/index.test.js

@@ -1,8 +1,8 @@
 import React from 'react';
-import { TransitionGroup } from 'react-transition-group';
 import { shallow, mount } from 'enzyme';
 import { render } from 'react-dom';
 
+import NodeWrapper from '../NodeWrapper';
 import Node from '../../Node';
 import Link from '../../Link';
 import Tree from '../index';
@@ -109,7 +109,7 @@ describe('<Tree />', () => {
       const fixture = { x: 123, y: 321 };
       const expected = `translate(${fixture.x},${fixture.y})`;
       const renderedComponent = shallow(<Tree data={mockData} translate={fixture} />);
-      expect(renderedComponent.find(TransitionGroup).prop('transform')).toContain(expected);
+      expect(renderedComponent.find(NodeWrapper).prop('transform')).toContain(expected);
     });
   });
 
@@ -216,26 +216,26 @@ describe('<Tree />', () => {
       const zoomLevel = 0.3;
       const expected = `scale(${zoomLevel})`;
       const renderedComponent = shallow(<Tree data={mockData} zoom={zoomLevel} />);
-      expect(renderedComponent.find(TransitionGroup).prop('transform')).toContain(expected);
+      expect(renderedComponent.find(NodeWrapper).prop('transform')).toContain(expected);
     });
 
     it('applies default zoom level when `zoom` is not specified', () => {
       const renderedComponent = shallow(<Tree data={mockData} />);
-      expect(renderedComponent.find(TransitionGroup).prop('transform')).toContain(`scale(1)`);
+      expect(renderedComponent.find(NodeWrapper).prop('transform')).toContain(`scale(1)`);
     });
 
-    it('respects `scaleExtent` constraints on initital display', () => {
+    it('respects `scaleExtent` constraints on initial display', () => {
       const scaleExtent = { min: 0.2, max: 0.8 };
 
       let renderedComponent = shallow(
         <Tree data={mockData} scaleExtent={scaleExtent} zoom={0.9} />,
       );
-      expect(renderedComponent.find(TransitionGroup).prop('transform')).toContain(
+      expect(renderedComponent.find(NodeWrapper).prop('transform')).toContain(
         `scale(${scaleExtent.max})`,
       );
 
       renderedComponent = shallow(<Tree data={mockData} scaleExtent={scaleExtent} zoom={0.1} />);
-      expect(renderedComponent.find(TransitionGroup).prop('transform')).toContain(
+      expect(renderedComponent.find(NodeWrapper).prop('transform')).toContain(
         `scale(${scaleExtent.min})`,
       );
     });
@@ -253,6 +253,13 @@ describe('<Tree />', () => {
       zoomProps.forEach(nextProps => renderedComponent.setProps(nextProps));
       expect(renderedComponent.instance().bindZoomListener).toHaveBeenCalledTimes(4);
     });
+
+    it('rebinds on `props.transitionDuration` change to handle switched DOM nodes from NodeWrapper', () => {
+      const renderedComponent = mount(<Tree data={mockData} />);
+      expect(renderedComponent.instance().bindZoomListener).toHaveBeenCalledTimes(1);
+      renderedComponent.setProps({ transitionDuration: 0 });
+      expect(renderedComponent.instance().bindZoomListener).toHaveBeenCalledTimes(2);
+    });
   });
 
   describe('onClick', () => {