Fix: Update while suspended fails to interrupt (#26739)

This fixes a bug with `use` where if you update a component that's currently suspended, React will sometimes mistake it for a render phase update. This happens because we don't reset `currentlyRenderingFiber` until the suspended is unwound. And with `use`, that can happen asynchronously, most commonly when the work loop is suspended during a transition. The fix is to make sure `currentlyRenderingFiber` is only set when we're in the middle of rendering, which used to be true until `use` was introduced. More specifically this means clearing `currentlyRenderingFiber` when something throws and setting it again when we resume work. In many cases, this bug will fail "gracefully" because the update is still added to the queue; it's not dropped completely. It's also somewhat rare because it has to be the exact same component that's currently suspended. But it's still a bug. I wrote a regression test that shows a sync update failing to interrupt a suspended component. DiffTrain build for commit 18282f8.
facebook · Apr 28, 2023 · bdec677 · bdec677
1 parent fcc18f8
commit bdec677
Show file tree

Hide file tree

Showing 13 changed files with 214 additions and 166 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<<147e07431dc4defee6d2e9faf584292a>>
+ * @generated SignedSource<<a56cd14b45443ff19095298bed47faaf>>
  */
 
 'use strict';
@@ -6927,6 +6927,7 @@ function renderWithHooksAgain(workInProgress, Component, props, secondArg) {
   //
   // Keep rendering in a loop for as long as render phase updates continue to
   // be scheduled. Use a counter to prevent infinite loops.
+  currentlyRenderingFiber$1 = workInProgress;
   var numberOfReRenders = 0;
   var children;
 
@@ -6994,11 +6995,12 @@ function resetHooksAfterThrow() {
   //
   // It should only reset things like the current dispatcher, to prevent hooks
   // from being called outside of a component.
-  // We can assume the previous dispatcher is always this one, since we set it
+  currentlyRenderingFiber$1 = null; // We can assume the previous dispatcher is always this one, since we set it
   // at the beginning of the render phase and there's no re-entrance.
+
   ReactCurrentDispatcher$1.current = ContextOnlyDispatcher;
 }
-function resetHooksOnUnwind() {
+function resetHooksOnUnwind(workInProgress) {
   if (didScheduleRenderPhaseUpdate) {
     // There were render phase updates. These are only valid for this render
     // phase, which we are now aborting. Remove the updates from the queues so
@@ -7008,7 +7010,7 @@ function resetHooksOnUnwind() {
     // Only reset the updates from the queue if it has a clone. If it does
     // not have a clone, that means it wasn't processed, and the updates were
     // scheduled before we entered the render phase.
-    var hook = currentlyRenderingFiber$1.memoizedState;
+    var hook = workInProgress.memoizedState;
 
     while (hook !== null) {
       var queue = hook.queue;
@@ -20773,7 +20775,7 @@ function resetWorkInProgressStack() {
   } else {
     // Work-in-progress is in suspended state. Reset the work loop and unwind
     // both the suspended fiber and all its parents.
-    resetSuspendedWorkLoopOnUnwind();
+    resetSuspendedWorkLoopOnUnwind(workInProgress);
     interruptedWork = workInProgress;
   }
 
@@ -20830,10 +20832,10 @@ function prepareFreshStack(root, lanes) {
   return rootWorkInProgress;
 }
 
-function resetSuspendedWorkLoopOnUnwind() {
+function resetSuspendedWorkLoopOnUnwind(fiber) {
   // Reset module-level state that was set during the render phase.
   resetContextDependencies();
-  resetHooksOnUnwind();
+  resetHooksOnUnwind(fiber);
   resetChildReconcilerOnUnwind();
 }
 
@@ -21488,7 +21490,7 @@ function replaySuspendedUnitOfWork(unitOfWork) {
       // is to reuse uncached promises, but we happen to know that the only
       // promises that a host component might suspend on are definitely cached
       // because they are controlled by us. So don't bother.
-      resetHooksOnUnwind(); // Fallthrough to the next branch.
+      resetHooksOnUnwind(unitOfWork); // Fallthrough to the next branch.
     }
 
     default: {
@@ -21534,7 +21536,7 @@ function throwAndUnwindWorkLoop(unitOfWork, thrownValue) {
   //
   // Return to the normal work loop. This will unwind the stack, and potentially
   // result in showing a fallback.
-  resetSuspendedWorkLoopOnUnwind();
+  resetSuspendedWorkLoopOnUnwind(unitOfWork);
   var returnFiber = unitOfWork.return;
 
   if (returnFiber === null || workInProgressRoot === null) {
@@ -23892,7 +23894,7 @@ function createFiberRoot(
   return root;
 }
 
-var ReactVersion = "18.3.0-next-540bab085-20230426";
+var ReactVersion = "18.3.0-next-18282f881-20230428";
 
 // 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<<1d48b6135e400c479a77aa3a6d0aa557>>
+ * @generated SignedSource<<36182b4efb97f53e0d4f613caa8813e2>>
  */
 
 "use strict";
@@ -2224,6 +2224,7 @@ function finishRenderingHooks() {
     );
 }
 function renderWithHooksAgain(workInProgress, Component, props, secondArg) {
+  currentlyRenderingFiber$1 = workInProgress;
   var numberOfReRenders = 0;
   do {
     didScheduleRenderPhaseUpdateDuringThisPass && (thenableState = null);
@@ -2246,12 +2247,16 @@ function bailoutHooks(current, workInProgress, lanes) {
   workInProgress.flags &= -2053;
   current.lanes &= ~lanes;
 }
-function resetHooksOnUnwind() {
+function resetHooksOnUnwind(workInProgress) {
   if (didScheduleRenderPhaseUpdate) {
-    for (var hook = currentlyRenderingFiber$1.memoizedState; null !== hook; ) {
-      var queue = hook.queue;
+    for (
+      workInProgress = workInProgress.memoizedState;
+      null !== workInProgress;
+
+    ) {
+      var queue = workInProgress.queue;
       null !== queue && (queue.pending = null);
-      hook = hook.next;
+      workInProgress = workInProgress.next;
     }
     didScheduleRenderPhaseUpdate = !1;
   }
@@ -6681,8 +6686,9 @@ function resetWorkInProgressStack() {
     if (0 === workInProgressSuspendedReason)
       var interruptedWork = workInProgress.return;
     else
-      resetContextDependencies(),
-        resetHooksOnUnwind(),
+      (interruptedWork = workInProgress),
+        resetContextDependencies(),
+        resetHooksOnUnwind(interruptedWork),
         (thenableState$1 = null),
         (thenableIndexCounter$1 = 0),
         (interruptedWork = workInProgress);
@@ -6720,6 +6726,7 @@ function prepareFreshStack(root, lanes) {
   return root;
 }
 function handleThrow(root, thrownValue) {
+  currentlyRenderingFiber$1 = null;
   ReactCurrentDispatcher$1.current = ContextOnlyDispatcher;
   ReactCurrentOwner.current = null;
   thrownValue === SuspenseException
@@ -6974,7 +6981,7 @@ function replaySuspendedUnitOfWork(unitOfWork) {
       );
       break;
     case 5:
-      resetHooksOnUnwind();
+      resetHooksOnUnwind(unitOfWork);
     default:
       unwindInterruptedWork(current, unitOfWork),
         (unitOfWork = workInProgress =
@@ -6989,7 +6996,7 @@ function replaySuspendedUnitOfWork(unitOfWork) {
 }
 function throwAndUnwindWorkLoop(unitOfWork, thrownValue) {
   resetContextDependencies();
-  resetHooksOnUnwind();
+  resetHooksOnUnwind(unitOfWork);
   thenableState$1 = null;
   thenableIndexCounter$1 = 0;
   var returnFiber = unitOfWork.return;
@@ -8596,19 +8603,19 @@ function wrapFiber(fiber) {
     fiberToWrapper.set(fiber, wrapper));
   return wrapper;
 }
-var devToolsConfig$jscomp$inline_1022 = {
+var devToolsConfig$jscomp$inline_1024 = {
   findFiberByHostInstance: function () {
     throw Error("TestRenderer does not support findFiberByHostInstance()");
   },
   bundleType: 0,
-  version: "18.3.0-next-540bab085-20230426",
+  version: "18.3.0-next-18282f881-20230428",
   rendererPackageName: "react-test-renderer"
 };
-var internals$jscomp$inline_1207 = {
-  bundleType: devToolsConfig$jscomp$inline_1022.bundleType,
-  version: devToolsConfig$jscomp$inline_1022.version,
-  rendererPackageName: devToolsConfig$jscomp$inline_1022.rendererPackageName,
-  rendererConfig: devToolsConfig$jscomp$inline_1022.rendererConfig,
+var internals$jscomp$inline_1209 = {
+  bundleType: devToolsConfig$jscomp$inline_1024.bundleType,
+  version: devToolsConfig$jscomp$inline_1024.version,
+  rendererPackageName: devToolsConfig$jscomp$inline_1024.rendererPackageName,
+  rendererConfig: devToolsConfig$jscomp$inline_1024.rendererConfig,
   overrideHookState: null,
   overrideHookStateDeletePath: null,
   overrideHookStateRenamePath: null,
@@ -8625,26 +8632,26 @@ var internals$jscomp$inline_1207 = {
     return null === fiber ? null : fiber.stateNode;
   },
   findFiberByHostInstance:
-    devToolsConfig$jscomp$inline_1022.findFiberByHostInstance ||
+    devToolsConfig$jscomp$inline_1024.findFiberByHostInstance ||
     emptyFindFiberByHostInstance,
   findHostInstancesForRefresh: null,
   scheduleRefresh: null,
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-next-540bab085-20230426"
+  reconcilerVersion: "18.3.0-next-18282f881-20230428"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
-  var hook$jscomp$inline_1208 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
+  var hook$jscomp$inline_1210 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
   if (
-    !hook$jscomp$inline_1208.isDisabled &&
-    hook$jscomp$inline_1208.supportsFiber
+    !hook$jscomp$inline_1210.isDisabled &&
+    hook$jscomp$inline_1210.supportsFiber
   )
     try {
-      (rendererID = hook$jscomp$inline_1208.inject(
-        internals$jscomp$inline_1207
+      (rendererID = hook$jscomp$inline_1210.inject(
+        internals$jscomp$inline_1209
       )),
-        (injectedHook = hook$jscomp$inline_1208);
+        (injectedHook = hook$jscomp$inline_1210);
     } catch (err) {}
 }
 exports._Scheduler = Scheduler;

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<<1f1b211c71736978a204aa9af530fa47>>
+ * @generated SignedSource<<54c4cf0ba74fa6cfb2cf017a817e526a>>
  */
 
 "use strict";
@@ -2244,6 +2244,7 @@ function finishRenderingHooks() {
     );
 }
 function renderWithHooksAgain(workInProgress, Component, props, secondArg) {
+  currentlyRenderingFiber$1 = workInProgress;
   var numberOfReRenders = 0;
   do {
     didScheduleRenderPhaseUpdateDuringThisPass && (thenableState = null);
@@ -2266,12 +2267,16 @@ function bailoutHooks(current, workInProgress, lanes) {
   workInProgress.flags &= -2053;
   current.lanes &= ~lanes;
 }
-function resetHooksOnUnwind() {
+function resetHooksOnUnwind(workInProgress) {
   if (didScheduleRenderPhaseUpdate) {
-    for (var hook = currentlyRenderingFiber$1.memoizedState; null !== hook; ) {
-      var queue = hook.queue;
+    for (
+      workInProgress = workInProgress.memoizedState;
+      null !== workInProgress;
+
+    ) {
+      var queue = workInProgress.queue;
       null !== queue && (queue.pending = null);
-      hook = hook.next;
+      workInProgress = workInProgress.next;
     }
     didScheduleRenderPhaseUpdate = !1;
   }
@@ -7021,8 +7026,9 @@ function resetWorkInProgressStack() {
     if (0 === workInProgressSuspendedReason)
       var interruptedWork = workInProgress.return;
     else
-      resetContextDependencies(),
-        resetHooksOnUnwind(),
+      (interruptedWork = workInProgress),
+        resetContextDependencies(),
+        resetHooksOnUnwind(interruptedWork),
         (thenableState$1 = null),
         (thenableIndexCounter$1 = 0),
         (interruptedWork = workInProgress);
@@ -7060,6 +7066,7 @@ function prepareFreshStack(root, lanes) {
   return root;
 }
 function handleThrow(root, thrownValue) {
+  currentlyRenderingFiber$1 = null;
   ReactCurrentDispatcher$1.current = ContextOnlyDispatcher;
   ReactCurrentOwner.current = null;
   thrownValue === SuspenseException
@@ -7325,7 +7332,7 @@ function replaySuspendedUnitOfWork(unitOfWork) {
       );
       break;
     case 5:
-      resetHooksOnUnwind();
+      resetHooksOnUnwind(unitOfWork);
     default:
       unwindInterruptedWork(current, unitOfWork),
         (unitOfWork = workInProgress =
@@ -7341,7 +7348,7 @@ function replaySuspendedUnitOfWork(unitOfWork) {
 }
 function throwAndUnwindWorkLoop(unitOfWork, thrownValue) {
   resetContextDependencies();
-  resetHooksOnUnwind();
+  resetHooksOnUnwind(unitOfWork);
   thenableState$1 = null;
   thenableIndexCounter$1 = 0;
   var returnFiber = unitOfWork.return;
@@ -9022,19 +9029,19 @@ function wrapFiber(fiber) {
     fiberToWrapper.set(fiber, wrapper));
   return wrapper;
 }
-var devToolsConfig$jscomp$inline_1064 = {
+var devToolsConfig$jscomp$inline_1066 = {
   findFiberByHostInstance: function () {
     throw Error("TestRenderer does not support findFiberByHostInstance()");
   },
   bundleType: 0,
-  version: "18.3.0-next-540bab085-20230426",
+  version: "18.3.0-next-18282f881-20230428",
   rendererPackageName: "react-test-renderer"
 };
-var internals$jscomp$inline_1248 = {
-  bundleType: devToolsConfig$jscomp$inline_1064.bundleType,
-  version: devToolsConfig$jscomp$inline_1064.version,
-  rendererPackageName: devToolsConfig$jscomp$inline_1064.rendererPackageName,
-  rendererConfig: devToolsConfig$jscomp$inline_1064.rendererConfig,
+var internals$jscomp$inline_1250 = {
+  bundleType: devToolsConfig$jscomp$inline_1066.bundleType,
+  version: devToolsConfig$jscomp$inline_1066.version,
+  rendererPackageName: devToolsConfig$jscomp$inline_1066.rendererPackageName,
+  rendererConfig: devToolsConfig$jscomp$inline_1066.rendererConfig,
   overrideHookState: null,
   overrideHookStateDeletePath: null,
   overrideHookStateRenamePath: null,
@@ -9051,26 +9058,26 @@ var internals$jscomp$inline_1248 = {
     return null === fiber ? null : fiber.stateNode;
   },
   findFiberByHostInstance:
-    devToolsConfig$jscomp$inline_1064.findFiberByHostInstance ||
+    devToolsConfig$jscomp$inline_1066.findFiberByHostInstance ||
     emptyFindFiberByHostInstance,
   findHostInstancesForRefresh: null,
   scheduleRefresh: null,
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-next-540bab085-20230426"
+  reconcilerVersion: "18.3.0-next-18282f881-20230428"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
-  var hook$jscomp$inline_1249 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
+  var hook$jscomp$inline_1251 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
   if (
-    !hook$jscomp$inline_1249.isDisabled &&
-    hook$jscomp$inline_1249.supportsFiber
+    !hook$jscomp$inline_1251.isDisabled &&
+    hook$jscomp$inline_1251.supportsFiber
   )
     try {
-      (rendererID = hook$jscomp$inline_1249.inject(
-        internals$jscomp$inline_1248
+      (rendererID = hook$jscomp$inline_1251.inject(
+        internals$jscomp$inline_1250
       )),
-        (injectedHook = hook$jscomp$inline_1249);
+        (injectedHook = hook$jscomp$inline_1251);
     } catch (err) {}
 }
 exports._Scheduler = Scheduler;

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-next-540bab085-20230426";
+var ReactVersion = "18.3.0-next-18282f881-20230428";
 
 // 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
@@ -639,4 +639,4 @@ exports.useSyncExternalStore = function (
   );
 };
 exports.useTransition = useTransition;
-exports.version = "18.3.0-next-540bab085-20230426";
+exports.version = "18.3.0-next-18282f881-20230428";
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
@@ -642,7 +642,7 @@ exports.useSyncExternalStore = function (
   );
 };
 exports.useTransition = useTransition;
-exports.version = "18.3.0-next-540bab085-20230426";
+exports.version = "18.3.0-next-18282f881-20230428";
 
           /* 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 @@
-540bab085d571789f4562565eebfd0db9f36345c
+18282f881dae106ebf6240aa52c8c02fe7c8d6f2