Skip to content

Commit 4c78ac0

Browse files
authored
Track Event Time as the Start Time for Suspense (#15358)
* Track the earliest event time in this render Rebase * Track the time of the fallback being shown as an event time When we switch back from fallback to content, we made progress and we track the time from when we showed the fallback in the first place as the last time we made progress. * Don't retry if synchronous * Only suspend when we switch to fallback mode This ensures that we don't resuspend unnecessarily if we're just retrying the same exact boundary again. We can still unnecessarily suspend for nested boundaries. * Rename timedOutAt to fallbackExpirationTime * Account for suspense in devtools suspense test
1 parent 875d05d commit 4c78ac0

14 files changed

+200
-228
lines changed

packages/react-debug-tools/src/__tests__/ReactDevToolsHooksIntegration-test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,10 @@ describe('React hooks DevTools integration', () => {
251251
</div>,
252252
{unstable_isConcurrent: true},
253253
);
254+
254255
expect(Scheduler).toFlushAndYield([]);
256+
// Ensure we timeout any suspense time.
257+
jest.advanceTimersByTime(1000);
255258
const fiber = renderer.root._currentFiber().child;
256259
if (__DEV__) {
257260
// First render was locked

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1413,7 +1413,8 @@ function updateSuspenseComponent(
14131413
// Something in this boundary's subtree already suspended. Switch to
14141414
// rendering the fallback children.
14151415
nextState = {
1416-
timedOutAt: nextState !== null ? nextState.timedOutAt : NoWork,
1416+
fallbackExpirationTime:
1417+
nextState !== null ? nextState.fallbackExpirationTime : NoWork,
14171418
};
14181419
nextDidTimeout = true;
14191420
workInProgress.effectTag &= ~DidCapture;

packages/react-reconciler/src/ReactFiberCommitWork.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ import invariant from 'shared/invariant';
6363
import warningWithoutStack from 'shared/warningWithoutStack';
6464
import warning from 'shared/warning';
6565

66-
import {NoWork} from './ReactFiberExpirationTime';
66+
import {
67+
NoWork,
68+
computeAsyncExpirationNoBucket,
69+
} from './ReactFiberExpirationTime';
6770
import {onCommitUnmount} from './ReactFiberDevToolsHook';
6871
import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
6972
import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
@@ -1272,11 +1275,15 @@ function commitSuspenseComponent(finishedWork: Fiber) {
12721275
} else {
12731276
newDidTimeout = true;
12741277
primaryChildParent = finishedWork.child;
1275-
if (newState.timedOutAt === NoWork) {
1278+
if (newState.fallbackExpirationTime === NoWork) {
12761279
// If the children had not already timed out, record the time.
12771280
// This is used to compute the elapsed time during subsequent
12781281
// attempts to render the children.
1279-
newState.timedOutAt = requestCurrentTime();
1282+
// We model this as a normal pri expiration time since that's
1283+
// how we infer start time for updates.
1284+
newState.fallbackExpirationTime = computeAsyncExpirationNoBucket(
1285+
requestCurrentTime(),
1286+
);
12801287
}
12811288
}
12821289

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type {
1818
ChildSet,
1919
} from './ReactFiberHostConfig';
2020
import type {ReactEventComponentInstance} from 'shared/ReactTypes';
21+
import type {SuspenseState} from './ReactFiberSuspenseComponent';
2122

2223
import {
2324
IndeterminateComponent,
@@ -42,6 +43,7 @@ import {
4243
EventComponent,
4344
EventTarget,
4445
} from 'shared/ReactWorkTags';
46+
import {ConcurrentMode, NoContext} from './ReactTypeOfMode';
4547
import {
4648
Placement,
4749
Ref,
@@ -92,6 +94,7 @@ import {
9294
enableSuspenseServerRenderer,
9395
enableEventAPI,
9496
} from 'shared/ReactFeatureFlags';
97+
import {markRenderEventTime, renderDidSuspend} from './ReactFiberScheduler';
9598

9699
function markUpdate(workInProgress: Fiber) {
97100
// Tag the fiber with an update effect. This turns a Placement into
@@ -665,7 +668,7 @@ function completeWork(
665668
case ForwardRef:
666669
break;
667670
case SuspenseComponent: {
668-
const nextState = workInProgress.memoizedState;
671+
const nextState: null | SuspenseState = workInProgress.memoizedState;
669672
if ((workInProgress.effectTag & DidCapture) !== NoEffect) {
670673
// Something suspended. Re-render with the fallback children.
671674
workInProgress.expirationTime = renderExpirationTime;
@@ -674,34 +677,58 @@ function completeWork(
674677
}
675678

676679
const nextDidTimeout = nextState !== null;
677-
const prevDidTimeout = current !== null && current.memoizedState !== null;
678-
680+
let prevDidTimeout = false;
679681
if (current === null) {
680682
// In cases where we didn't find a suitable hydration boundary we never
681683
// downgraded this to a DehydratedSuspenseComponent, but we still need to
682684
// pop the hydration state since we might be inside the insertion tree.
683685
popHydrationState(workInProgress);
684-
} else if (!nextDidTimeout && prevDidTimeout) {
685-
// We just switched from the fallback to the normal children. Delete
686-
// the fallback.
687-
// TODO: Would it be better to store the fallback fragment on
688-
// the stateNode during the begin phase?
689-
const currentFallbackChild: Fiber | null = (current.child: any).sibling;
690-
if (currentFallbackChild !== null) {
691-
// Deletions go at the beginning of the return fiber's effect list
692-
const first = workInProgress.firstEffect;
693-
if (first !== null) {
694-
workInProgress.firstEffect = currentFallbackChild;
695-
currentFallbackChild.nextEffect = first;
696-
} else {
697-
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChild;
698-
currentFallbackChild.nextEffect = null;
686+
} else {
687+
const prevState: null | SuspenseState = current.memoizedState;
688+
prevDidTimeout = prevState !== null;
689+
if (!nextDidTimeout && prevState !== null) {
690+
// We just switched from the fallback to the normal children.
691+
692+
// Mark the event time of the switching from fallback to normal children,
693+
// based on the start of when we first showed the fallback. This time
694+
// was given a normal pri expiration time at the time it was shown.
695+
const fallbackExpirationTimeExpTime: ExpirationTime =
696+
prevState.fallbackExpirationTime;
697+
markRenderEventTime(fallbackExpirationTimeExpTime);
698+
699+
// Delete the fallback.
700+
// TODO: Would it be better to store the fallback fragment on
701+
// the stateNode during the begin phase?
702+
const currentFallbackChild: Fiber | null = (current.child: any)
703+
.sibling;
704+
if (currentFallbackChild !== null) {
705+
// Deletions go at the beginning of the return fiber's effect list
706+
const first = workInProgress.firstEffect;
707+
if (first !== null) {
708+
workInProgress.firstEffect = currentFallbackChild;
709+
currentFallbackChild.nextEffect = first;
710+
} else {
711+
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChild;
712+
currentFallbackChild.nextEffect = null;
713+
}
714+
currentFallbackChild.effectTag = Deletion;
699715
}
700-
currentFallbackChild.effectTag = Deletion;
716+
}
717+
}
718+
719+
if (nextDidTimeout && !prevDidTimeout) {
720+
// If this subtreee is running in concurrent mode we can suspend,
721+
// otherwise we won't suspend.
722+
// TODO: This will still suspend a synchronous tree if anything
723+
// in the concurrent tree already suspended during this render.
724+
// This is a known bug.
725+
if ((workInProgress.mode & ConcurrentMode) !== NoContext) {
726+
renderDidSuspend();
701727
}
702728
}
703729

704730
if (supportsPersistence) {
731+
// TODO: Only schedule updates if not prevDidTimeout.
705732
if (nextDidTimeout) {
706733
// If this boundary just timed out, schedule an effect to attach a
707734
// retry listener to the proimse. This flag is also used to hide the
@@ -710,6 +737,7 @@ function completeWork(
710737
}
711738
}
712739
if (supportsMutation) {
740+
// TODO: Only schedule updates if these values are non equal, i.e. it changed.
713741
if (nextDidTimeout || prevDidTimeout) {
714742
// If this boundary just timed out, schedule an effect to attach a
715743
// retry listener to the proimse. This flag is also used to hide the

packages/react-reconciler/src/ReactFiberExpirationTime.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ export function computeAsyncExpiration(
7070
);
7171
}
7272

73+
// Same as computeAsyncExpiration but without the bucketing logic. This is
74+
// used to compute timestamps instead of actual expiration times.
75+
export function computeAsyncExpirationNoBucket(
76+
currentTime: ExpirationTime,
77+
): ExpirationTime {
78+
return currentTime - LOW_PRIORITY_EXPIRATION / UNIT_SIZE;
79+
}
80+
7381
// We intentionally set a higher expiration time for interactive updates in
7482
// dev than in production.
7583
//

packages/react-reconciler/src/ReactFiberHooks.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
flushPassiveEffects,
3535
requestCurrentTime,
3636
warnIfNotCurrentlyActingUpdatesInDev,
37+
markRenderEventTime,
3738
} from './ReactFiberScheduler';
3839

3940
import invariant from 'shared/invariant';
@@ -718,6 +719,16 @@ function updateReducer<S, I, A>(
718719
remainingExpirationTime = updateExpirationTime;
719720
}
720721
} else {
722+
// This update does have sufficient priority.
723+
724+
// Mark the event time of this update as relevant to this render pass.
725+
// TODO: This should ideally use the true event time of this update rather than
726+
// its priority which is a derived and not reverseable value.
727+
// TODO: We should skip this update if it was already committed but currently
728+
// we have no way of detecting the difference between a committed and suspended
729+
// update here.
730+
markRenderEventTime(updateExpirationTime);
731+
721732
// Process this update.
722733
if (update.eagerReducer === reducer) {
723734
// If this update was processed eagerly, and its reducer matches the

packages/react-reconciler/src/ReactFiberPendingPriority.js

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -219,23 +219,6 @@ function clearPing(root, completedTime) {
219219
}
220220
}
221221

222-
export function findEarliestOutstandingPriorityLevel(
223-
root: FiberRoot,
224-
renderExpirationTime: ExpirationTime,
225-
): ExpirationTime {
226-
let earliestExpirationTime = renderExpirationTime;
227-
228-
const earliestPendingTime = root.earliestPendingTime;
229-
const earliestSuspendedTime = root.earliestSuspendedTime;
230-
if (earliestPendingTime > earliestExpirationTime) {
231-
earliestExpirationTime = earliestPendingTime;
232-
}
233-
if (earliestSuspendedTime > earliestExpirationTime) {
234-
earliestExpirationTime = earliestSuspendedTime;
235-
}
236-
return earliestExpirationTime;
237-
}
238-
239222
export function didExpireAtExpirationTime(
240223
root: FiberRoot,
241224
currentTime: ExpirationTime,

packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
computeExpirationForFiber as computeExpirationForFiber_old,
1515
captureCommitPhaseError as captureCommitPhaseError_old,
1616
onUncaughtError as onUncaughtError_old,
17+
markRenderEventTime as markRenderEventTime_old,
1718
renderDidSuspend as renderDidSuspend_old,
1819
renderDidError as renderDidError_old,
1920
pingSuspendedRoot as pingSuspendedRoot_old,
@@ -34,14 +35,14 @@ import {
3435
computeUniqueAsyncExpiration as computeUniqueAsyncExpiration_old,
3536
flushPassiveEffects as flushPassiveEffects_old,
3637
warnIfNotCurrentlyActingUpdatesInDev as warnIfNotCurrentlyActingUpdatesInDev_old,
37-
inferStartTimeFromExpirationTime as inferStartTimeFromExpirationTime_old,
3838
} from './ReactFiberScheduler.old';
3939

4040
import {
4141
requestCurrentTime as requestCurrentTime_new,
4242
computeExpirationForFiber as computeExpirationForFiber_new,
4343
captureCommitPhaseError as captureCommitPhaseError_new,
4444
onUncaughtError as onUncaughtError_new,
45+
markRenderEventTime as markRenderEventTime_new,
4546
renderDidSuspend as renderDidSuspend_new,
4647
renderDidError as renderDidError_new,
4748
pingSuspendedRoot as pingSuspendedRoot_new,
@@ -62,7 +63,6 @@ import {
6263
computeUniqueAsyncExpiration as computeUniqueAsyncExpiration_new,
6364
flushPassiveEffects as flushPassiveEffects_new,
6465
warnIfNotCurrentlyActingUpdatesInDev as warnIfNotCurrentlyActingUpdatesInDev_new,
65-
inferStartTimeFromExpirationTime as inferStartTimeFromExpirationTime_new,
6666
} from './ReactFiberScheduler.new';
6767

6868
export const requestCurrentTime = enableNewScheduler
@@ -77,6 +77,9 @@ export const captureCommitPhaseError = enableNewScheduler
7777
export const onUncaughtError = enableNewScheduler
7878
? onUncaughtError_new
7979
: onUncaughtError_old;
80+
export const markRenderEventTime = enableNewScheduler
81+
? markRenderEventTime_new
82+
: markRenderEventTime_old;
8083
export const renderDidSuspend = enableNewScheduler
8184
? renderDidSuspend_new
8285
: renderDidSuspend_old;
@@ -133,9 +136,6 @@ export const flushPassiveEffects = enableNewScheduler
133136
export const warnIfNotCurrentlyActingUpdatesInDev = enableNewScheduler
134137
? warnIfNotCurrentlyActingUpdatesInDev_new
135138
: warnIfNotCurrentlyActingUpdatesInDev_old;
136-
export const inferStartTimeFromExpirationTime = enableNewScheduler
137-
? inferStartTimeFromExpirationTime_new
138-
: inferStartTimeFromExpirationTime_old;
139139

140140
export type Thenable = {
141141
then(resolve: () => mixed, reject?: () => mixed): void | Thenable,

0 commit comments

Comments
 (0)