Fix: useDeferredValue initialValue suspends forever without switching…

… to final (#27888) Fixes a bug in the experimental `initialValue` option for `useDeferredValue` (added in #27500). If rendering the `initialValue` causes the tree to suspend, React should skip it and switch to rendering the final value instead. It should not wait for `initialValue` to resolve. This is not just an optimization, because in some cases the initial value may _never_ resolve — intentionally. For example, if the application does not provide an instant fallback state. This capability is, in fact, the primary motivation for the `initialValue` API. I mostly implemented this correctly in the original PR, but I missed some cases where it wasn't working: - If there's no Suspense boundary between the `useDeferredValue` hook and the component that suspends, and we're not in the shell of the transition (i.e. there's a parent Suspense boundary wrapping the `useDeferredValue` hook), the deferred task would get incorrectly dropped. - Similarly, if there's no Suspense boundary between the `useDeferredValue` hook and the component that suspends, and we're rendering a synchronous update, the deferred task would get incorrectly dropped. What these cases have in common is that it causes the `useDeferredValue` hook itself to be replaced by a Suspense fallback. The fix was the same for both. (It already worked in cases where there's no Suspense fallback at all, because those are handled differently, at the root.) The way I discovered this was when investigating a particular bug in Next.js that would happen during a 'popstate' transition (back/forward), but not during a regular navigation. That's because we render popstate transitions synchronously to preserve browser's scroll position — which in this case triggered the second scenario above. DiffTrain build for [f1039be](f1039be)
facebook · Jan 8, 2024 · f748f73 · f748f73
1 parent f2094ee
commit f748f73
Show file tree

Hide file tree

Showing 19 changed files with 1,891 additions and 1,133 deletions.
diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION
@@ -1 +1 @@
-c5b9375767e2c4102d7e5559d383523736f1c902
+f1039be4a48384e7e4b0a87d4d92c48e900053b9
diff --git a/compiled/facebook-www/React-dev.classic.js b/compiled/facebook-www/React-dev.classic.js
@@ -24,7 +24,7 @@ if (__DEV__) {
     ) {
       __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
     }
-    var ReactVersion = "18.3.0-www-classic-fc109ac4";
+    var ReactVersion = "18.3.0-www-classic-b43bd001";
 
     // ATTENTION
     // When adding new symbols to this file,

diff --git a/compiled/facebook-www/React-prod.classic.js b/compiled/facebook-www/React-prod.classic.js
@@ -587,4 +587,4 @@ exports.useSyncExternalStore = function (
 exports.useTransition = function () {
   return ReactCurrentDispatcher.current.useTransition();
 };
-exports.version = "18.3.0-www-classic-dd7a0299";
+exports.version = "18.3.0-www-classic-9e2eb6c9";
diff --git a/compiled/facebook-www/ReactART-dev.classic.js b/compiled/facebook-www/ReactART-dev.classic.js
@@ -66,7 +66,7 @@ if (__DEV__) {
       return self;
     }
 
-    var ReactVersion = "18.3.0-www-classic-cb4986de";
+    var ReactVersion = "18.3.0-www-classic-1401d50e";
 
     var LegacyRoot = 0;
     var ConcurrentRoot = 1;
@@ -544,6 +544,7 @@ if (__DEV__) {
 
     var ScheduleRetry = StoreConsistency;
     var ShouldSuspendCommit = Visibility;
+    var DidDefer = ContentReset;
     var LifecycleEffectMask =
       Passive$1 | Update | Callback | Ref | Snapshot | StoreConsistency; // Union of all commit flags (flags with the lifetime of a particular commit)
 
@@ -15591,9 +15592,26 @@ if (__DEV__) {
       return hasSuspenseListContext(suspenseContext, ForceSuspenseFallback);
     }
 
-    function getRemainingWorkInPrimaryTree(current, renderLanes) {
-      // TODO: Should not remove render lanes that were pinged during this render
-      return removeLanes(current.childLanes, renderLanes);
+    function getRemainingWorkInPrimaryTree(
+      current,
+      primaryTreeDidDefer,
+      renderLanes
+    ) {
+      var remainingLanes =
+        current !== null
+          ? removeLanes(current.childLanes, renderLanes)
+          : NoLanes;
+
+      if (primaryTreeDidDefer) {
+        // A useDeferredValue hook spawned a deferred task inside the primary tree.
+        // Ensure that we retry this component at the deferred priority.
+        // TODO: We could make this a per-subtree value instead of a global one.
+        // Would need to track it on the context stack somehow, similar to what
+        // we'd have to do for resumable contexts.
+        remainingLanes = mergeLanes(remainingLanes, peekDeferredLane());
+      }
+
+      return remainingLanes;
     }
 
     function updateSuspenseComponent(current, workInProgress, renderLanes) {
@@ -15613,7 +15631,12 @@ if (__DEV__) {
         // rendering the fallback children.
         showFallback = true;
         workInProgress.flags &= ~DidCapture;
-      } // OK, the next part is confusing. We're about to reconcile the Suspense
+      } // Check if the primary children spawned a deferred task (useDeferredValue)
+      // during the first pass.
+
+      var didPrimaryChildrenDefer =
+        (workInProgress.flags & DidDefer) !== NoFlags$1;
+      workInProgress.flags &= ~DidDefer; // OK, the next part is confusing. We're about to reconcile the Suspense
       // boundary's children. This involves some custom reconciliation logic. Two
       // main reasons this is so complicated.
       //
@@ -15651,6 +15674,11 @@ if (__DEV__) {
           var primaryChildFragment = workInProgress.child;
           primaryChildFragment.memoizedState =
             mountSuspenseOffscreenState(renderLanes);
+          primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
+            current,
+            didPrimaryChildrenDefer,
+            renderLanes
+          );
           workInProgress.memoizedState = SUSPENDED_MARKER;
 
           if (enableTransitionTracing) {
@@ -15691,6 +15719,11 @@ if (__DEV__) {
           var _primaryChildFragment = workInProgress.child;
           _primaryChildFragment.memoizedState =
             mountSuspenseOffscreenState(renderLanes);
+          _primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
+            current,
+            didPrimaryChildrenDefer,
+            renderLanes
+          );
           workInProgress.memoizedState = SUSPENDED_MARKER; // TODO: Transition Tracing is not yet implemented for CPU Suspense.
           // Since nothing actually suspended, there will nothing to ping this to
           // get it started back up to attempt the next item. While in terms of
@@ -15723,6 +15756,7 @@ if (__DEV__) {
               current,
               workInProgress,
               didSuspend,
+              didPrimaryChildrenDefer,
               nextProps,
               _dehydrated,
               prevState,
@@ -15786,6 +15820,7 @@ if (__DEV__) {
 
           _primaryChildFragment2.childLanes = getRemainingWorkInPrimaryTree(
             current,
+            didPrimaryChildrenDefer,
             renderLanes
           );
           workInProgress.memoizedState = SUSPENDED_MARKER;
@@ -16104,6 +16139,7 @@ if (__DEV__) {
       current,
       workInProgress,
       didSuspend,
+      didPrimaryChildrenDefer,
       nextProps,
       suspenseInstance,
       suspenseState,
@@ -16320,6 +16356,11 @@ if (__DEV__) {
           var _primaryChildFragment4 = workInProgress.child;
           _primaryChildFragment4.memoizedState =
             mountSuspenseOffscreenState(renderLanes);
+          _primaryChildFragment4.childLanes = getRemainingWorkInPrimaryTree(
+            current,
+            didPrimaryChildrenDefer,
+            renderLanes
+          );
           workInProgress.memoizedState = SUSPENDED_MARKER;
           return fallbackChildFragment;
         }
@@ -24766,10 +24807,22 @@ if (__DEV__) {
           // Everything else is spawned as a transition.
           workInProgressDeferredLane = requestTransitionLane();
         }
+      } // Mark the parent Suspense boundary so it knows to spawn the deferred lane.
+
+      var suspenseHandler = getSuspenseHandler();
+
+      if (suspenseHandler !== null) {
+        // TODO: As an optimization, we shouldn't entangle the lanes at the root; we
+        // can entangle them using the baseLanes of the Suspense boundary instead.
+        // We only need to do something special if there's no Suspense boundary.
+        suspenseHandler.flags |= DidDefer;
       }
 
       return workInProgressDeferredLane;
     }
+    function peekDeferredLane() {
+      return workInProgressDeferredLane;
+    }
     function scheduleUpdateOnFiber(root, fiber, lane) {
       {
         if (isRunningInsertionEffect) {
@@ -25387,7 +25440,7 @@ if (__DEV__) {
         // The render unwound without completing the tree. This happens in special
         // cases where need to exit the current render without producing a
         // consistent tree or committing.
-        markRootSuspended(root, lanes, NoLane);
+        markRootSuspended(root, lanes, workInProgressDeferredLane);
         ensureRootIsScheduled(root);
         return null;
       } // We now have a consistent tree. Because this is a sync render, we

diff --git a/compiled/facebook-www/ReactART-dev.modern.js b/compiled/facebook-www/ReactART-dev.modern.js
@@ -66,7 +66,7 @@ if (__DEV__) {
       return self;
     }
 
-    var ReactVersion = "18.3.0-www-modern-0a58ac99";
+    var ReactVersion = "18.3.0-www-modern-a32ad479";
 
     var LegacyRoot = 0;
     var ConcurrentRoot = 1;
@@ -544,6 +544,7 @@ if (__DEV__) {
 
     var ScheduleRetry = StoreConsistency;
     var ShouldSuspendCommit = Visibility;
+    var DidDefer = ContentReset;
     var LifecycleEffectMask =
       Passive$1 | Update | Callback | Ref | Snapshot | StoreConsistency; // Union of all commit flags (flags with the lifetime of a particular commit)
 
@@ -15270,9 +15271,26 @@ if (__DEV__) {
       return hasSuspenseListContext(suspenseContext, ForceSuspenseFallback);
     }
 
-    function getRemainingWorkInPrimaryTree(current, renderLanes) {
-      // TODO: Should not remove render lanes that were pinged during this render
-      return removeLanes(current.childLanes, renderLanes);
+    function getRemainingWorkInPrimaryTree(
+      current,
+      primaryTreeDidDefer,
+      renderLanes
+    ) {
+      var remainingLanes =
+        current !== null
+          ? removeLanes(current.childLanes, renderLanes)
+          : NoLanes;
+
+      if (primaryTreeDidDefer) {
+        // A useDeferredValue hook spawned a deferred task inside the primary tree.
+        // Ensure that we retry this component at the deferred priority.
+        // TODO: We could make this a per-subtree value instead of a global one.
+        // Would need to track it on the context stack somehow, similar to what
+        // we'd have to do for resumable contexts.
+        remainingLanes = mergeLanes(remainingLanes, peekDeferredLane());
+      }
+
+      return remainingLanes;
     }
 
     function updateSuspenseComponent(current, workInProgress, renderLanes) {
@@ -15292,7 +15310,12 @@ if (__DEV__) {
         // rendering the fallback children.
         showFallback = true;
         workInProgress.flags &= ~DidCapture;
-      } // OK, the next part is confusing. We're about to reconcile the Suspense
+      } // Check if the primary children spawned a deferred task (useDeferredValue)
+      // during the first pass.
+
+      var didPrimaryChildrenDefer =
+        (workInProgress.flags & DidDefer) !== NoFlags$1;
+      workInProgress.flags &= ~DidDefer; // OK, the next part is confusing. We're about to reconcile the Suspense
       // boundary's children. This involves some custom reconciliation logic. Two
       // main reasons this is so complicated.
       //
@@ -15330,6 +15353,11 @@ if (__DEV__) {
           var primaryChildFragment = workInProgress.child;
           primaryChildFragment.memoizedState =
             mountSuspenseOffscreenState(renderLanes);
+          primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
+            current,
+            didPrimaryChildrenDefer,
+            renderLanes
+          );
           workInProgress.memoizedState = SUSPENDED_MARKER;
 
           if (enableTransitionTracing) {
@@ -15370,6 +15398,11 @@ if (__DEV__) {
           var _primaryChildFragment = workInProgress.child;
           _primaryChildFragment.memoizedState =
             mountSuspenseOffscreenState(renderLanes);
+          _primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree(
+            current,
+            didPrimaryChildrenDefer,
+            renderLanes
+          );
           workInProgress.memoizedState = SUSPENDED_MARKER; // TODO: Transition Tracing is not yet implemented for CPU Suspense.
           // Since nothing actually suspended, there will nothing to ping this to
           // get it started back up to attempt the next item. While in terms of
@@ -15402,6 +15435,7 @@ if (__DEV__) {
               current,
               workInProgress,
               didSuspend,
+              didPrimaryChildrenDefer,
               nextProps,
               _dehydrated,
               prevState,
@@ -15465,6 +15499,7 @@ if (__DEV__) {
 
           _primaryChildFragment2.childLanes = getRemainingWorkInPrimaryTree(
             current,
+            didPrimaryChildrenDefer,
             renderLanes
           );
           workInProgress.memoizedState = SUSPENDED_MARKER;
@@ -15783,6 +15818,7 @@ if (__DEV__) {
       current,
       workInProgress,
       didSuspend,
+      didPrimaryChildrenDefer,
       nextProps,
       suspenseInstance,
       suspenseState,
@@ -15999,6 +16035,11 @@ if (__DEV__) {
           var _primaryChildFragment4 = workInProgress.child;
           _primaryChildFragment4.memoizedState =
             mountSuspenseOffscreenState(renderLanes);
+          _primaryChildFragment4.childLanes = getRemainingWorkInPrimaryTree(
+            current,
+            didPrimaryChildrenDefer,
+            renderLanes
+          );
           workInProgress.memoizedState = SUSPENDED_MARKER;
           return fallbackChildFragment;
         }
@@ -24410,10 +24451,22 @@ if (__DEV__) {
           // Everything else is spawned as a transition.
           workInProgressDeferredLane = requestTransitionLane();
         }
+      } // Mark the parent Suspense boundary so it knows to spawn the deferred lane.
+
+      var suspenseHandler = getSuspenseHandler();
+
+      if (suspenseHandler !== null) {
+        // TODO: As an optimization, we shouldn't entangle the lanes at the root; we
+        // can entangle them using the baseLanes of the Suspense boundary instead.
+        // We only need to do something special if there's no Suspense boundary.
+        suspenseHandler.flags |= DidDefer;
       }
 
       return workInProgressDeferredLane;
     }
+    function peekDeferredLane() {
+      return workInProgressDeferredLane;
+    }
     function scheduleUpdateOnFiber(root, fiber, lane) {
       {
         if (isRunningInsertionEffect) {
@@ -25031,7 +25084,7 @@ if (__DEV__) {
         // The render unwound without completing the tree. This happens in special
         // cases where need to exit the current render without producing a
         // consistent tree or committing.
-        markRootSuspended(root, lanes, NoLane);
+        markRootSuspended(root, lanes, workInProgressDeferredLane);
         ensureRootIsScheduled(root);
         return null;
       } // We now have a consistent tree. Because this is a sync render, we
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		c5b9375767e2c4102d7e5559d383523736f1c902
		f1039be4a48384e7e4b0a87d4d92c48e900053b9