fix suspense throttling

facebook · Apr 4, 2022 · a2bfae1 · a2bfae1
1 parent b8cfda1
commit a2bfae1
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 16 deletions.
diff --git 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
@@ -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
@@ -285,6 +285,57 @@ describe('ReactSuspense', () => {
     expect(root).toMatchRenderedOutput('AsyncAfter SuspenseSibling');
   });
 
+  it('globally throttles fallback committing', () => {
+    function Foo() {
+      Scheduler.unstable_yieldValue('Foo');
+      return (
+        <Suspense fallback={<Text text="Loading..." />}>
+          <AsyncText text="A" ms={200} />
+          <Suspense fallback={<Text text="Loading more..." />}>
+            <AsyncText text="B" ms={300} />
+          </Suspense>
+        </Suspense>
+      );
+    }
+
+    // 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(<Foo />, {
+      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 ' +