Skip to content

Commit 49dd7d1

Browse files
committed
Bail out if stateProps can be calculated early and did not change
Fixes #1437, #300
1 parent a0947cf commit 49dd7d1

File tree

2 files changed

+66
-6
lines changed

2 files changed

+66
-6
lines changed

src/components/connect.js

+18-5
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
219219
this.mergedProps = null
220220
this.haveOwnPropsChanged = true
221221
this.hasStoreStateChanged = true
222+
this.haveStatePropsBeenPrecalculated = false
222223
this.renderedElement = null
223224
this.finalMapDispatchToProps = null
224225
this.finalMapStateToProps = null
@@ -229,13 +230,21 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
229230
return
230231
}
231232

232-
const prevStoreState = this.state.storeState
233233
const storeState = this.store.getState()
234+
const prevStoreState = this.state.storeState
235+
if (pure && prevStoreState === storeState) {
236+
return
237+
}
234238

235-
if (!pure || prevStoreState !== storeState) {
236-
this.hasStoreStateChanged = true
237-
this.setState({ storeState })
239+
if (pure && !this.doStatePropsDependOnOwnProps) {
240+
if (!this.updateStatePropsIfNeeded()) {
241+
return
242+
}
243+
this.haveStatePropsBeenPrecalculated = true
238244
}
245+
246+
this.hasStoreStateChanged = true
247+
this.setState({ storeState })
239248
}
240249

241250
getWrappedInstance() {
@@ -251,11 +260,13 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
251260
const {
252261
haveOwnPropsChanged,
253262
hasStoreStateChanged,
263+
haveStatePropsBeenPrecalculated,
254264
renderedElement
255265
} = this
256266

257267
this.haveOwnPropsChanged = false
258268
this.hasStoreStateChanged = false
269+
this.haveStatePropsBeenPrecalculated = false
259270

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

270281
let haveStatePropsChanged = false
271282
let haveDispatchPropsChanged = false
272-
if (shouldUpdateStateProps) {
283+
if (haveStatePropsBeenPrecalculated) {
284+
haveStatePropsChanged = true
285+
} else if (shouldUpdateStateProps) {
273286
haveStatePropsChanged = this.updateStatePropsIfNeeded()
274287
}
275288
if (shouldUpdateDispatchProps) {

test/components/connect.spec.js

+48-1
Original file line numberDiff line numberDiff line change
@@ -1082,7 +1082,6 @@ describe('React', () => {
10821082
)
10831083
spy.destroy()
10841084

1085-
10861085
spy = expect.spyOn(console, 'error')
10871086
TestUtils.renderIntoDocument(
10881087
<ProviderMock store={store}>
@@ -1547,6 +1546,54 @@ describe('React', () => {
15471546
expect(renderCalls).toBe(1)
15481547
})
15491548

1549+
it('should bail out early if neither mapState nor mapDispatch depend on props', () => {
1550+
const store = createStore(stringBuilder)
1551+
let renderCalls = 0
1552+
let mapDispatchCalls = 0
1553+
let mapStateCalls = 0
1554+
1555+
@connect(state => {
1556+
mapStateCalls++
1557+
return state === 'aaa' ? { change: 1 } : {}
1558+
}, _ => {
1559+
return {}
1560+
})
1561+
class Container extends Component {
1562+
render() {
1563+
renderCalls++
1564+
return <Passthrough {...this.props} />
1565+
}
1566+
}
1567+
1568+
TestUtils.renderIntoDocument(
1569+
<ProviderMock store={store}>
1570+
<Container />
1571+
</ProviderMock>
1572+
)
1573+
1574+
expect(renderCalls).toBe(1)
1575+
expect(mapStateCalls).toBe(1)
1576+
1577+
const spy = expect.spyOn(Container.prototype, 'setState').andCallThrough()
1578+
1579+
store.dispatch({ type: 'APPEND', body: 'a' })
1580+
expect(mapStateCalls).toBe(2)
1581+
expect(renderCalls).toBe(1)
1582+
expect(spy.calls.length).toBe(0)
1583+
1584+
store.dispatch({ type: 'APPEND', body: 'a' })
1585+
expect(mapStateCalls).toBe(3)
1586+
expect(renderCalls).toBe(1)
1587+
expect(spy.calls.length).toBe(0)
1588+
1589+
store.dispatch({ type: 'APPEND', body: 'a' })
1590+
expect(mapStateCalls).toBe(4)
1591+
expect(renderCalls).toBe(2)
1592+
expect(spy.calls.length).toBe(1)
1593+
1594+
spy.destroy()
1595+
})
1596+
15501597
it('should allow providing a factory function to mapStateToProps', () => {
15511598
let updatedCount = 0
15521599
let memoizedReturnCount = 0

0 commit comments

Comments
 (0)