Skip to content

Commit 8c89862

Browse files
author
vvo
committed
perf(autoHideContainer): stop re-creating React components
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.
1 parent 77586cb commit 8c89862

File tree

2 files changed

+69
-21
lines changed

2 files changed

+69
-21
lines changed
Lines changed: 61 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,80 @@
11
/* eslint-env mocha */
22

33
import React from 'react';
4+
import ReactDOM from 'react-dom';
45
import expect from 'expect';
56
import TestUtils from 'react-addons-test-utils';
67
import TestComponent from './TestComponent';
78
import autoHideContainer from '../autoHideContainer';
9+
import jsdom from 'jsdom-global';
10+
import sinon from 'sinon';
811

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

1215
describe('autoHideContainer', () => {
13-
let renderer;
16+
beforeEach(function() {this.jsdom = jsdom();});
17+
afterEach(function() {this.jsdom();});
1418

15-
beforeEach(() => {
16-
let {createRenderer} = TestUtils;
17-
renderer = createRenderer();
18-
});
19+
let props = {};
1920

2021
it('should render autoHideContainer(<TestComponent />)', () => {
21-
let out = render();
22-
expect(out).toEqualJSX(<TestComponent />);
22+
let {createRenderer} = TestUtils;
23+
let renderer = createRenderer();
24+
props.hello = 'son';
25+
let AutoHide = autoHideContainer(TestComponent);
26+
renderer.render(<AutoHide {...props} />);
27+
let out = renderer.getRenderOutput();
28+
expect(out).toEqualJSX(<TestComponent hello="son" />);
2329
});
2430

25-
it('should not render autoHideContainer(<TestComponent />)', () => {
26-
let out = render({shouldAutoHideContainer: true});
27-
expect(out).toEqualJSX(<div />);
28-
});
31+
context('props.shouldAutoHideContainer', () => {
32+
let AutoHide;
33+
let component;
34+
let container;
2935

30-
function render(props = {}) {
31-
let AutoHide = autoHideContainer(TestComponent);
32-
renderer.render(<AutoHide {...props} />);
33-
return renderer.getRenderOutput();
34-
}
35-
});
36+
beforeEach(() => {
37+
AutoHide = autoHideContainer(TestComponent);
38+
container = document.createElement('div');
39+
props = {hello: 'mom'};
40+
component = ReactDOM.render(<AutoHide {...props} />, container);
41+
});
3642

43+
it('creates a component', () => expect(component).toExist());
44+
45+
it('shows the container at first', () => {
46+
expect(container.style.display).toNotEqual('none');
47+
});
48+
49+
context('when set to true', () => {
50+
beforeEach(() => {
51+
sinon.spy(component, 'render');
52+
props.shouldAutoHideContainer = true;
53+
ReactDOM.render(<AutoHide {...props} />, container);
54+
});
55+
56+
it('hides the container', () => {
57+
expect(container.style.display).toEqual('none');
58+
});
59+
60+
it('does not call component.render()', () => {
61+
expect(component.render.called).toBe(false);
62+
});
63+
64+
context('when set back to false', () => {
65+
beforeEach(() => {
66+
props.shouldAutoHideContainer = false;
67+
ReactDOM.render(<AutoHide {...props} />, container);
68+
});
69+
70+
it('shows the container', () => {
71+
expect(container.style.display).toNotEqual('none');
72+
});
73+
74+
it('calls component.render()', () => {
75+
expect(component.render.calledOnce).toBe(true);
76+
});
77+
});
78+
});
79+
});
80+
});

src/decorators/autoHideContainer.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,23 @@ function autoHideContainer(ComposedComponent) {
1111
}
1212

1313
componentWillReceiveProps(nextProps) {
14+
if (this.props.shouldAutoHideContainer === nextProps.shouldAutoHideContainer) {
15+
return;
16+
}
17+
1418
this._hideOrShowContainer(nextProps);
1519
}
1620

21+
shouldComponentUpdate(nextProps) {
22+
return nextProps.shouldAutoHideContainer === false;
23+
}
24+
1725
_hideOrShowContainer(props) {
1826
let container = ReactDOM.findDOMNode(this).parentNode;
1927
container.style.display = (props.shouldAutoHideContainer === true) ? 'none' : '';
2028
}
2129

2230
render() {
23-
if (this.props.shouldAutoHideContainer === true) {
24-
return <div />;
25-
}
26-
2731
return <ComposedComponent {...this.props} />;
2832
}
2933
}

0 commit comments

Comments
 (0)