Skip to content

Commit

Permalink
Avoid unnecessary selector evaluations (#1273)
Browse files Browse the repository at this point in the history
* Avoid unnecessary selector evaluations

* Clean up state assignment logic

* Add missing shallowEqual export
  • Loading branch information
josepot authored and markerikson committed May 19, 2019
1 parent 8a673c9 commit 007b5c7
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 7 deletions.
4 changes: 3 additions & 1 deletion src/alternate-renderers.js
Expand Up @@ -8,6 +8,7 @@ import { useSelector } from './hooks/useSelector'
import { useStore } from './hooks/useStore'

import { getBatch } from './utils/batch'
import shallowEqual from './utils/shallowEqual'

// For other renderers besides ReactDOM and React Native, use the default noop batch function
const batch = getBatch()
Expand All @@ -20,5 +21,6 @@ export {
batch,
useDispatch,
useSelector,
useStore
useStore,
shallowEqual
}
16 changes: 11 additions & 5 deletions src/hooks/useSelector.js
Expand Up @@ -52,12 +52,20 @@ export function useSelector(selector, equalityFn = refEquality) {
])

const latestSubscriptionCallbackError = useRef()
const latestSelector = useRef(selector)
const latestSelector = useRef()
const latestSelectedState = useRef()

let selectedState = undefined
let selectedState

try {
selectedState = selector(store.getState())
if (
selector !== latestSelector.current ||
latestSubscriptionCallbackError.current
) {
selectedState = selector(store.getState())
} else {
selectedState = latestSelectedState.current
}
} catch (err) {
let errorMessage = `An error occured while selecting the store state: ${
err.message
Expand All @@ -72,8 +80,6 @@ export function useSelector(selector, equalityFn = refEquality) {
throw new Error(errorMessage)
}

const latestSelectedState = useRef(selectedState)

useIsomorphicLayoutEffect(() => {
latestSelector.current = selector
latestSelectedState.current = selectedState
Expand Down
58 changes: 57 additions & 1 deletion test/hooks/useSelector.spec.js
@@ -1,6 +1,6 @@
/*eslint-disable react/prop-types*/

import React from 'react'
import React, { useCallback, useReducer } from 'react'
import { createStore } from 'redux'
import { renderHook, act } from 'react-hooks-testing-library'
import * as rtl from 'react-testing-library'
Expand Down Expand Up @@ -51,6 +51,29 @@ describe('React', () => {
})

describe('lifeycle interactions', () => {
it('always uses the latest state', () => {
store = createStore(c => c + 1, -1)

const Comp = () => {
const selector = useCallback(c => c + 1, [])
const value = useSelector(selector)
renderedItems.push(value)
return <div />
}

rtl.render(
<ProviderMock store={store}>
<Comp />
</ProviderMock>
)

expect(renderedItems).toEqual([1])

store.dispatch({ type: '' })

expect(renderedItems).toEqual([1, 2])
})

it('subscribes to the store synchronously', () => {
let rootSubscription

Expand Down Expand Up @@ -183,6 +206,39 @@ describe('React', () => {
})
})

it('uses the latest selector', () => {
let selectorId = 0
let forceRender

const Comp = () => {
const [, f] = useReducer(c => c + 1, 0)
forceRender = f
const renderedSelectorId = selectorId++
const value = useSelector(() => renderedSelectorId)
renderedItems.push(value)
return <div />
}

rtl.render(
<ProviderMock store={store}>
<Comp />
</ProviderMock>
)

expect(renderedItems).toEqual([0])

rtl.act(forceRender)
expect(renderedItems).toEqual([0, 1])

rtl.act(() => {
store.dispatch({ type: '' })
})
expect(renderedItems).toEqual([0, 1])

rtl.act(forceRender)
expect(renderedItems).toEqual([0, 1, 2])
})

describe('edge cases', () => {
it('ignores transient errors in selector (e.g. due to stale props)', () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => {})
Expand Down

0 comments on commit 007b5c7

Please sign in to comment.