From a53f5cc22eab9617a1d5473e16ce872c7158edef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 1 Aug 2019 22:02:28 -0700 Subject: [PATCH] [SuspenseList] Bug fix: Reset renderState when bailing out (#16278) If there are adjacent updates we bail out of rendering the suspense list at all but we may still complete the node. We need to reset the render state in that case. I restructured so that this is in the same code path so we don't forget it in the future. --- .../src/ReactFiberBeginWork.js | 47 ++++++++++--------- .../ReactSuspenseList-test.internal.js | 46 ++++++++++++++++++ 2 files changed, 72 insertions(+), 21 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 83b7f08538b7f..f0f22b38384c2 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -2782,29 +2782,26 @@ function beginWork( const didSuspendBefore = (current.effectTag & DidCapture) !== NoEffect; - const childExpirationTime = workInProgress.childExpirationTime; - if (childExpirationTime < renderExpirationTime) { + const hasChildWork = + workInProgress.childExpirationTime >= renderExpirationTime; + + if (didSuspendBefore) { + if (hasChildWork) { + // If something was in fallback state last time, and we have all the + // same children then we're still in progressive loading state. + // Something might get unblocked by state updates or retries in the + // tree which will affect the tail. So we need to use the normal + // path to compute the correct tail. + return updateSuspenseListComponent( + current, + workInProgress, + renderExpirationTime, + ); + } // If none of the children had any work, that means that none of // them got retried so they'll still be blocked in the same way // as before. We can fast bail out. - pushSuspenseContext(workInProgress, suspenseStackCursor.current); - if (didSuspendBefore) { - workInProgress.effectTag |= DidCapture; - } - return null; - } - - if (didSuspendBefore) { - // If something was in fallback state last time, and we have all the - // same children then we're still in progressive loading state. - // Something might get unblocked by state updates or retries in the - // tree which will affect the tail. So we need to use the normal - // path to compute the correct tail. - return updateSuspenseListComponent( - current, - workInProgress, - renderExpirationTime, - ); + workInProgress.effectTag |= DidCapture; } // If nothing suspended before and we're rendering the same children, @@ -2818,7 +2815,15 @@ function beginWork( renderState.tail = null; } pushSuspenseContext(workInProgress, suspenseStackCursor.current); - break; + + if (hasChildWork) { + break; + } else { + // If none of the children had any work, that means that none of + // them got retried so they'll still be blocked in the same way + // as before. We can fast bail out. + return null; + } } } return bailoutOnAlreadyFinishedWork( diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js index 94fc54601ce79..e8ab1f808b2d6 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js @@ -1826,4 +1826,50 @@ describe('ReactSuspenseList', () => { , ); }); + + it('can do unrelated adjacent updates', async () => { + let updateAdjacent; + function Adjacent() { + let [text, setText] = React.useState('-'); + updateAdjacent = setText; + return ; + } + + function Foo() { + return ( +
+ + + + + +
+ ); + } + + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield(['A', 'B', '-']); + + expect(ReactNoop).toMatchRenderedOutput( +
+ A + B + - +
, + ); + + // Update the row adjacent to the list + ReactNoop.act(() => updateAdjacent('C')); + + expect(Scheduler).toHaveYielded(['C']); + + expect(ReactNoop).toMatchRenderedOutput( +
+ A + B + C +
, + ); + }); });