Skip to content

Commit e3f1918

Browse files
authored
[Fiber] Adjust the suspensey image/css timeout based on already elapsed time (#34478)
Currently suspensey images doesn't account for how long we've already been waiting. This means that you can for example wait for 300ms for the throttle + 500ms for the images. If a Transition takes a while to complete you can also wait that time + an additional 500ms for the images. This tracks the start time of a Transition so that we can count the timeout starting from when the user interacted or when the last fallback committed (which is where the 300ms throttle is computed from). Creating a single timeline. This also moves the timeout to a central place which I'll use in a follow up.
1 parent e12b0bd commit e3f1918

File tree

9 files changed

+77
-28
lines changed

9 files changed

+77
-28
lines changed

packages/react-art/src/ReactFiberConfigART.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ export function suspendInstance(instance, type, props) {}
615615

616616
export function suspendOnActiveViewTransition(container) {}
617617

618-
export function waitForCommitToBeReady() {
618+
export function waitForCommitToBeReady(timeoutOffset) {
619619
return null;
620620
}
621621

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

Lines changed: 49 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5905,7 +5905,9 @@ export function preloadResource(resource: Resource): boolean {
59055905

59065906
type SuspendedState = {
59075907
stylesheets: null | Map<StylesheetResource, HoistableRoot>,
5908-
count: number,
5908+
count: number, // suspensey css and active view transitions
5909+
imgCount: number, // suspensey images
5910+
waitingForImages: boolean, // false when we're no longer blocking on images
59095911
unsuspend: null | (() => void),
59105912
};
59115913
let suspendedState: null | SuspendedState = null;
@@ -5914,6 +5916,8 @@ export function startSuspendingCommit(): void {
59145916
suspendedState = {
59155917
stylesheets: null,
59165918
count: 0,
5919+
imgCount: 0,
5920+
waitingForImages: true,
59175921
// We use a noop function when we begin suspending because if possible we want the
59185922
// waitfor step to finish synchronously. If it doesn't we'll return a function to
59195923
// provide the actual unsuspend function and that will get completed when the count
@@ -5922,6 +5926,8 @@ export function startSuspendingCommit(): void {
59225926
};
59235927
}
59245928

5929+
const SUSPENSEY_STYLESHEET_TIMEOUT = 60000;
5930+
59255931
const SUSPENSEY_IMAGE_TIMEOUT = 500;
59265932

59275933
export function suspendInstance(
@@ -5946,13 +5952,10 @@ export function suspendInstance(
59465952
// If this browser supports decode() API, we use it to suspend waiting on the image.
59475953
// The loading should have already started at this point, so it should be enough to
59485954
// just call decode() which should also wait for the data to finish loading.
5949-
state.count++;
5950-
const ping = onUnsuspend.bind(state);
5951-
Promise.race([
5952-
// $FlowFixMe[prop-missing]
5953-
instance.decode(),
5954-
new Promise(resolve => setTimeout(resolve, SUSPENSEY_IMAGE_TIMEOUT)),
5955-
]).then(ping, ping);
5955+
state.imgCount++;
5956+
const ping = onUnsuspendImg.bind(state);
5957+
// $FlowFixMe[prop-missing]
5958+
instance.decode().then(ping, ping);
59565959
}
59575960
}
59585961

@@ -6067,7 +6070,9 @@ export function suspendOnActiveViewTransition(rootContainer: Container): void {
60676070
activeViewTransition.finished.then(ping, ping);
60686071
}
60696072

6070-
export function waitForCommitToBeReady(): null | ((() => void) => () => void) {
6073+
export function waitForCommitToBeReady(
6074+
timeoutOffset: number,
6075+
): null | ((() => void) => () => void) {
60716076
if (suspendedState === null) {
60726077
throw new Error(
60736078
'Internal React Error: suspendedState null when it was expected to exists. Please report this as a React bug.',
@@ -6085,7 +6090,7 @@ export function waitForCommitToBeReady(): null | ((() => void) => () => void) {
60856090

60866091
// We need to check the count again because the inserted stylesheets may have led to new
60876092
// tasks to wait on.
6088-
if (state.count > 0) {
6093+
if (state.count > 0 || state.imgCount > 0) {
60896094
return commit => {
60906095
// We almost never want to show content before its styles have loaded. But
60916096
// eventually we will give up and allow unstyled content. So this number is
@@ -6102,37 +6107,62 @@ export function waitForCommitToBeReady(): null | ((() => void) => () => void) {
61026107
state.unsuspend = null;
61036108
unsuspend();
61046109
}
6105-
}, 60000); // one minute
6110+
}, SUSPENSEY_STYLESHEET_TIMEOUT + timeoutOffset);
6111+
6112+
const imgTimer = setTimeout(() => {
6113+
// We're no longer blocked on images. If CSS resolves after this we can commit.
6114+
state.waitingForImages = false;
6115+
if (state.count === 0) {
6116+
if (state.stylesheets) {
6117+
insertSuspendedStylesheets(state, state.stylesheets);
6118+
}
6119+
if (state.unsuspend) {
6120+
const unsuspend = state.unsuspend;
6121+
state.unsuspend = null;
6122+
unsuspend();
6123+
}
6124+
}
6125+
}, SUSPENSEY_IMAGE_TIMEOUT + timeoutOffset);
61066126

61076127
state.unsuspend = commit;
61086128

61096129
return () => {
61106130
state.unsuspend = null;
61116131
clearTimeout(stylesheetTimer);
6132+
clearTimeout(imgTimer);
61126133
};
61136134
};
61146135
}
61156136
return null;
61166137
}
61176138

6118-
function onUnsuspend(this: SuspendedState) {
6119-
this.count--;
6120-
if (this.count === 0) {
6121-
if (this.stylesheets) {
6139+
function checkIfFullyUnsuspended(state: SuspendedState) {
6140+
if (state.count === 0 && (state.imgCount === 0 || !state.waitingForImages)) {
6141+
if (state.stylesheets) {
61226142
// If we haven't actually inserted the stylesheets yet we need to do so now before starting the commit.
61236143
// The reason we do this after everything else has finished is because we want to have all the stylesheets
61246144
// load synchronously right before mutating. Ideally the new styles will cause a single recalc only on the
61256145
// new tree. When we filled up stylesheets we only inlcuded stylesheets with matching media attributes so we
61266146
// wait for them to load before actually continuing. We expect this to increase the count above zero
6127-
insertSuspendedStylesheets(this, this.stylesheets);
6128-
} else if (this.unsuspend) {
6129-
const unsuspend = this.unsuspend;
6130-
this.unsuspend = null;
6147+
insertSuspendedStylesheets(state, state.stylesheets);
6148+
} else if (state.unsuspend) {
6149+
const unsuspend = state.unsuspend;
6150+
state.unsuspend = null;
61316151
unsuspend();
61326152
}
61336153
}
61346154
}
61356155

6156+
function onUnsuspend(this: SuspendedState) {
6157+
this.count--;
6158+
checkIfFullyUnsuspended(this);
6159+
}
6160+
6161+
function onUnsuspendImg(this: SuspendedState) {
6162+
this.imgCount--;
6163+
checkIfFullyUnsuspended(this);
6164+
}
6165+
61366166
// We use a value that is type distinct from precedence to track which one is last.
61376167
// This ensures there is no collision with user defined precedences. Normally we would
61386168
// just track this in module scope but since the precedences are tracked per HoistableRoot

packages/react-native-renderer/src/ReactFiberConfigFabric.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ export function suspendInstance(
612612

613613
export function suspendOnActiveViewTransition(container: Container): void {}
614614

615-
export function waitForCommitToBeReady(): null {
615+
export function waitForCommitToBeReady(timeoutOffset: number): null {
616616
return null;
617617
}
618618

packages/react-native-renderer/src/ReactFiberConfigNative.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ export function suspendInstance(
788788

789789
export function suspendOnActiveViewTransition(container: Container): void {}
790790

791-
export function waitForCommitToBeReady(): null {
791+
export function waitForCommitToBeReady(timeoutOffset: number): null {
792792
return null;
793793
}
794794

packages/react-noop-renderer/src/createReactNoop.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
361361
}
362362
}
363363

364-
function waitForCommitToBeReady():
365-
| ((commit: () => mixed) => () => void)
366-
| null {
364+
function waitForCommitToBeReady(
365+
timeoutOffset: number,
366+
): ((commit: () => mixed) => () => void) | null {
367367
const subscription = suspenseyCommitSubscription;
368368
if (subscription !== null) {
369369
suspenseyCommitSubscription = null;

packages/react-reconciler/src/ReactFiberTransition.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {createCursor, push, pop} from './ReactFiberStack';
2828
import {
2929
getWorkInProgressRoot,
3030
getWorkInProgressTransitions,
31+
markTransitionStarted,
3132
} from './ReactFiberWorkLoop';
3233
import {
3334
createCache,
@@ -79,6 +80,7 @@ ReactSharedInternals.S = function onStartTransitionFinishForReconciler(
7980
transition: Transition,
8081
returnValue: mixed,
8182
) {
83+
markTransitionStarted();
8284
if (
8385
typeof returnValue === 'object' &&
8486
returnValue !== null &&

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,11 @@ let didIncludeCommitPhaseUpdate: boolean = false;
475475
// content as it streams in, to minimize jank.
476476
// TODO: Think of a better name for this variable?
477477
let globalMostRecentFallbackTime: number = 0;
478+
// Track the most recent time we started a new Transition. This lets us apply
479+
// heuristics like the suspensey image timeout based on how long we've waited
480+
// already.
481+
let globalMostRecentTransitionTime: number = 0;
482+
478483
const FALLBACK_THROTTLE_MS: number = 300;
479484

480485
// The absolute time for when we should start giving up on rendering
@@ -1500,10 +1505,18 @@ function commitRootWhenReady(
15001505
suspendOnActiveViewTransition(root.containerInfo);
15011506
}
15021507
}
1508+
// For timeouts we use the previous fallback commit for retries and
1509+
// the start time of the transition for transitions. This offset
1510+
// represents the time already passed.
1511+
const timeoutOffset = includesOnlyRetries(lanes)
1512+
? globalMostRecentFallbackTime - now()
1513+
: includesOnlyTransitions(lanes)
1514+
? globalMostRecentTransitionTime - now()
1515+
: 0;
15031516
// At the end, ask the renderer if it's ready to commit, or if we should
15041517
// suspend. If it's not ready, it will return a callback to subscribe to
15051518
// a ready event.
1506-
const schedulePendingCommit = waitForCommitToBeReady();
1519+
const schedulePendingCommit = waitForCommitToBeReady(timeoutOffset);
15071520
if (schedulePendingCommit !== null) {
15081521
// NOTE: waitForCommitToBeReady returns a subscribe function so that we
15091522
// only allocate a function if the commit isn't ready yet. The other
@@ -2284,6 +2297,10 @@ export function markRenderDerivedCause(fiber: Fiber): void {
22842297
}
22852298
}
22862299

2300+
export function markTransitionStarted() {
2301+
globalMostRecentTransitionTime = now();
2302+
}
2303+
22872304
export function markCommitTimeOfFallback() {
22882305
globalMostRecentFallbackTime = now();
22892306
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ describe('ReactFiberHostContext', () => {
109109
startSuspendingCommit() {},
110110
suspendInstance(instance, type, props) {},
111111
suspendOnActiveViewTransition(container) {},
112-
waitForCommitToBeReady() {
112+
waitForCommitToBeReady(timeoutOffset: number) {
113113
return null;
114114
},
115115
supportsMutation: true,

packages/react-test-renderer/src/ReactFiberConfigTestHost.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ export function suspendInstance(
571571

572572
export function suspendOnActiveViewTransition(container: Container): void {}
573573

574-
export function waitForCommitToBeReady(): null {
574+
export function waitForCommitToBeReady(timeoutOffset: number): null {
575575
return null;
576576
}
577577

0 commit comments

Comments
 (0)