Skip to content

Commit 28f928a

Browse files
committed
Avoid unnecessary selector evaluations
1 parent d849ede commit 28f928a

File tree

2 files changed

+108
-39
lines changed

2 files changed

+108
-39
lines changed

src/hooks/useSelector.js

Lines changed: 49 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useReducer, useRef, useEffect, useMemo, useLayoutEffect } from 'react'
1+
import { useState, useRef, useEffect, useMemo, useLayoutEffect } from 'react'
22
import invariant from 'invariant'
33
import { useReduxContext } from './useReduxContext'
44
import shallowEqual from '../utils/shallowEqual'
@@ -42,52 +42,27 @@ export function useSelector(selector) {
4242
invariant(selector, `You must pass a selector to useSelectors`)
4343

4444
const { store, subscription: contextSub } = useReduxContext()
45-
const [, forceRender] = useReducer(s => s + 1, 0)
46-
4745
const subscription = useMemo(() => new Subscription(store, contextSub), [
4846
store,
4947
contextSub
5048
])
5149

52-
const latestSubscriptionCallbackError = useRef()
53-
const latestSelector = useRef(selector)
54-
55-
let selectedState = undefined
56-
57-
try {
58-
selectedState = selector(store.getState())
59-
} catch (err) {
60-
let errorMessage = `An error occured while selecting the store state: ${
61-
err.message
62-
}.`
50+
const [subscriptionResult, setSubscriptionResult] = useState()
6351

64-
if (latestSubscriptionCallbackError.current) {
65-
errorMessage += `\nThe error may be correlated with this previous error:\n${
66-
latestSubscriptionCallbackError.current.stack
67-
}\n\nOriginal stack trace:`
68-
}
69-
70-
throw new Error(errorMessage)
71-
}
72-
73-
const latestSelectedState = useRef(selectedState)
74-
75-
useIsomorphicLayoutEffect(() => {
76-
latestSelector.current = selector
77-
latestSelectedState.current = selectedState
78-
latestSubscriptionCallbackError.current = undefined
79-
})
52+
const latestSelector = useRef()
53+
const latestResult = useRef()
54+
const latestSubscriptionCallbackError = useRef()
55+
const latestSubscriptionResult = useRef()
8056

8157
useIsomorphicLayoutEffect(() => {
8258
function checkForUpdates() {
59+
let newSelectedState
8360
try {
84-
const newSelectedState = latestSelector.current(store.getState())
61+
newSelectedState = latestSelector.current(store.getState())
8562

86-
if (shallowEqual(newSelectedState, latestSelectedState.current)) {
63+
if (shallowEqual(newSelectedState, latestResult.current)) {
8764
return
8865
}
89-
90-
latestSelectedState.current = newSelectedState
9166
} catch (err) {
9267
// we ignore all errors here, since when the component
9368
// is re-rendered, the selectors are called again, and
@@ -96,16 +71,52 @@ export function useSelector(selector) {
9671
latestSubscriptionCallbackError.current = err
9772
}
9873

99-
forceRender({})
74+
const newSubscriptionResult = new Array(1)
75+
newSubscriptionResult[0] = newSelectedState
76+
setSubscriptionResult(newSubscriptionResult)
10077
}
10178

10279
subscription.onStateChange = checkForUpdates
10380
subscription.trySubscribe()
10481

105-
checkForUpdates()
106-
10782
return () => subscription.tryUnsubscribe()
10883
}, [store, subscription])
10984

110-
return selectedState
85+
let result = latestResult.current
86+
87+
useIsomorphicLayoutEffect(() => {
88+
latestResult.current = result
89+
latestSelector.current = selector
90+
latestSubscriptionCallbackError.current = undefined
91+
latestSubscriptionResult.current = subscriptionResult
92+
})
93+
94+
try {
95+
return (result =
96+
// If the selector has changed or if there has been an error while
97+
// it was being evaluated inside "checkForUpdates", then the selector
98+
// has to be re-evaluated
99+
selector !== latestSelector.current ||
100+
latestSubscriptionCallbackError.current
101+
? selector(store.getState())
102+
: // If this update was triggered by "checkForUpdates", then we
103+
// should return the result that was already calculated in there.
104+
// Otherwise, this is an update triggered by an ancestor and
105+
// there is no need to recalculate the result
106+
subscriptionResult !== latestSubscriptionResult.current
107+
? subscriptionResult[0]
108+
: result)
109+
} catch (err) {
110+
let errorMessage = `An error occured while selecting the store state: ${
111+
err.message
112+
}.`
113+
114+
if (latestSubscriptionCallbackError.current) {
115+
errorMessage += `\nThe error may be correlated with this previous error:\n${
116+
latestSubscriptionCallbackError.current.stack
117+
}\n\nOriginal stack trace:`
118+
}
119+
120+
throw new Error(errorMessage)
121+
}
111122
}

test/hooks/useSelector.spec.js

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*eslint-disable react/prop-types*/
22

3-
import React from 'react'
3+
import React, { useCallback, useReducer } from 'react'
44
import { createStore } from 'redux'
55
import { renderHook, act } from 'react-hooks-testing-library'
66
import * as rtl from 'react-testing-library'
@@ -31,6 +31,31 @@ describe('React', () => {
3131
expect(result.current).toEqual(0)
3232
})
3333

34+
it('always uses the latest state', () => {
35+
store = createStore(c => c + 1, -1)
36+
37+
const Comp = () => {
38+
const selector = useCallback(c => c + 1, [])
39+
const value = useSelector(selector)
40+
renderedItems.push(value)
41+
return <div />
42+
}
43+
44+
rtl.render(
45+
<ProviderMock store={store}>
46+
<Comp />
47+
</ProviderMock>
48+
)
49+
50+
expect(renderedItems).toEqual([1])
51+
52+
act(() => {
53+
store.dispatch({ type: '' })
54+
})
55+
56+
expect(renderedItems).toEqual([1, 2])
57+
})
58+
3459
it('selects the state and renders the component when the store updates', () => {
3560
const { result } = renderHook(() => useSelector(s => s.count), {
3661
wrapper: props => <ProviderMock {...props} store={store} />
@@ -156,6 +181,39 @@ describe('React', () => {
156181
})
157182
})
158183

184+
it('uses the latest selector', () => {
185+
let selectorId = 0
186+
let forceRender
187+
188+
const Comp = () => {
189+
const [, f] = useReducer(c => c + 1, 0)
190+
forceRender = f
191+
const renderedSelectorId = selectorId++
192+
const value = useSelector(() => renderedSelectorId)
193+
renderedItems.push(value)
194+
return <div />
195+
}
196+
197+
rtl.render(
198+
<ProviderMock store={store}>
199+
<Comp />
200+
</ProviderMock>
201+
)
202+
203+
expect(renderedItems).toEqual([0])
204+
205+
rtl.act(forceRender)
206+
expect(renderedItems).toEqual([0, 1])
207+
208+
rtl.act(() => {
209+
store.dispatch({ type: '' })
210+
})
211+
expect(renderedItems).toEqual([0, 1])
212+
213+
rtl.act(forceRender)
214+
expect(renderedItems).toEqual([0, 1, 2])
215+
})
216+
159217
describe('edge cases', () => {
160218
it('ignores transient errors in selector (e.g. due to stale props)', () => {
161219
const spy = jest.spyOn(console, 'error').mockImplementation(() => {})

0 commit comments

Comments
 (0)