Bugfix: Selective hydration triggers false update loop error (#27439)

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 commit d900fad.
facebook · Sep 29, 2023 · a49e06d · a49e06d
1 parent 97845e6
commit a49e06d
Show file tree

Hide file tree

Showing 13 changed files with 189 additions and 153 deletions.
diff --git a/...ook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-dev.js b/...ook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-dev.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<aab7866dd4ea4a33aa518692d1dfa02f>>
+ * @generated SignedSource<<3cf0c7070b82c229b406663b7dbae937>>
  */
 
 'use strict';
@@ -1073,6 +1073,7 @@ var DefaultHydrationLane =
 var DefaultLane =
   /*                     */
   32;
+var SyncUpdateLanes = SyncLane;
 var TransitionHydrationLane =
   /*                */
   64;
@@ -1157,7 +1158,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)
 var NoTimestamp = -1;
 var nextTransitionLane = TransitionLane1;
 var nextRetryLane = RetryLane1;
@@ -22064,9 +22069,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
@@ -23985,7 +23997,7 @@ function createFiberRoot(
   return root;
 }
 
-var ReactVersion = "18.3.0-canary-13d0225c7-20230928";
+var ReactVersion = "18.3.0-canary-d900fadbf-20230929";
 
 // Might add PROFILE later.
 

diff --git a/...ok-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-prod.js b/...ok-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-prod.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<9dc743c9a683670534bfd600e2d75ee4>>
+ * @generated SignedSource<<14e99fd9af6d1296fe24000cb0277c83>>
  */
 
 "use strict";
@@ -7302,12 +7302,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),
@@ -7319,7 +7319,7 @@ function commitRootImpl(
     0 !== root.tag &&
     flushPassiveEffects();
   remainingLanes = root.pendingLanes;
-  0 !== (remainingLanes & 3)
+  0 !== (lanes & 8388522) && 0 !== (remainingLanes & 2)
     ? root === rootWithNestedUpdates
       ? nestedUpdateCount++
       : ((nestedUpdateCount = 0), (rootWithNestedUpdates = root))
@@ -8623,7 +8623,7 @@ var devToolsConfig$jscomp$inline_1030 = {
     throw Error("TestRenderer does not support findFiberByHostInstance()");
   },
   bundleType: 0,
-  version: "18.3.0-canary-13d0225c7-20230928",
+  version: "18.3.0-canary-d900fadbf-20230929",
   rendererPackageName: "react-test-renderer"
 };
 var internals$jscomp$inline_1229 = {
@@ -8654,7 +8654,7 @@ var internals$jscomp$inline_1229 = {
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-canary-13d0225c7-20230928"
+  reconcilerVersion: "18.3.0-canary-d900fadbf-20230929"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_1230 = __REACT_DEVTOOLS_GLOBAL_HOOK__;

diff --git a/...source/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-profiling.js b/...source/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-profiling.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<429b5d46fdda6991b1590ed4f8ef58b2>>
+ * @generated SignedSource<<56dc6116bc4d9736f3dd6138f782afb8>>
  */
 
 "use strict";
@@ -7664,12 +7664,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),
@@ -7681,7 +7681,7 @@ function commitRootImpl(
     0 !== root.tag &&
     flushPassiveEffects();
   remainingLanes = root.pendingLanes;
-  0 !== (remainingLanes & 3)
+  0 !== (lanes & 8388522) && 0 !== (remainingLanes & 2)
     ? ((nestedUpdateScheduled = !0),
       root === rootWithNestedUpdates
         ? nestedUpdateCount++
@@ -9049,7 +9049,7 @@ var devToolsConfig$jscomp$inline_1072 = {
     throw Error("TestRenderer does not support findFiberByHostInstance()");
   },
   bundleType: 0,
-  version: "18.3.0-canary-13d0225c7-20230928",
+  version: "18.3.0-canary-d900fadbf-20230929",
   rendererPackageName: "react-test-renderer"
 };
 var internals$jscomp$inline_1270 = {
@@ -9080,7 +9080,7 @@ var internals$jscomp$inline_1270 = {
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-canary-13d0225c7-20230928"
+  reconcilerVersion: "18.3.0-canary-d900fadbf-20230929"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_1271 = __REACT_DEVTOOLS_GLOBAL_HOOK__;

diff --git a/compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-dev.js b/compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-dev.js
@@ -27,7 +27,7 @@ if (
 }
           "use strict";
 
-var ReactVersion = "18.3.0-canary-13d0225c7-20230928";
+var ReactVersion = "18.3.0-canary-d900fadbf-20230929";
 
 // ATTENTION
 // When adding new symbols to this file,

diff --git a/compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-prod.js b/compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-prod.js
@@ -616,4 +616,4 @@ exports.useSyncExternalStore = function (
 exports.useTransition = function () {
   return ReactCurrentDispatcher.current.useTransition();
 };
-exports.version = "18.3.0-canary-13d0225c7-20230928";
+exports.version = "18.3.0-canary-d900fadbf-20230929";
diff --git a/compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-profiling.js b/compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-profiling.js
@@ -619,7 +619,7 @@ exports.useSyncExternalStore = function (
 exports.useTransition = function () {
   return ReactCurrentDispatcher.current.useTransition();
 };
-exports.version = "18.3.0-canary-13d0225c7-20230928";
+exports.version = "18.3.0-canary-d900fadbf-20230929";
 
           /* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
 if (

diff --git a/compiled-rn/facebook-fbsource/xplat/js/react-native-github/Libraries/Renderer/REVISION b/compiled-rn/facebook-fbsource/xplat/js/react-native-github/Libraries/Renderer/REVISION
@@ -1 +1 @@
-13d0225c7d4715d98772a85d8deb26d244921287
+d900fadbf9017063fecb2641b7e99303b82a6f17
diff --git a/...rce/xplat/js/react-native-github/Libraries/Renderer/implementations/ReactFabric-dev.fb.js b/...rce/xplat/js/react-native-github/Libraries/Renderer/implementations/ReactFabric-dev.fb.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<f96d625b3a84e085323266384ba6cb78>>
+ * @generated SignedSource<<86fb5a18e030273bb12b79024c6073c2>>
  */
 
 'use strict';
@@ -3925,6 +3925,7 @@ var DefaultHydrationLane =
 var DefaultLane =
   /*                     */
   32;
+var SyncUpdateLanes = SyncLane;
 var TransitionHydrationLane =
   /*                */
   64;
@@ -4009,7 +4010,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) {
@@ -24973,9 +24978,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
@@ -27022,7 +27034,7 @@ function createFiberRoot(
   return root;
 }
 
-var ReactVersion = "18.3.0-canary-d111ec43";
+var ReactVersion = "18.3.0-canary-84876954";
 
 function createPortal$1(
   children,

diff --git a/...ce/xplat/js/react-native-github/Libraries/Renderer/implementations/ReactFabric-prod.fb.js b/...ce/xplat/js/react-native-github/Libraries/Renderer/implementations/ReactFabric-prod.fb.js
@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<797c622016220d3670cc8ec55829f5d2>>
+ * @generated SignedSource<<c369fd4070037f1dc7d616d80d10dcfd>>
  */
 
 "use strict";
@@ -8233,70 +8233,70 @@ function commitRootImpl(
   while (null !== rootWithPendingPassiveEffects);
   if (0 !== (executionContext & 6))
     throw Error("Should not already be working.");
-  transitions = root.finishedWork;
-  var lanes = root.finishedLanes;
-  if (null === transitions) return null;
+  var finishedWork = root.finishedWork;
+  transitions = root.finishedLanes;
+  if (null === finishedWork) return null;
   root.finishedWork = null;
   root.finishedLanes = 0;
-  if (transitions === root.current)
+  if (finishedWork === root.current)
     throw Error(
       "Cannot commit the same tree as before. This error is likely caused by a bug in React. Please file an issue."
     );
   root.callbackNode = null;
   root.callbackPriority = 0;
   root.cancelPendingCommit = null;
-  var remainingLanes = transitions.lanes | transitions.childLanes;
+  var remainingLanes = finishedWork.lanes | finishedWork.childLanes;
   remainingLanes |= concurrentlyUpdatedLanes;
   markRootFinished(root, remainingLanes);
   root === workInProgressRoot &&
     ((workInProgress = workInProgressRoot = null),
     (workInProgressRootRenderLanes = 0));
-  (0 === (transitions.subtreeFlags & 10256) &&
-    0 === (transitions.flags & 10256)) ||
+  (0 === (finishedWork.subtreeFlags & 10256) &&
+    0 === (finishedWork.flags & 10256)) ||
     rootDoesHavePassiveEffects ||
     ((rootDoesHavePassiveEffects = !0),
     scheduleCallback(NormalPriority, function () {
       flushPassiveEffects();
       return null;
     }));
-  remainingLanes = 0 !== (transitions.flags & 15990);
-  if (0 !== (transitions.subtreeFlags & 15990) || remainingLanes) {
+  remainingLanes = 0 !== (finishedWork.flags & 15990);
+  if (0 !== (finishedWork.subtreeFlags & 15990) || remainingLanes) {
     remainingLanes = ReactCurrentBatchConfig.transition;
     ReactCurrentBatchConfig.transition = null;
     var previousPriority = currentUpdatePriority;
     currentUpdatePriority = 2;
     var prevExecutionContext = executionContext;
     executionContext |= 4;
     ReactCurrentOwner.current = null;
-    commitBeforeMutationEffects(root, transitions);
-    commitMutationEffectsOnFiber(transitions, root);
-    root.current = transitions;
-    commitLayoutEffectOnFiber(root, transitions.alternate, transitions);
+    commitBeforeMutationEffects(root, finishedWork);
+    commitMutationEffectsOnFiber(finishedWork, root);
+    root.current = finishedWork;
+    commitLayoutEffectOnFiber(root, finishedWork.alternate, finishedWork);
     requestPaint();
     executionContext = prevExecutionContext;
     currentUpdatePriority = previousPriority;
     ReactCurrentBatchConfig.transition = remainingLanes;
-  } else root.current = transitions;
+  } else root.current = finishedWork;
   rootDoesHavePassiveEffects &&
     ((rootDoesHavePassiveEffects = !1),
     (rootWithPendingPassiveEffects = root),
-    (pendingPassiveEffectsLanes = lanes));
+    (pendingPassiveEffectsLanes = transitions));
   remainingLanes = root.pendingLanes;
   0 === remainingLanes && (legacyErrorBoundariesThatAlreadyFailed = null);
-  onCommitRoot(transitions.stateNode, renderPriorityLevel);
+  onCommitRoot(finishedWork.stateNode, renderPriorityLevel);
   ensureRootIsScheduled(root);
   if (null !== recoverableErrors)
     for (
-      renderPriorityLevel = root.onRecoverableError, transitions = 0;
-      transitions < recoverableErrors.length;
-      transitions++
+      renderPriorityLevel = root.onRecoverableError, finishedWork = 0;
+      finishedWork < recoverableErrors.length;
+      finishedWork++
     )
-      (lanes = recoverableErrors[transitions]),
-        (remainingLanes = {
-          digest: lanes.digest,
-          componentStack: lanes.stack
+      (remainingLanes = recoverableErrors[finishedWork]),
+        (previousPriority = {
+          digest: remainingLanes.digest,
+          componentStack: remainingLanes.stack
         }),
-        renderPriorityLevel(lanes.value, remainingLanes);
+        renderPriorityLevel(remainingLanes.value, previousPriority);
   if (hasUncaughtError)
     throw (
       ((hasUncaughtError = !1),
@@ -8308,7 +8308,7 @@ function commitRootImpl(
     0 !== root.tag &&
     flushPassiveEffects();
   remainingLanes = root.pendingLanes;
-  0 !== (remainingLanes & 3)
+  0 !== (transitions & 8388522) && 0 !== (remainingLanes & 2)
     ? root === rootWithNestedUpdates
       ? nestedUpdateCount++
       : ((nestedUpdateCount = 0), (rootWithNestedUpdates = root))
@@ -9428,7 +9428,7 @@ var roots = new Map(),
   devToolsConfig$jscomp$inline_1039 = {
     findFiberByHostInstance: getInstanceFromNode,
     bundleType: 0,
-    version: "18.3.0-canary-aa452ea5",
+    version: "18.3.0-canary-0ca674fe",
     rendererPackageName: "react-native-renderer",
     rendererConfig: {
       getInspectorDataForInstance: getInspectorDataForInstance,
@@ -9471,7 +9471,7 @@ var internals$jscomp$inline_1281 = {
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-canary-aa452ea5"
+  reconcilerVersion: "18.3.0-canary-0ca674fe"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
   var hook$jscomp$inline_1282 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		13d0225c7d4715d98772a85d8deb26d244921287
		d900fadbf9017063fecb2641b7e99303b82a6f17