-
Notifications
You must be signed in to change notification settings - Fork 516
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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.
- Loading branch information
vvo
committed
Feb 17, 2016
1 parent
77586cb
commit 8c89862
Showing
2 changed files
with
69 additions
and
21 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
}); | ||
}); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters