Skip to content

Commit

Permalink
Bail out if stateProps can be calculated early and did not change
Browse files Browse the repository at this point in the history
Fixes #1437, #300
  • Loading branch information
gaearon committed Apr 12, 2016
1 parent a0947cf commit 388c050
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 6 deletions.
23 changes: 18 additions & 5 deletions src/components/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
this.mergedProps = null
this.haveOwnPropsChanged = true
this.hasStoreStateChanged = true
this.haveStatePropsBeenPrecalculated = false
this.renderedElement = null
this.finalMapDispatchToProps = null
this.finalMapStateToProps = null
Expand All @@ -229,13 +230,21 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
return
}

const prevStoreState = this.state.storeState
const storeState = this.store.getState()
const prevStoreState = this.state.storeState
if (pure && prevStoreState === storeState) {
return
}

if (!pure || prevStoreState !== storeState) {
this.hasStoreStateChanged = true
this.setState({ storeState })
if (pure && !this.doStatePropsDependOnOwnProps) {
if (!this.updateStatePropsIfNeeded()) {
return
}
this.haveStatePropsBeenPrecalculated = true
}

this.hasStoreStateChanged = true
this.setState({ storeState })
}

getWrappedInstance() {
Expand All @@ -251,11 +260,13 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
const {
haveOwnPropsChanged,
hasStoreStateChanged,
haveStatePropsBeenPrecalculated,
renderedElement
} = this

this.haveOwnPropsChanged = false
this.hasStoreStateChanged = false
this.haveStatePropsBeenPrecalculated = false

let shouldUpdateStateProps = true
let shouldUpdateDispatchProps = true
Expand All @@ -269,7 +280,9 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,

let haveStatePropsChanged = false
let haveDispatchPropsChanged = false
if (shouldUpdateStateProps) {
if (haveStatePropsBeenPrecalculated) {
haveStatePropsChanged = true
} else if (shouldUpdateStateProps) {
haveStatePropsChanged = this.updateStatePropsIfNeeded()
}
if (shouldUpdateDispatchProps) {
Expand Down
46 changes: 45 additions & 1 deletion test/components/connect.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,6 @@ describe('React', () => {
)
spy.destroy()


spy = expect.spyOn(console, 'error')
TestUtils.renderIntoDocument(
<ProviderMock store={store}>
Expand Down Expand Up @@ -1547,6 +1546,51 @@ describe('React', () => {
expect(renderCalls).toBe(1)
})

it('should bail out early if mapState does not depend on props', () => {
const store = createStore(stringBuilder)
let renderCalls = 0
let mapStateCalls = 0

@connect(state => {
mapStateCalls++
return state === 'aaa' ? { change: 1 } : {}
})
class Container extends Component {
render() {
renderCalls++
return <Passthrough {...this.props} />
}
}

TestUtils.renderIntoDocument(
<ProviderMock store={store}>
<Container />
</ProviderMock>
)

expect(renderCalls).toBe(1)
expect(mapStateCalls).toBe(1)

const spy = expect.spyOn(Container.prototype, 'setState').andCallThrough()

store.dispatch({ type: 'APPEND', body: 'a' })
expect(mapStateCalls).toBe(2)
expect(renderCalls).toBe(1)
expect(spy.calls.length).toBe(0)

store.dispatch({ type: 'APPEND', body: 'a' })
expect(mapStateCalls).toBe(3)
expect(renderCalls).toBe(1)
expect(spy.calls.length).toBe(0)

store.dispatch({ type: 'APPEND', body: 'a' })
expect(mapStateCalls).toBe(4)
expect(renderCalls).toBe(2)
expect(spy.calls.length).toBe(1)

spy.destroy()
})

it('should allow providing a factory function to mapStateToProps', () => {
let updatedCount = 0
let memoizedReturnCount = 0
Expand Down

0 comments on commit 388c050

Please sign in to comment.