Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid unnecessary selector evaluations #1273

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -50,12 +50,20 @@ export function useSelector(selector) {
])

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 @@ -70,8 +78,6 @@ export function useSelector(selector) {
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 @@ -47,6 +47,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 @@ -156,6 +179,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