Skip to content

Commit 7b59f84

Browse files
acdliteacdlite
committed
Fix Suspense throttling mechanism (#26802)
The throttling mechanism for fallbacks should apply to both their appearance _and_ disappearance. This was mostly addressed by #26611. See that PR for additional context. However, a flaw in the implementation is that we only update the the timestamp used for throttling when the fallback initially appears. We don't update it when the real content pops in. If lots of content in separate Suspense trees loads around the same time, you can still get jank. The issue is fixed by updating the throttling timestamp whenever the visibility of a fallback changes. Not just when it appears. DiffTrain build for [4bfcd02](4bfcd02)
1 parent 646a30e commit 7b59f84

18 files changed

+1287
-1179
lines changed

‎compiled/facebook-www/REVISION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
4cd7065665ea2cf33c306265c8d817904bb401ca
1+
4bfcd02b2cebcb390f5aff0d7747c60a55012d5d

‎compiled/facebook-www/React-dev.modern.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if (
2727
}
2828
"use strict";
2929

30-
var ReactVersion = "18.3.0-www-modern-6995898a";
30+
var ReactVersion = "18.3.0-www-modern-102b0f04";
3131

3232
// ATTENTION
3333
// When adding new symbols to this file,

‎compiled/facebook-www/ReactART-dev.classic.js‎

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
6969
return self;
7070
}
7171

72-
var ReactVersion = "18.3.0-www-classic-d297891e";
72+
var ReactVersion = "18.3.0-www-classic-ed32f0c6";
7373

7474
var LegacyRoot = 0;
7575
var ConcurrentRoot = 1;
@@ -22110,20 +22110,35 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
2211022110

2211122111
case SuspenseComponent: {
2211222112
recursivelyTraverseMutationEffects(root, finishedWork);
22113-
commitReconciliationEffects(finishedWork);
22113+
commitReconciliationEffects(finishedWork); // TODO: We should mark a flag on the Suspense fiber itself, rather than
22114+
// relying on the Offscreen fiber having a flag also being marked. The
22115+
// reason is that this offscreen fiber might not be part of the work-in-
22116+
// progress tree! It could have been reused from a previous render. This
22117+
// doesn't lead to incorrect behavior because we don't rely on the flag
22118+
// check alone; we also compare the states explicitly below. But for
22119+
// modeling purposes, we _should_ be able to rely on the flag check alone.
22120+
// So this is a bit fragile.
22121+
//
22122+
// Also, all this logic could/should move to the passive phase so it
22123+
// doesn't block paint.
22124+
2211422125
var offscreenFiber = finishedWork.child;
2211522126

2211622127
if (offscreenFiber.flags & Visibility) {
22117-
var newState = offscreenFiber.memoizedState;
22118-
var isHidden = newState !== null;
22119-
22120-
if (isHidden) {
22121-
var wasHidden =
22122-
offscreenFiber.alternate !== null &&
22123-
offscreenFiber.alternate.memoizedState !== null;
22124-
22125-
if (!wasHidden) {
22126-
// TODO: Move to passive phase
22128+
// Throttle the appearance and disappearance of Suspense fallbacks.
22129+
var isShowingFallback = finishedWork.memoizedState !== null;
22130+
var wasShowingFallback =
22131+
current !== null && current.memoizedState !== null;
22132+
22133+
if (alwaysThrottleRetries) {
22134+
if (isShowingFallback !== wasShowingFallback) {
22135+
// A fallback is either appearing or disappearing.
22136+
markCommitTimeOfFallback();
22137+
}
22138+
} else {
22139+
if (isShowingFallback && !wasShowingFallback) {
22140+
// Old behavior. Only mark when a fallback appears, not when
22141+
// it disappears.
2212722142
markCommitTimeOfFallback();
2212822143
}
2212922144
}
@@ -22154,20 +22169,18 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
2215422169
}
2215522170
}
2215622171

22157-
var _newState = finishedWork.memoizedState;
22158-
22159-
var _isHidden = _newState !== null;
22160-
22161-
var _wasHidden = current !== null && current.memoizedState !== null;
22172+
var newState = finishedWork.memoizedState;
22173+
var isHidden = newState !== null;
22174+
var wasHidden = current !== null && current.memoizedState !== null;
2216222175

2216322176
if (finishedWork.mode & ConcurrentMode) {
2216422177
// Before committing the children, track on the stack whether this
2216522178
// offscreen subtree was already hidden, so that we don't unmount the
2216622179
// effects again.
2216722180
var prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
2216822181
var prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
22169-
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || _isHidden;
22170-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || _wasHidden;
22182+
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || isHidden;
22183+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
2217122184
recursivelyTraverseMutationEffects(root, finishedWork);
2217222185
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2217322186
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
@@ -22188,21 +22201,21 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
2218822201
if (flags & Visibility) {
2218922202
// Track the current state on the Offscreen instance so we can
2219022203
// read it during an event
22191-
if (_isHidden) {
22204+
if (isHidden) {
2219222205
offscreenInstance._visibility &= ~OffscreenVisible;
2219322206
} else {
2219422207
offscreenInstance._visibility |= OffscreenVisible;
2219522208
}
2219622209

22197-
if (_isHidden) {
22210+
if (isHidden) {
2219822211
var isUpdate = current !== null;
2219922212
var wasHiddenByAncestorOffscreen =
2220022213
offscreenSubtreeIsHidden || offscreenSubtreeWasHidden; // Only trigger disapper layout effects if:
2220122214
// - This is an update, not first mount.
2220222215
// - This Offscreen was not hidden before.
2220322216
// - Ancestor Offscreen was not hidden in previous commit.
2220422217

22205-
if (isUpdate && !_wasHidden && !wasHiddenByAncestorOffscreen) {
22218+
if (isUpdate && !wasHidden && !wasHiddenByAncestorOffscreen) {
2220622219
if ((finishedWork.mode & ConcurrentMode) !== NoMode) {
2220722220
// Disappear the layout effects of all the children
2220822221
recursivelyTraverseDisappearLayoutEffects(finishedWork);
@@ -22213,7 +22226,7 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
2221322226
if (!isOffscreenManual(finishedWork)) {
2221422227
// TODO: This needs to run whenever there's an insertion or update
2221522228
// inside a hidden Offscreen tree.
22216-
hideOrUnhideAllChildren(finishedWork, _isHidden);
22229+
hideOrUnhideAllChildren(finishedWork, isHidden);
2221722230
}
2221822231
} // TODO: Move to passive phase
2221922232

@@ -24000,8 +24013,10 @@ var workInProgressRootPingedLanes = NoLanes; // Errors that are thrown during th
2400024013
var workInProgressRootConcurrentErrors = null; // These are errors that we recovered from without surfacing them to the UI.
2400124014
// We will log them once the tree commits.
2400224015

24003-
var workInProgressRootRecoverableErrors = null; // The most recent time we committed a fallback. This lets us ensure a train
24004-
// model where we don't commit new loading states in too quick succession.
24016+
var workInProgressRootRecoverableErrors = null; // The most recent time we either committed a fallback, or when a fallback was
24017+
// filled in with the resolved UI. This lets us throttle the appearance of new
24018+
// content as it streams in, to minimize jank.
24019+
// TODO: Think of a better name for this variable?
2400524020

2400624021
var globalMostRecentFallbackTime = 0;
2400724022
var FALLBACK_THROTTLE_MS = 500; // The absolute time for when we should start giving up on rendering

‎compiled/facebook-www/ReactART-dev.modern.js‎

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
6969
return self;
7070
}
7171

72-
var ReactVersion = "18.3.0-www-modern-2f2a7948";
72+
var ReactVersion = "18.3.0-www-modern-c4ac5f42";
7373

7474
var LegacyRoot = 0;
7575
var ConcurrentRoot = 1;
@@ -21775,20 +21775,35 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
2177521775

2177621776
case SuspenseComponent: {
2177721777
recursivelyTraverseMutationEffects(root, finishedWork);
21778-
commitReconciliationEffects(finishedWork);
21778+
commitReconciliationEffects(finishedWork); // TODO: We should mark a flag on the Suspense fiber itself, rather than
21779+
// relying on the Offscreen fiber having a flag also being marked. The
21780+
// reason is that this offscreen fiber might not be part of the work-in-
21781+
// progress tree! It could have been reused from a previous render. This
21782+
// doesn't lead to incorrect behavior because we don't rely on the flag
21783+
// check alone; we also compare the states explicitly below. But for
21784+
// modeling purposes, we _should_ be able to rely on the flag check alone.
21785+
// So this is a bit fragile.
21786+
//
21787+
// Also, all this logic could/should move to the passive phase so it
21788+
// doesn't block paint.
21789+
2177921790
var offscreenFiber = finishedWork.child;
2178021791

2178121792
if (offscreenFiber.flags & Visibility) {
21782-
var newState = offscreenFiber.memoizedState;
21783-
var isHidden = newState !== null;
21784-
21785-
if (isHidden) {
21786-
var wasHidden =
21787-
offscreenFiber.alternate !== null &&
21788-
offscreenFiber.alternate.memoizedState !== null;
21789-
21790-
if (!wasHidden) {
21791-
// TODO: Move to passive phase
21793+
// Throttle the appearance and disappearance of Suspense fallbacks.
21794+
var isShowingFallback = finishedWork.memoizedState !== null;
21795+
var wasShowingFallback =
21796+
current !== null && current.memoizedState !== null;
21797+
21798+
if (alwaysThrottleRetries) {
21799+
if (isShowingFallback !== wasShowingFallback) {
21800+
// A fallback is either appearing or disappearing.
21801+
markCommitTimeOfFallback();
21802+
}
21803+
} else {
21804+
if (isShowingFallback && !wasShowingFallback) {
21805+
// Old behavior. Only mark when a fallback appears, not when
21806+
// it disappears.
2179221807
markCommitTimeOfFallback();
2179321808
}
2179421809
}
@@ -21819,20 +21834,18 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
2181921834
}
2182021835
}
2182121836

21822-
var _newState = finishedWork.memoizedState;
21823-
21824-
var _isHidden = _newState !== null;
21825-
21826-
var _wasHidden = current !== null && current.memoizedState !== null;
21837+
var newState = finishedWork.memoizedState;
21838+
var isHidden = newState !== null;
21839+
var wasHidden = current !== null && current.memoizedState !== null;
2182721840

2182821841
if (finishedWork.mode & ConcurrentMode) {
2182921842
// Before committing the children, track on the stack whether this
2183021843
// offscreen subtree was already hidden, so that we don't unmount the
2183121844
// effects again.
2183221845
var prevOffscreenSubtreeIsHidden = offscreenSubtreeIsHidden;
2183321846
var prevOffscreenSubtreeWasHidden = offscreenSubtreeWasHidden;
21834-
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || _isHidden;
21835-
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || _wasHidden;
21847+
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden || isHidden;
21848+
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden || wasHidden;
2183621849
recursivelyTraverseMutationEffects(root, finishedWork);
2183721850
offscreenSubtreeWasHidden = prevOffscreenSubtreeWasHidden;
2183821851
offscreenSubtreeIsHidden = prevOffscreenSubtreeIsHidden;
@@ -21853,21 +21866,21 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
2185321866
if (flags & Visibility) {
2185421867
// Track the current state on the Offscreen instance so we can
2185521868
// read it during an event
21856-
if (_isHidden) {
21869+
if (isHidden) {
2185721870
offscreenInstance._visibility &= ~OffscreenVisible;
2185821871
} else {
2185921872
offscreenInstance._visibility |= OffscreenVisible;
2186021873
}
2186121874

21862-
if (_isHidden) {
21875+
if (isHidden) {
2186321876
var isUpdate = current !== null;
2186421877
var wasHiddenByAncestorOffscreen =
2186521878
offscreenSubtreeIsHidden || offscreenSubtreeWasHidden; // Only trigger disapper layout effects if:
2186621879
// - This is an update, not first mount.
2186721880
// - This Offscreen was not hidden before.
2186821881
// - Ancestor Offscreen was not hidden in previous commit.
2186921882

21870-
if (isUpdate && !_wasHidden && !wasHiddenByAncestorOffscreen) {
21883+
if (isUpdate && !wasHidden && !wasHiddenByAncestorOffscreen) {
2187121884
if ((finishedWork.mode & ConcurrentMode) !== NoMode) {
2187221885
// Disappear the layout effects of all the children
2187321886
recursivelyTraverseDisappearLayoutEffects(finishedWork);
@@ -21878,7 +21891,7 @@ function commitMutationEffectsOnFiber(finishedWork, root, lanes) {
2187821891
if (!isOffscreenManual(finishedWork)) {
2187921892
// TODO: This needs to run whenever there's an insertion or update
2188021893
// inside a hidden Offscreen tree.
21881-
hideOrUnhideAllChildren(finishedWork, _isHidden);
21894+
hideOrUnhideAllChildren(finishedWork, isHidden);
2188221895
}
2188321896
} // TODO: Move to passive phase
2188421897

@@ -23665,8 +23678,10 @@ var workInProgressRootPingedLanes = NoLanes; // Errors that are thrown during th
2366523678
var workInProgressRootConcurrentErrors = null; // These are errors that we recovered from without surfacing them to the UI.
2366623679
// We will log them once the tree commits.
2366723680

23668-
var workInProgressRootRecoverableErrors = null; // The most recent time we committed a fallback. This lets us ensure a train
23669-
// model where we don't commit new loading states in too quick succession.
23681+
var workInProgressRootRecoverableErrors = null; // The most recent time we either committed a fallback, or when a fallback was
23682+
// filled in with the resolved UI. This lets us throttle the appearance of new
23683+
// content as it streams in, to minimize jank.
23684+
// TODO: Think of a better name for this variable?
2367023685

2367123686
var globalMostRecentFallbackTime = 0;
2367223687
var FALLBACK_THROTTLE_MS = 500; // The absolute time for when we should start giving up on rendering

0 commit comments

Comments
 (0)