Skip to content

Commit

Permalink
perf(autoHideContainer): stop re-creating React components
Browse files Browse the repository at this point in the history
The previous strategy for autoHideContainer was to either return an
empty div or the wrapped element.

So when the widget cycle was like this: init => hide (no results) =>
show (results) then we would re-create instances of React components because of a
different render() output in autoHideContainer.

This I discovered it while going to do #835: when the widgets would
hide => show we would lose the wrapped React component state resulting
in lost collaspable state.

Not only this commit will permit doing #835 easily but it also somehow
enhance performance since I also added a shouldComponentUpdate
strategy.

Added more precise tests on this decorator/HOC.
  • Loading branch information
vvo committed Feb 17, 2016
1 parent 77586cb commit 8c89862
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 21 deletions.
78 changes: 61 additions & 17 deletions src/decorators/__tests__/autoHideContainer-test.js
Original file line number Diff line number Diff line change
@@ -1,36 +1,80 @@
/* eslint-env mocha */

import React from 'react';
import ReactDOM from 'react-dom';
import expect from 'expect';
import TestUtils from 'react-addons-test-utils';
import TestComponent from './TestComponent';
import autoHideContainer from '../autoHideContainer';
import jsdom from 'jsdom-global';
import sinon from 'sinon';

import expectJSX from 'expect-jsx';
expect.extend(expectJSX);

describe('autoHideContainer', () => {
let renderer;
beforeEach(function() {this.jsdom = jsdom();});
afterEach(function() {this.jsdom();});

beforeEach(() => {
let {createRenderer} = TestUtils;
renderer = createRenderer();
});
let props = {};

it('should render autoHideContainer(<TestComponent />)', () => {
let out = render();
expect(out).toEqualJSX(<TestComponent />);
let {createRenderer} = TestUtils;
let renderer = createRenderer();
props.hello = 'son';
let AutoHide = autoHideContainer(TestComponent);
renderer.render(<AutoHide {...props} />);
let out = renderer.getRenderOutput();
expect(out).toEqualJSX(<TestComponent hello="son" />);
});

it('should not render autoHideContainer(<TestComponent />)', () => {
let out = render({shouldAutoHideContainer: true});
expect(out).toEqualJSX(<div />);
});
context('props.shouldAutoHideContainer', () => {
let AutoHide;
let component;
let container;

function render(props = {}) {
let AutoHide = autoHideContainer(TestComponent);
renderer.render(<AutoHide {...props} />);
return renderer.getRenderOutput();
}
});
beforeEach(() => {
AutoHide = autoHideContainer(TestComponent);
container = document.createElement('div');
props = {hello: 'mom'};
component = ReactDOM.render(<AutoHide {...props} />, container);
});

it('creates a component', () => expect(component).toExist());

it('shows the container at first', () => {
expect(container.style.display).toNotEqual('none');
});

context('when set to true', () => {
beforeEach(() => {
sinon.spy(component, 'render');
props.shouldAutoHideContainer = true;
ReactDOM.render(<AutoHide {...props} />, container);
});

it('hides the container', () => {
expect(container.style.display).toEqual('none');
});

it('does not call component.render()', () => {
expect(component.render.called).toBe(false);
});

context('when set back to false', () => {
beforeEach(() => {
props.shouldAutoHideContainer = false;
ReactDOM.render(<AutoHide {...props} />, container);
});

it('shows the container', () => {
expect(container.style.display).toNotEqual('none');
});

it('calls component.render()', () => {
expect(component.render.calledOnce).toBe(true);
});
});
});
});
});
12 changes: 8 additions & 4 deletions src/decorators/autoHideContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,23 @@ function autoHideContainer(ComposedComponent) {
}

componentWillReceiveProps(nextProps) {
if (this.props.shouldAutoHideContainer === nextProps.shouldAutoHideContainer) {
return;
}

this._hideOrShowContainer(nextProps);
}

shouldComponentUpdate(nextProps) {
return nextProps.shouldAutoHideContainer === false;
}

_hideOrShowContainer(props) {
let container = ReactDOM.findDOMNode(this).parentNode;
container.style.display = (props.shouldAutoHideContainer === true) ? 'none' : '';
}

render() {
if (this.props.shouldAutoHideContainer === true) {
return <div />;
}

return <ComposedComponent {...this.props} />;
}
}
Expand Down

0 comments on commit 8c89862

Please sign in to comment.