diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js
index 56d9dd5b3c8da..da5385eb3b059 100644
--- a/packages/react-reconciler/src/ReactFiberCompleteWork.js
+++ b/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
diff --git a/packages/react-reconciler/src/ReactFiberScheduler.new.js b/packages/react-reconciler/src/ReactFiberScheduler.new.js
index 669f121c5433f..9c1065a568781 100644
--- a/packages/react-reconciler/src/ReactFiberScheduler.new.js
+++ b/packages/react-reconciler/src/ReactFiberScheduler.new.js
@@ -160,6 +160,8 @@ import {
} from 'shared/ReactErrorUtils';
import {onCommitRoot} from './ReactFiberDevToolsHook';
+const ceil = Math.ceil;
+
const {
ReactCurrentDispatcher,
ReactCurrentOwner,
@@ -893,10 +895,12 @@ function renderRoot(
// track any event times. That can happen if we retried but nothing switched
// from fallback to content. There's no reason to delay doing no work.
if (workInProgressRootMostRecentEventTime !== Sync) {
- const msUntilTimeout = computeMsUntilTimeout(
+ let msUntilTimeout = computeMsUntilTimeout(
workInProgressRootMostRecentEventTime,
+ expirationTime,
);
- 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,7 +1819,35 @@ export function resolveRetryThenable(boundaryFiber: Fiber, thenable: Thenable) {
retryTimedOutBoundary(boundaryFiber);
}
-function computeMsUntilTimeout(mostRecentEventTime: ExpirationTime) {
+// 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,
+ committedExpirationTime: ExpirationTime,
+) {
if (disableYielding) {
// Timeout immediately when yielding is disabled.
return 0;
@@ -1825,11 +1857,21 @@ 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;
+ let msUntilTimeout = jnd(timeElapsed) - timeElapsed;
+
+ // Compute the time until this render pass would expire.
+ const timeUntilExpirationMs =
+ expirationTimeToMs(committedExpirationTime) + initialTimeMs - currentTimeMs;
+
+ // Clamp the timeout to the expiration time.
+ // TODO: Once the event time is exact instead of inferred from expiration time
+ // we don't need this.
+ if (timeUntilExpirationMs < msUntilTimeout) {
+ msUntilTimeout = timeUntilExpirationMs;
+ }
+
// This is the value that is passed to `setTimeout`.
- return msUntilTimeout < 0 ? 0 : msUntilTimeout;
+ return msUntilTimeout;
}
function checkForNestedUpdates() {
diff --git a/packages/react-reconciler/src/ReactFiberScheduler.old.js b/packages/react-reconciler/src/ReactFiberScheduler.old.js
index 8614c2eb15de7..c4184fbb9465d 100644
--- a/packages/react-reconciler/src/ReactFiberScheduler.old.js
+++ b/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,22 @@ 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;
+ } else {
+ // Compute the time until this render pass would expire.
+ const timeUntilExpirationMs =
+ expirationTimeToMs(suspendedExpirationTime) +
+ originalStartTimeMs -
+ currentTimeMs;
+ // Clamp the timeout to the expiration time.
+ if (timeUntilExpirationMs < msUntilTimeout) {
+ msUntilTimeout = timeUntilExpirationMs;
+ }
+ }
const rootExpirationTime = root.expirationTime;
onSuspend(
diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
index 89f3f2a698c62..46ace8065bfca 100644
--- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
@@ -1668,6 +1668,162 @@ describe('ReactSuspenseWithNoopRenderer', () => {
,
);
});
+
+ it('suspends for longer if something took a long (CPU bound) time to render', async () => {
+ function Foo() {
+ Scheduler.yieldValue('Foo');
+ return (
+ }>
+
+
+ );
+ }
+
+ ReactNoop.render();
+ 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 (
+ }>
+
+ }>
+
+
+
+ );
+ }
+
+ ReactNoop.render();
+ // 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')]);
+ });
+
+ it('does not suspend for very long after a higher priority update', async () => {
+ function Foo() {
+ Scheduler.yieldValue('Foo');
+ return (
+ }>
+
+
+ );
+ }
+
+ ReactNoop.interactiveUpdates(() => ReactNoop.render());
+ expect(Scheduler).toFlushAndYieldThrough(['Foo']);
+
+ // Advance some time.
+ Scheduler.advanceTime(100);
+ await advanceTimers(100);
+
+ 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(500);
+ await advanceTimers(500);
+ // We should have already shown the fallback.
+ // When we wrote this test, we inferred the start time of high priority
+ // updates as way earlier in the past. This test ensures that we don't
+ // use this assumption to add a very long JND.
+ expect(Scheduler).toFlushWithoutYielding();
+ expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
+ });
});
// TODO: