Skip to content

Commit

Permalink
Bugfix: Selective hydration triggers false update loop error (#27439)
Browse files Browse the repository at this point in the history
This adds a regression test and fix for a case where a sync update
triggers selective hydration, which then leads to a "Maximum update
depth exceeded" error, even though there was only a single update. This
happens when a single sync update flows into many sibling dehydrated
Suspense boundaries.

This fix is, if a commit was the result of selective hydration, we
should not increment the nested update count, because those renders
conceptually are not updates.

Ideally, they wouldn't even be in a separate commit — we should be able
to hydrate a tree and apply an update on top of it within the same
render phase. We could do this once we implement resumable context
stacks.

DiffTrain build for [d900fad](d900fad)
  • Loading branch information
acdlite committed Sep 29, 2023
1 parent ab2c406 commit c6d3f9a
Show file tree
Hide file tree
Showing 23 changed files with 249 additions and 149 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
13d0225c7d4715d98772a85d8deb26d244921287
d900fadbf9017063fecb2641b7e99303b82a6f17
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-classic-f8c6554b";
var ReactVersion = "18.3.0-www-classic-d42cdbcf";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -623,4 +623,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-classic-c257e462";
exports.version = "18.3.0-www-classic-f8e56db8";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -615,4 +615,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-1f62d21d";
exports.version = "18.3.0-www-modern-34d9e517";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-9df16c08";
exports.version = "18.3.0-www-modern-a77dd006";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
25 changes: 18 additions & 7 deletions compiled/facebook-www/ReactART-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-classic-bface95b";
var ReactVersion = "18.3.0-www-classic-6b865fa6";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -1588,9 +1588,9 @@ var DefaultHydrationLane =
var DefaultLane =
/* */
32;
var SyncUpdateLanes =
/* */
42;
var SyncUpdateLanes = enableUnifiedSyncLane
? SyncLane | InputContinuousLane | DefaultLane
: SyncLane;
var TransitionHydrationLane =
/* */
64;
Expand Down Expand Up @@ -1675,7 +1675,11 @@ var IdleLane =
536870912;
var OffscreenLane =
/* */
1073741824; // This function is used for the experimental timeline (react-devtools-timeline)
1073741824; // Any lane that might schedule an update. This is used to detect infinite
// update loops, so it doesn't include hydration lanes or retries.

var UpdateLanes =
SyncLane | InputContinuousLane | DefaultLane | TransitionLanes; // This function is used for the experimental timeline (react-devtools-timeline)
// It should be kept in sync with the Lanes values above.

function getLabelForLane(lane) {
Expand Down Expand Up @@ -26165,9 +26169,16 @@ function commitRootImpl(
flushPassiveEffects();
} // Read this again, since a passive effect might have updated it

remainingLanes = root.pendingLanes;
remainingLanes = root.pendingLanes; // Check if this render scheduled a cascading synchronous update. This is a
// heurstic to detect infinite update loops. We are intentionally excluding
// hydration lanes in this check, because render triggered by selective
// hydration is conceptually not an update.

if (includesSyncLane(remainingLanes)) {
if (
// Was the finished render the result of an update (not hydration)?
includesSomeLane(lanes, UpdateLanes) && // Did it schedule a sync update?
includesSomeLane(remainingLanes, SyncUpdateLanes)
) {
{
markNestedUpdateScheduled();
} // Count the number of times the root synchronously re-renders without
Expand Down
25 changes: 18 additions & 7 deletions compiled/facebook-www/ReactART-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-modern-87c144fd";
var ReactVersion = "18.3.0-www-modern-10178652";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -1585,9 +1585,9 @@ var DefaultHydrationLane =
var DefaultLane =
/* */
32;
var SyncUpdateLanes =
/* */
42;
var SyncUpdateLanes = enableUnifiedSyncLane
? SyncLane | InputContinuousLane | DefaultLane
: SyncLane;
var TransitionHydrationLane =
/* */
64;
Expand Down Expand Up @@ -1672,7 +1672,11 @@ var IdleLane =
536870912;
var OffscreenLane =
/* */
1073741824; // This function is used for the experimental timeline (react-devtools-timeline)
1073741824; // Any lane that might schedule an update. This is used to detect infinite
// update loops, so it doesn't include hydration lanes or retries.

var UpdateLanes =
SyncLane | InputContinuousLane | DefaultLane | TransitionLanes; // This function is used for the experimental timeline (react-devtools-timeline)
// It should be kept in sync with the Lanes values above.

function getLabelForLane(lane) {
Expand Down Expand Up @@ -25825,9 +25829,16 @@ function commitRootImpl(
flushPassiveEffects();
} // Read this again, since a passive effect might have updated it

remainingLanes = root.pendingLanes;
remainingLanes = root.pendingLanes; // Check if this render scheduled a cascading synchronous update. This is a
// heurstic to detect infinite update loops. We are intentionally excluding
// hydration lanes in this check, because render triggered by selective
// hydration is conceptually not an update.

if (includesSyncLane(remainingLanes)) {
if (
// Was the finished render the result of an update (not hydration)?
includesSomeLane(lanes, UpdateLanes) && // Did it schedule a sync update?
includesSomeLane(remainingLanes, SyncUpdateLanes)
) {
{
markNestedUpdateScheduled();
} // Count the number of times the root synchronously re-renders without
Expand Down
24 changes: 13 additions & 11 deletions compiled/facebook-www/ReactART-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,11 +393,12 @@ function clz32Fallback(x) {
x >>>= 0;
return 0 === x ? 32 : (31 - ((log(x) / LN2) | 0)) | 0;
}
var nextTransitionLane = 128,
var SyncUpdateLanes = enableUnifiedSyncLane ? 42 : 2,
nextTransitionLane = 128,
nextRetryLane = 8388608;
function getHighestPriorityLanes(lanes) {
if (enableUnifiedSyncLane) {
var pendingSyncLanes = lanes & 42;
var pendingSyncLanes = lanes & SyncUpdateLanes;
if (0 !== pendingSyncLanes) return pendingSyncLanes;
}
switch (lanes & -lanes) {
Expand Down Expand Up @@ -4812,7 +4813,8 @@ function updateDehydratedSuspenseComponent(
nextProps = workInProgressRoot;
if (null !== nextProps) {
didSuspend = renderLanes & -renderLanes;
if (enableUnifiedSyncLane && 0 !== (didSuspend & 42)) didSuspend = 1;
if (enableUnifiedSyncLane && 0 !== (didSuspend & SyncUpdateLanes))
didSuspend = 1;
else
switch (didSuspend) {
case 2:
Expand Down Expand Up @@ -8945,12 +8947,12 @@ function commitRootImpl(
finishedWork < recoverableErrors.length;
finishedWork++
)
(lanes = recoverableErrors[finishedWork]),
(remainingLanes = {
digest: lanes.digest,
componentStack: lanes.stack
(remainingLanes = recoverableErrors[finishedWork]),
(transitions = {
digest: remainingLanes.digest,
componentStack: remainingLanes.stack
}),
renderPriorityLevel(lanes.value, remainingLanes);
renderPriorityLevel(remainingLanes.value, transitions);
if (hasUncaughtError)
throw (
((hasUncaughtError = !1),
Expand All @@ -8962,7 +8964,7 @@ function commitRootImpl(
0 !== root.tag &&
flushPassiveEffects();
remainingLanes = root.pendingLanes;
0 !== (remainingLanes & 3)
0 !== (lanes & 8388522) && 0 !== (remainingLanes & SyncUpdateLanes)
? root === rootWithNestedUpdates
? nestedUpdateCount++
: ((nestedUpdateCount = 0), (rootWithNestedUpdates = root))
Expand Down Expand Up @@ -10105,7 +10107,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-classic-b3975372",
version: "18.3.0-www-classic-2b1e1230",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1304 = {
Expand Down Expand Up @@ -10136,7 +10138,7 @@ var internals$jscomp$inline_1304 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-classic-b3975372"
reconcilerVersion: "18.3.0-www-classic-2b1e1230"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1305 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
24 changes: 13 additions & 11 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,11 +277,12 @@ function clz32Fallback(x) {
x >>>= 0;
return 0 === x ? 32 : (31 - ((log(x) / LN2) | 0)) | 0;
}
var nextTransitionLane = 128,
var SyncUpdateLanes = enableUnifiedSyncLane ? 42 : 2,
nextTransitionLane = 128,
nextRetryLane = 8388608;
function getHighestPriorityLanes(lanes) {
if (enableUnifiedSyncLane) {
var pendingSyncLanes = lanes & 42;
var pendingSyncLanes = lanes & SyncUpdateLanes;
if (0 !== pendingSyncLanes) return pendingSyncLanes;
}
switch (lanes & -lanes) {
Expand Down Expand Up @@ -4571,7 +4572,8 @@ function updateDehydratedSuspenseComponent(
nextProps = workInProgressRoot;
if (null !== nextProps) {
didSuspend = renderLanes & -renderLanes;
if (enableUnifiedSyncLane && 0 !== (didSuspend & 42)) didSuspend = 1;
if (enableUnifiedSyncLane && 0 !== (didSuspend & SyncUpdateLanes))
didSuspend = 1;
else
switch (didSuspend) {
case 2:
Expand Down Expand Up @@ -8677,12 +8679,12 @@ function commitRootImpl(
finishedWork < recoverableErrors.length;
finishedWork++
)
(lanes = recoverableErrors[finishedWork]),
(remainingLanes = {
digest: lanes.digest,
componentStack: lanes.stack
(remainingLanes = recoverableErrors[finishedWork]),
(transitions = {
digest: remainingLanes.digest,
componentStack: remainingLanes.stack
}),
renderPriorityLevel(lanes.value, remainingLanes);
renderPriorityLevel(remainingLanes.value, transitions);
if (hasUncaughtError)
throw (
((hasUncaughtError = !1),
Expand All @@ -8694,7 +8696,7 @@ function commitRootImpl(
0 !== root.tag &&
flushPassiveEffects();
remainingLanes = root.pendingLanes;
0 !== (remainingLanes & 3)
0 !== (lanes & 8388522) && 0 !== (remainingLanes & SyncUpdateLanes)
? root === rootWithNestedUpdates
? nestedUpdateCount++
: ((nestedUpdateCount = 0), (rootWithNestedUpdates = root))
Expand Down Expand Up @@ -9770,7 +9772,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-modern-a4e8bc7c",
version: "18.3.0-www-modern-df7dbab9",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1284 = {
Expand Down Expand Up @@ -9801,7 +9803,7 @@ var internals$jscomp$inline_1284 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-a4e8bc7c"
reconcilerVersion: "18.3.0-www-modern-df7dbab9"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1285 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
25 changes: 18 additions & 7 deletions compiled/facebook-www/ReactDOM-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -1758,9 +1758,9 @@ var DefaultHydrationLane =
var DefaultLane =
/* */
32;
var SyncUpdateLanes =
/* */
42;
var SyncUpdateLanes = enableUnifiedSyncLane
? SyncLane | InputContinuousLane | DefaultLane
: SyncLane;
var TransitionHydrationLane =
/* */
64;
Expand Down Expand Up @@ -1845,7 +1845,11 @@ var IdleLane =
536870912;
var OffscreenLane =
/* */
1073741824; // This function is used for the experimental timeline (react-devtools-timeline)
1073741824; // Any lane that might schedule an update. This is used to detect infinite
// update loops, so it doesn't include hydration lanes or retries.

var UpdateLanes =
SyncLane | InputContinuousLane | DefaultLane | TransitionLanes; // This function is used for the experimental timeline (react-devtools-timeline)
// It should be kept in sync with the Lanes values above.

function getLabelForLane(lane) {
Expand Down Expand Up @@ -31705,9 +31709,16 @@ function commitRootImpl(
flushPassiveEffects();
} // Read this again, since a passive effect might have updated it

remainingLanes = root.pendingLanes;
remainingLanes = root.pendingLanes; // Check if this render scheduled a cascading synchronous update. This is a
// heurstic to detect infinite update loops. We are intentionally excluding
// hydration lanes in this check, because render triggered by selective
// hydration is conceptually not an update.

if (includesSyncLane(remainingLanes)) {
if (
// Was the finished render the result of an update (not hydration)?
includesSomeLane(lanes, UpdateLanes) && // Did it schedule a sync update?
includesSomeLane(remainingLanes, SyncUpdateLanes)
) {
{
markNestedUpdateScheduled();
} // Count the number of times the root synchronously re-renders without
Expand Down Expand Up @@ -33961,7 +33972,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-classic-babee8fb";
var ReactVersion = "18.3.0-www-classic-707e543f";

function createPortal$1(
children,
Expand Down
25 changes: 18 additions & 7 deletions compiled/facebook-www/ReactDOM-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -1110,9 +1110,9 @@ var DefaultHydrationLane =
var DefaultLane =
/* */
32;
var SyncUpdateLanes =
/* */
42;
var SyncUpdateLanes = enableUnifiedSyncLane
? SyncLane | InputContinuousLane | DefaultLane
: SyncLane;
var TransitionHydrationLane =
/* */
64;
Expand Down Expand Up @@ -1197,7 +1197,11 @@ var IdleLane =
536870912;
var OffscreenLane =
/* */
1073741824; // This function is used for the experimental timeline (react-devtools-timeline)
1073741824; // Any lane that might schedule an update. This is used to detect infinite
// update loops, so it doesn't include hydration lanes or retries.

var UpdateLanes =
SyncLane | InputContinuousLane | DefaultLane | TransitionLanes; // This function is used for the experimental timeline (react-devtools-timeline)
// It should be kept in sync with the Lanes values above.

function getLabelForLane(lane) {
Expand Down Expand Up @@ -31550,9 +31554,16 @@ function commitRootImpl(
flushPassiveEffects();
} // Read this again, since a passive effect might have updated it

remainingLanes = root.pendingLanes;
remainingLanes = root.pendingLanes; // Check if this render scheduled a cascading synchronous update. This is a
// heurstic to detect infinite update loops. We are intentionally excluding
// hydration lanes in this check, because render triggered by selective
// hydration is conceptually not an update.

if (includesSyncLane(remainingLanes)) {
if (
// Was the finished render the result of an update (not hydration)?
includesSomeLane(lanes, UpdateLanes) && // Did it schedule a sync update?
includesSomeLane(remainingLanes, SyncUpdateLanes)
) {
{
markNestedUpdateScheduled();
} // Count the number of times the root synchronously re-renders without
Expand Down Expand Up @@ -33806,7 +33817,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-modern-0bdd4f9b";
var ReactVersion = "18.3.0-www-modern-b8e372cd";

function createPortal$1(
children,
Expand Down
Loading

0 comments on commit c6d3f9a

Please sign in to comment.