Apply the Just Noticeable Difference boundary

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -692,9 +692,9 @@ function completeWork(
           // Mark the event time of the switching from fallback to normal children,
           // based on the start of when we first showed the fallback. This time
           // was given a normal pri expiration time at the time it was shown.
-          const fallbackExpirationTimeExpTime: ExpirationTime =
+          const fallbackExpirationTime: ExpirationTime =
             prevState.fallbackExpirationTime;
-          markRenderEventTime(fallbackExpirationTimeExpTime);
+          markRenderEventTime(fallbackExpirationTime);
 
           // Delete the fallback.
           // TODO: Would it be better to store the fallback fragment on

packages/react-reconciler/src/ReactFiberScheduler.new.js

@@ -896,7 +896,8 @@ function renderRoot(
           const msUntilTimeout = computeMsUntilTimeout(
             workInProgressRootMostRecentEventTime,
           );
-          if (msUntilTimeout > 0) {
+          // Don't bother with a very short suspense time.
+          if (msUntilTimeout > 10) {
             // The render is suspended, it hasn't timed out, and there's no lower
             // priority work to do. Instead of committing the fallback
             // immediately, wait for more data to arrive.
@@ -1815,6 +1816,33 @@ export function resolveRetryThenable(boundaryFiber: Fiber, thenable: Thenable) {
   retryTimedOutBoundary(boundaryFiber);
 }
 
+const ceil = Math.ceil;
+
+// Computes the next Just Noticeable Difference (JND) boundary.
+// The theory is that a person can't tell the difference between small differences in time.
+// Therefore, if we wait a bit longer than necessary that won't translate to a noticeable
+// difference in the experience. However, waiting for longer might mean that we can avoid
+// showing an intermediate loading state. The longer we have already waited, the harder it
+// is to tell small differences in time. Therefore, the longer we've already waited,
+// the longer we can wait additionally. At some point we have to give up though.
+// We pick a train model where the next boundary commits at a consistent schedule.
+// These particular numbers are vague estimates. We expect to adjust them based on research.
+function jnd(timeElapsed: number) {
+  return timeElapsed < 120
+    ? 120
+    : timeElapsed < 480
+      ? 480
+      : timeElapsed < 1080
+        ? 1080
+        : timeElapsed < 1920
+          ? 1920
+          : timeElapsed < 3000
+            ? 3000
+            : timeElapsed < 4320
+              ? 4320
+              : ceil(timeElapsed / 1960) * 1960;
+}
+
 function computeMsUntilTimeout(mostRecentEventTime: ExpirationTime) {
   if (disableYielding) {
     // Timeout immediately when yielding is disabled.
@@ -1825,11 +1853,9 @@ function computeMsUntilTimeout(mostRecentEventTime: ExpirationTime) {
   const currentTimeMs: number = now();
   const timeElapsed = currentTimeMs - eventTimeMs;
 
-  // TODO: Account for the Just Noticeable Difference
-  const timeoutMs = 150;
-  const msUntilTimeout = timeoutMs - timeElapsed;
+  const msUntilTimeout = jnd(timeElapsed) - timeElapsed;
   // This is the value that is passed to `setTimeout`.
-  return msUntilTimeout < 0 ? 0 : msUntilTimeout;
+  return msUntilTimeout;
 }
 
 function checkForNestedUpdates() {

packages/react-reconciler/src/ReactFiberScheduler.old.js

@@ -1241,6 +1241,22 @@ function workLoop(isYieldy) {
   }
 }
 
+function jnd(timeElapsed: number) {
+  return timeElapsed < 120
+    ? 120
+    : timeElapsed < 480
+      ? 480
+      : timeElapsed < 1080
+        ? 1080
+        : timeElapsed < 1920
+          ? 1920
+          : timeElapsed < 3000
+            ? 3000
+            : timeElapsed < 4320
+              ? 4320
+              : Math.ceil(timeElapsed / 1960) * 1960;
+}
+
 function renderRoot(root: FiberRoot, isYieldy: boolean): void {
   invariant(
     !isWorking,
@@ -1518,10 +1534,11 @@ function renderRoot(root: FiberRoot, isYieldy: boolean): void {
     const currentTimeMs: number = now();
     const timeElapsed = currentTimeMs - eventTimeMs;
 
-    // TODO: Account for the Just Noticeable Difference
-    const timeoutMs = 150;
-    let msUntilTimeout = timeoutMs - timeElapsed;
-    msUntilTimeout = msUntilTimeout < 0 ? 0 : msUntilTimeout;
+    let msUntilTimeout = jnd(timeElapsed) - timeElapsed;
+    if (msUntilTimeout < 10) {
+      // Don't bother with a very short suspense time.
+      msUntilTimeout = 0;
+    }
 
     const rootExpirationTime = root.expirationTime;
     onSuspend(

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js

@@ -1668,6 +1668,126 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       </React.Fragment>,
     );
   });
+
+  it('suspends for longer if something took a long (CPU bound) time to render', async () => {
+    function Foo() {
+      Scheduler.yieldValue('Foo');
+      return (
+        <Suspense fallback={<Text text="Loading..." />}>
+          <AsyncText text="A" ms={5000} />
+        </Suspense>
+      );
+    }
+
+    ReactNoop.render(<Foo />);
+    Scheduler.advanceTime(100);
+    await advanceTimers(100);
+    // Start rendering
+    expect(Scheduler).toFlushAndYieldThrough(['Foo']);
+    // For some reason it took a long time to render Foo.
+    Scheduler.advanceTime(1250);
+    await advanceTimers(1250);
+    expect(Scheduler).toFlushAndYield([
+      // A suspends
+      'Suspend! [A]',
+      'Loading...',
+    ]);
+    // We're now suspended and we haven't shown anything yet.
+    expect(ReactNoop.getChildren()).toEqual([]);
+
+    // Flush some of the time
+    Scheduler.advanceTime(450);
+    await advanceTimers(450);
+    // Because we've already been waiting for so long we can
+    // wait a bit longer. Still nothing...
+    expect(Scheduler).toFlushWithoutYielding();
+    expect(ReactNoop.getChildren()).toEqual([]);
+
+    // Eventually we'll show the fallback.
+    Scheduler.advanceTime(500);
+    await advanceTimers(500);
+    // No need to rerender.
+    expect(Scheduler).toFlushWithoutYielding();
+    expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
+
+    // Flush the promise completely
+    Scheduler.advanceTime(4500);
+    await advanceTimers(4500);
+    // Renders successfully
+    expect(Scheduler).toHaveYielded(['Promise resolved [A]']);
+    expect(Scheduler).toFlushAndYield(['A']);
+    expect(ReactNoop.getChildren()).toEqual([span('A')]);
+  });
+
+  it('suspends for longer if a fallback has been shown for a long time', async () => {
+    function Foo() {
+      Scheduler.yieldValue('Foo');
+      return (
+        <Suspense fallback={<Text text="Loading..." />}>
+          <AsyncText text="A" ms={5000} />
+          <Suspense fallback={<Text text="Loading more..." />}>
+            <AsyncText text="B" ms={10000} />
+          </Suspense>
+        </Suspense>
+      );
+    }
+
+    ReactNoop.render(<Foo />);
+    // Start rendering
+    expect(Scheduler).toFlushAndYield([
+      'Foo',
+      // A suspends
+      'Suspend! [A]',
+      // B suspends
+      'Suspend! [B]',
+      'Loading more...',
+      'Loading...',
+    ]);
+    // We're now suspended and we haven't shown anything yet.
+    expect(ReactNoop.getChildren()).toEqual([]);
+
+    // Show the fallback.
+    Scheduler.advanceTime(400);
+    await advanceTimers(400);
+    expect(Scheduler).toFlushWithoutYielding();
+    expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
+
+    // Wait a long time.
+    Scheduler.advanceTime(5000);
+    await advanceTimers(5000);
+    expect(Scheduler).toHaveYielded(['Promise resolved [A]']);
+
+    // Retry with the new content.
+    expect(Scheduler).toFlushAndYield([
+      'A',
+      // B still suspends
+      'Suspend! [B]',
+      'Loading more...',
+    ]);
+    // Because we've already been waiting for so long we can
+    // wait a bit longer. Still nothing...
+    Scheduler.advanceTime(600);
+    await advanceTimers(600);
+    expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
+
+    // Eventually we'll show more content with inner fallback.
+    Scheduler.advanceTime(3000);
+    await advanceTimers(3000);
+    // No need to rerender.
+    expect(Scheduler).toFlushWithoutYielding();
+    expect(ReactNoop.getChildren()).toEqual([
+      span('A'),
+      span('Loading more...'),
+    ]);
+
+    // Flush the last promise completely
+    Scheduler.advanceTime(4500);
+    await advanceTimers(4500);
+    // Renders successfully
+    expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
+    expect(Scheduler).toFlushAndYield(['B']);
+    expect(ReactNoop.getChildren()).toEqual([span('A'), span('B')]);
+  });
 });
 
 // TODO: