From 64acfd5afab17ab35132007379a62f8d52bdb2dc Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Sat, 5 Apr 2025 00:33:53 -0400
Subject: [PATCH] [Bugfix] Infinite uDV loop in popstate event
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Found a bug that occurs during a specific combination of very subtle
implementation details.

It occurs sometimes (not always) when 1) a transition is scheduled
during a popstate event, and 2) as a result, a new value is passed to an
already-mounted useDeferredValue hook.

The fix is relatively straightforward, and I found it almost
immediately; it took a bit longer to figure out exactly how the scenario
occurred in production and create a test case to simulate it.

Rather than couple the test to the implementation details, I've chosen
to keep it as high-level as possible so that it doesn't break if the
details change. In the future, it might not be trigger the exact set of
internal circumstances anymore, but it could be useful for catching
similar bugs because it represents a realistic real world situation —
namely, switching tabs repeatedly in an app that uses useDeferredValue.
---
 .../src/__tests__/ReactDOMFiberAsync-test.js  | 68 +++++++++++++++++++
 .../react-reconciler/src/ReactFiberHooks.js   |  4 +-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
index 47ae48eb767bd..24a9c4308a2f9 100644
--- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
@@ -776,6 +776,74 @@ describe('ReactDOMFiberAsync', () => {
     });
   });
 
+  it('regression: useDeferredValue in popState leads to infinite deferral loop', async () => {
+    // At the time this test was written, it simulated a particular crash that
+    // was happened due to a combination of very subtle implementation details.
+    // Rather than couple this test to those implementation details, I've chosen
+    // to keep it as high-level as possible so that it doesn't break if the
+    // details change. In the future, it might not be trigger the exact set of
+    // internal circumstances anymore, but it could be useful for catching
+    // similar bugs because it represents a realistic real world situation —
+    // namely, switching tabs repeatedly in an app that uses useDeferredValue.
+    //
+    // But don't worry too much about why this test is written the way it is.
+
+    // Represents the browser's current location
+    let browserPathname = '/path/a';
+
+    let setPathname;
+    function App({initialPathname}) {
+      const [pathname, _setPathname] = React.useState('/path/a');
+      setPathname = _setPathname;
+
+      const deferredPathname = React.useDeferredValue(pathname);
+
+      // Attach a popstate listener on mount. Normally this would be in the
+      // in the router implementation.
+      React.useEffect(() => {
+        function onPopstate() {
+          React.startTransition(() => {
+            setPathname(browserPathname);
+          });
+        }
+        window.addEventListener('popstate', onPopstate);
+        return () => window.removeEventListener('popstate', onPopstate);
+      }, []);
+
+      return `Current:  ${pathname}\nDeferred: ${deferredPathname}`;
+    }
+
+    const root = ReactDOMClient.createRoot(container);
+    await act(async () => {
+      root.render(<App initialPathname={browserPathname} />);
+    });
+
+    // Simulate a series of popstate events that toggle back and forth between
+    // two locations. In the original regression case, a certain combination
+    // of transition lanes would cause React to fall into an infinite deferral
+    // loop — specifically, when the spawned by the useDeferredValue hook was
+    // assigned a "higher" bit value than the one assigned to the "popstate".
+
+    // For alignment reasons, call this once to advance the internal variable
+    // that assigns transition lanes. Because this is a no-op update, it will
+    // bump the counter, but it won't trigger the useDeferredValue hook.
+    setPathname(browserPathname);
+
+    // Trigger enough popstate events that the scenario occurs for every
+    // possible transition lane.
+    for (let i = 0; i < 50; i++) {
+      await act(async () => {
+        // Simulate a popstate event
+        browserPathname = browserPathname === '/path/a' ? '/path/b' : '/path/a';
+        const popStateEvent = new Event('popstate');
+        window.event = popStateEvent;
+        window.dispatchEvent(popStateEvent);
+        await waitForMicrotasks();
+        window.event = undefined;
+      });
+    }
+  });
+
   it('regression: infinite deferral loop caused by unstable useDeferredValue input', async () => {
     function Text({text}) {
       Scheduler.log(text);
diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
index 6499c4ec7f6be..967be9be94fae 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -3033,7 +3033,9 @@ function updateDeferredValueImpl<T>(
       return resultValue;
     }
 
-    const shouldDeferValue = !includesOnlyNonUrgentLanes(renderLanes);
+    const shouldDeferValue =
+      !includesOnlyNonUrgentLanes(renderLanes) &&
+      !includesSomeLane(renderLanes, DeferredLane);
     if (shouldDeferValue) {
       // This is an urgent update. Since the value has changed, keep using the
       // previous value and spawn a deferred render to update it later.