From a2bfae1164130ed1d365ddd8659c2b5cd8fe3c0f Mon Sep 17 00:00:00 2001 From: sunderls <2394070+sunderls@users.noreply.github.com> Date: Sat, 2 Apr 2022 00:13:34 +0100 Subject: [PATCH] fix suspense throttling --- .../src/ReactFiberCommitWork.new.js | 18 ++++--- .../src/ReactFiberCommitWork.old.js | 18 ++++--- .../__tests__/ReactSuspense-test.internal.js | 51 +++++++++++++++++++ 3 files changed, 71 insertions(+), 16 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index fb60401095184..f3a447e0c2d20 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -2250,22 +2250,25 @@ function commitMutationEffectsOnFiber( } } - if (flags & Visibility) { - switch (finishedWork.tag) { - case SuspenseComponent: { - const newState: OffscreenState | null = finishedWork.memoizedState; + switch (finishedWork.tag) { + case SuspenseComponent: { + const offscreenFiber: Fiber = (finishedWork.child: any); + if (offscreenFiber.flags & Visibility) { + const newState: OffscreenState | null = offscreenFiber.memoizedState; const isHidden = newState !== null; if (isHidden) { - const current = finishedWork.alternate; + const current = offscreenFiber.alternate; const wasHidden = current !== null && current.memoizedState !== null; if (!wasHidden) { // TODO: Move to passive phase markCommitTimeOfFallback(); } } - break; } - case OffscreenComponent: { + break; + } + case OffscreenComponent: { + if (flags & Visibility) { const newState: OffscreenState | null = finishedWork.memoizedState; const isHidden = newState !== null; const current = finishedWork.alternate; @@ -2301,7 +2304,6 @@ function commitMutationEffectsOnFiber( } } } - // The following switch statement is only concerned about placement, // updates, and deletions. To avoid needing to add a case for every possible // bitmap value, we remove the secondary effects from the effect tag and diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index e962039d9ca9c..4a93e4dc94d82 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -2250,22 +2250,25 @@ function commitMutationEffectsOnFiber( } } - if (flags & Visibility) { - switch (finishedWork.tag) { - case SuspenseComponent: { - const newState: OffscreenState | null = finishedWork.memoizedState; + switch (finishedWork.tag) { + case SuspenseComponent: { + const offscreenFiber: Fiber = (finishedWork.child: any); + if (offscreenFiber.flags & Visibility) { + const newState: OffscreenState | null = offscreenFiber.memoizedState; const isHidden = newState !== null; if (isHidden) { - const current = finishedWork.alternate; + const current = offscreenFiber.alternate; const wasHidden = current !== null && current.memoizedState !== null; if (!wasHidden) { // TODO: Move to passive phase markCommitTimeOfFallback(); } } - break; } - case OffscreenComponent: { + break; + } + case OffscreenComponent: { + if (flags & Visibility) { const newState: OffscreenState | null = finishedWork.memoizedState; const isHidden = newState !== null; const current = finishedWork.alternate; @@ -2301,7 +2304,6 @@ function commitMutationEffectsOnFiber( } } } - // The following switch statement is only concerned about placement, // updates, and deletions. To avoid needing to add a case for every possible // bitmap value, we remove the secondary effects from the effect tag and diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 82ee6ee6bf1dc..183b5ba554fea 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -285,6 +285,57 @@ describe('ReactSuspense', () => { expect(root).toMatchRenderedOutput('AsyncAfter SuspenseSibling'); }); + it('globally throttles fallback committing', () => { + function Foo() { + Scheduler.unstable_yieldValue('Foo'); + return ( + }> + + }> + + + + ); + } + + // throttling works on fallback committing + // here advance some time to skip the first threshold + jest.advanceTimersByTime(600); + Scheduler.unstable_advanceTime(600); + + const root = ReactTestRenderer.create(, { + unstable_isConcurrent: true, + }); + + expect(Scheduler).toFlushAndYield([ + 'Foo', + 'Suspend! [A]', + 'Suspend! [B]', + 'Loading more...', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput('Loading...'); + + // resolve A + jest.advanceTimersByTime(200); + Scheduler.unstable_advanceTime(200); + expect(Scheduler).toHaveYielded(['Promise resolved [A]']); + expect(Scheduler).toFlushAndYield(['A', 'Suspend! [B]', 'Loading more...']); + + // should still renders previous fallback + expect(root).toMatchRenderedOutput('Loading...'); + + // resolve B + jest.advanceTimersByTime(100); + Scheduler.unstable_advanceTime(100); + expect(Scheduler).toHaveYielded(['Promise resolved [B]']); + + // before commiting we still shows previous fallback + expect(root).toMatchRenderedOutput('Loading...'); + expect(Scheduler).toFlushAndYield(['A', 'B']); + expect(root).toMatchRenderedOutput('AB'); + }); + // @gate !enableSyncDefaultUpdates it( 'interrupts current render when something suspends with a ' +