From d16262582b2eeb62c05313fca3eb59dc0b395955 Mon Sep 17 00:00:00 2001 From: Jonathan Ziller Date: Fri, 26 Apr 2019 15:43:49 +0200 Subject: [PATCH] fix timing issue in component updates due to consecutive dispatches (#1263) --- src/components/connectAdvanced.js | 7 ++++- test/components/connect.spec.js | 48 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/components/connectAdvanced.js b/src/components/connectAdvanced.js index c1f98f1b2..1815053ad 100644 --- a/src/components/connectAdvanced.js +++ b/src/components/connectAdvanced.js @@ -253,6 +253,7 @@ export default function connectAdvanced( const lastChildProps = useRef() const lastWrapperProps = useRef(wrapperProps) const childPropsFromStoreUpdate = useRef() + const renderIsScheduled = useRef(false) const actualChildProps = usePureOnlyMemo(() => { // Tricky logic here: @@ -282,6 +283,7 @@ export default function connectAdvanced( // We want to capture the wrapper props and child props we used for later comparisons lastWrapperProps.current = wrapperProps lastChildProps.current = actualChildProps + renderIsScheduled.current = false // If the render was from a store update, clear out that reference and cascade the subscriber update if (childPropsFromStoreUpdate.current) { @@ -328,7 +330,9 @@ export default function connectAdvanced( // If the child props haven't changed, nothing to do here - cascade the subscription update if (newChildProps === lastChildProps.current) { - notifyNestedSubs() + if (!renderIsScheduled.current) { + notifyNestedSubs() + } } else { // Save references to the new child props. Note that we track the "child props from store update" // as a ref instead of a useState/useReducer because we need a way to determine if that value has @@ -336,6 +340,7 @@ export default function connectAdvanced( // forcing another re-render, which we don't want. lastChildProps.current = newChildProps childPropsFromStoreUpdate.current = newChildProps + renderIsScheduled.current = true // If the child props _did_ change (or we caught an error), this wrapper component needs to re-render forceComponentUpdateDispatch({ diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index b2bbe2a4d..4b89fa19d 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -3182,6 +3182,54 @@ describe('React', () => { expect(reduxCountPassedToMapState).toEqual(3) }) + + it('should ensure top-down updates for consecutive batched updates', () => { + const INC = 'INC' + const reducer = (c = 0, { type }) => (type === INC ? c + 1 : c) + const store = createStore(reducer) + + let executionOrder = [] + let expectedExecutionOrder = [ + 'parent map', + 'parent render', + 'child map', + 'child render' + ] + + const ChildImpl = () => { + executionOrder.push('child render') + return
child
+ } + + const Child = connect(state => { + executionOrder.push('child map') + return { state } + })(ChildImpl) + + const ParentImpl = () => { + executionOrder.push('parent render') + return + } + + const Parent = connect(state => { + executionOrder.push('parent map') + return { state } + })(ParentImpl) + + rtl.render( + + + + ) + + executionOrder = [] + rtl.act(() => { + store.dispatch({ type: INC }) + store.dispatch({ type: '' }) + }) + + expect(executionOrder).toEqual(expectedExecutionOrder) + }) }) it("should enforce top-down updates to ensure a deleted child's mapState doesn't throw errors", () => {