From 76a1d087f04e2148dec0ff9ed33c47918611fb04 Mon Sep 17 00:00:00 2001 From: daishi Date: Mon, 27 Jan 2020 19:37:51 +0900 Subject: [PATCH 1/3] Fix useSelector tearing in Concurrent Mode --- src/hooks/useSelector.js | 86 ++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 39 deletions(-) diff --git a/src/hooks/useSelector.js b/src/hooks/useSelector.js index bfb2d70fe..7f98d0c8c 100644 --- a/src/hooks/useSelector.js +++ b/src/hooks/useSelector.js @@ -1,7 +1,6 @@ -import { useReducer, useRef, useMemo, useContext } from 'react' +import { useState, useEffect, useMemo, useContext } 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 @@ -12,61 +11,70 @@ function useSelectorWithStoreAndSubscription( store, contextSub ) { - const [, forceRender] = useReducer(s => s + 1, 0) + const [state, setState] = useState(() => ({ + storeState: store.getState(), + selector, + selectedState: selector(store.getState()) + // subscriptionCallbackError: undefined + })) const subscription = useMemo(() => new Subscription(store, contextSub), [ store, contextSub ]) - const latestSubscriptionCallbackError = useRef() - const latestSelector = useRef() - const latestSelectedState = useRef() - - let selectedState + let selectedState = state.selectedState try { - if ( - selector !== latestSelector.current || - latestSubscriptionCallbackError.current - ) { - selectedState = selector(store.getState()) - } else { - selectedState = latestSelectedState.current + if (selector !== state.selector || state.subscriptionCallbackError) { + const newSelectedState = selector(state.storeState) + if (!equalityFn(newSelectedState, selectedState)) { + selectedState = newSelectedState + // schedule another update + setState(prevState => ({ + ...prevState, + selector, + selectedState + // subscriptionCallbackError: undefined + })) + } } } catch (err) { - if (latestSubscriptionCallbackError.current) { - err.message += `\nThe error may be correlated with this previous error:\n${latestSubscriptionCallbackError.current.stack}\n\n` + if (state.subscriptionCallbackError) { + err.message += `\nThe error may be correlated with this previous error:\n${state.subscriptionCallbackError.stack}\n\n` } throw err } - useIsomorphicLayoutEffect(() => { - latestSelector.current = selector - latestSelectedState.current = selectedState - latestSubscriptionCallbackError.current = undefined - }) - - useIsomorphicLayoutEffect(() => { + useEffect(() => { function checkForUpdates() { - try { - const newSelectedState = latestSelector.current(store.getState()) - - if (equalityFn(newSelectedState, latestSelectedState.current)) { - return + const newStoreState = store.getState() + setState(prevState => { + let newSelectedState + let subscriptionCallbackError + try { + newSelectedState = prevState.selector(newStoreState) + + if (equalityFn(newSelectedState, prevState.selectedState)) { + // bail out rendering + return prevState + } + } 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 + subscriptionCallbackError = err } - 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({}) + return { + ...prevState, + storeState: newStoreState, + selectedState: newSelectedState, + subscriptionCallbackError + } + }) } subscription.onStateChange = checkForUpdates From 32481c9817c5dbfe05b74873c5537df76cb6f3a7 Mon Sep 17 00:00:00 2001 From: daishi Date: Tue, 28 Jan 2020 00:37:14 +0900 Subject: [PATCH 2/3] combine the idea used in #1505 --- src/hooks/useSelector.js | 84 ++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 56 deletions(-) diff --git a/src/hooks/useSelector.js b/src/hooks/useSelector.js index 7f98d0c8c..c18ab0c1e 100644 --- a/src/hooks/useSelector.js +++ b/src/hooks/useSelector.js @@ -1,4 +1,4 @@ -import { useState, useEffect, useMemo, useContext } from 'react' +import { useReducer, useEffect, useMemo, useContext } from 'react' import { useReduxContext as useDefaultReduxContext } from './useReduxContext' import Subscription from '../utils/Subscription' import { ReactReduxContext } from '../components/Context' @@ -11,12 +11,26 @@ function useSelectorWithStoreAndSubscription( store, contextSub ) { - const [state, setState] = useState(() => ({ - storeState: store.getState(), - selector, - selectedState: selector(store.getState()) - // subscriptionCallbackError: undefined - })) + const [state, dispatch] = useReducer( + (prevState, storeState) => { + const nextSelectedState = selector(storeState) + if (equalityFn(nextSelectedState, prevState.selectedState)) { + // bail out + return prevState + } + return { + selector, + storeState, + selectedState: nextSelectedState + } + }, + store.getState(), + storeState => ({ + selector, + storeState, + selectedState: selector(storeState) + }) + ) const subscription = useMemo(() => new Subscription(store, contextSub), [ store, @@ -24,59 +38,17 @@ function useSelectorWithStoreAndSubscription( ]) let selectedState = state.selectedState - - try { - if (selector !== state.selector || state.subscriptionCallbackError) { - const newSelectedState = selector(state.storeState) - if (!equalityFn(newSelectedState, selectedState)) { - selectedState = newSelectedState - // schedule another update - setState(prevState => ({ - ...prevState, - selector, - selectedState - // subscriptionCallbackError: undefined - })) - } - } - } catch (err) { - if (state.subscriptionCallbackError) { - err.message += `\nThe error may be correlated with this previous error:\n${state.subscriptionCallbackError.stack}\n\n` + if (state.selector !== selector) { + const nextSelectedState = selector(state.storeState) + if (!equalityFn(nextSelectedState, state.selectedState)) { + selectedState = nextSelectedState + // schedule another update + dispatch(state.storeState) } - - throw err } useEffect(() => { - function checkForUpdates() { - const newStoreState = store.getState() - setState(prevState => { - let newSelectedState - let subscriptionCallbackError - try { - newSelectedState = prevState.selector(newStoreState) - - if (equalityFn(newSelectedState, prevState.selectedState)) { - // bail out rendering - return prevState - } - } 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 - subscriptionCallbackError = err - } - - return { - ...prevState, - storeState: newStoreState, - selectedState: newSelectedState, - subscriptionCallbackError - } - }) - } - + const checkForUpdates = () => dispatch(store.getState()) subscription.onStateChange = checkForUpdates subscription.trySubscribe() From 0768b4505270744bac84e2c4965fbc427afe4309 Mon Sep 17 00:00:00 2001 From: daishi Date: Tue, 28 Jan 2020 07:04:11 +0900 Subject: [PATCH 3/3] read fresh store state on dispatch --- src/hooks/useSelector.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useSelector.js b/src/hooks/useSelector.js index c18ab0c1e..4fd7af2f2 100644 --- a/src/hooks/useSelector.js +++ b/src/hooks/useSelector.js @@ -43,7 +43,7 @@ function useSelectorWithStoreAndSubscription( if (!equalityFn(nextSelectedState, state.selectedState)) { selectedState = nextSelectedState // schedule another update - dispatch(state.storeState) + dispatch(store.getState()) } }