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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental useSelector to avoid stale props #1505

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 24 additions & 53 deletions src/hooks/useSelector.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { useReducer, useRef, useMemo, useContext } from 'react'
import { useReducer, useEffect, useMemo, useContext, useRef } from 'react'
import { useReduxContext as useDefaultReduxContext } from './useReduxContext'
import Subscription from '../utils/Subscription'
import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect'
import { ReactReduxContext } from '../components/Context'

const refEquality = (a, b) => a === b
Expand All @@ -12,63 +11,35 @@ function useSelectorWithStoreAndSubscription(
store,
contextSub
) {
const [, forceRender] = useReducer(s => s + 1, 0)
const dispatching = useRef(false)
const [state, dispatch] = useReducer(
(prevState, storeState) => {
if (dispatching.current) {
// schedule update
return { ...prevState }
}
const nextSelectedState = selector(storeState)
if (equalityFn(nextSelectedState, prevState.selectedState)) {
// bail out
return prevState
}
return { selectedState: nextSelectedState }
},
store.getState(),
storeState => ({ selectedState: selector(storeState) })
)

const subscription = useMemo(() => new Subscription(store, contextSub), [
store,
contextSub
])

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

let selectedState

try {
if (
selector !== latestSelector.current ||
latestSubscriptionCallbackError.current
) {
selectedState = selector(store.getState())
} else {
selectedState = latestSelectedState.current
useEffect(() => {
const checkForUpdates = () => {
dispatching.current = true
dispatch(store.getState())
dispatching.current = false
}
} catch (err) {
if (latestSubscriptionCallbackError.current) {
err.message += `\nThe error may be correlated with this previous error:\n${latestSubscriptionCallbackError.current.stack}\n\n`
}

throw err
}

useIsomorphicLayoutEffect(() => {
latestSelector.current = selector
latestSelectedState.current = selectedState
latestSubscriptionCallbackError.current = undefined
})

useIsomorphicLayoutEffect(() => {
function checkForUpdates() {
try {
const newSelectedState = latestSelector.current(store.getState())

if (equalityFn(newSelectedState, latestSelectedState.current)) {
return
}

latestSelectedState.current = newSelectedState
} catch (err) {
// we ignore all errors here, since when the component
// is re-rendered, the selectors are called again, and
// will throw again, if neither props nor store state
// changed
latestSubscriptionCallbackError.current = err
}

forceRender({})
}

subscription.onStateChange = checkForUpdates
subscription.trySubscribe()

Expand All @@ -77,7 +48,7 @@ function useSelectorWithStoreAndSubscription(
return () => subscription.tryUnsubscribe()
}, [store, subscription])

return selectedState
return state.selectedState
}

/**
Expand Down
77 changes: 77 additions & 0 deletions test/integration/stale-props.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*eslint-disable react/prop-types*/

import React from 'react'
import { createStore } from 'redux'
import { Provider, useSelector } from '../../src/index.js'
import * as rtl from '@testing-library/react'

describe('React', () => {
describe('stale props with useSelector', () => {
it('ignores transient errors in selector (e.g. due to stale props)', () => {
const Parent = () => {
const count = useSelector(count => count)
return <Child parentCount={count} />
}

const Child = ({ parentCount }) => {
const result = useSelector(count => {
if (count !== parentCount) {
throw new Error()
}

return count + parentCount
})

return <div>{result}</div>
}

const store = createStore((count = -1) => count + 1)

const App = () => (
<Provider store={store}>
<Parent />
</Provider>
)

rtl.render(<App />)

rtl.act(() => {
expect(() => store.dispatch({ type: '' })).not.toThrowError()
})
})

it('ensures consistency of state and props in selector', () => {
let selectorSawInconsistencies = false

const Parent = () => {
const count = useSelector(count => count)
return <Child parentCount={count} />
}

const Child = ({ parentCount }) => {
const result = useSelector(count => {
selectorSawInconsistencies =
selectorSawInconsistencies || count !== parentCount
return count + parentCount
})

return <div>{result}</div>
}

const store = createStore((count = -1) => count + 1)

const App = () => (
<Provider store={store}>
<Parent />
</Provider>
)

rtl.render(<App />)

rtl.act(() => {
store.dispatch({ type: '' })
})
expect(selectorSawInconsistencies).toBe(false)
})
})
})