@samitouri / QOS-React / commits / 6a7650c75c

[Bugfix] Infinite uDV loop in popstate event (#32821)

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.

Andrew Clark committed Apr 5, 2025 at 00:49 UTC 6a7650c75c1bc110517bd9b3eefdc66eadbb9cbf
2 files changed +71 -1
packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
+68
@@ -776,6 +776,74 @@ describe('ReactDOMFiberAsync', () => {
776 });
777 });
778
779 + it('regression: useDeferredValue in popState leads to infinite deferral loop', async () => {
780 + // At the time this test was written, it simulated a particular crash that
781 + // was happened due to a combination of very subtle implementation details.
782 + // Rather than couple this test to those implementation details, I've chosen
783 + // to keep it as high-level as possible so that it doesn't break if the
784 + // details change. In the future, it might not be trigger the exact set of
785 + // internal circumstances anymore, but it could be useful for catching
786 + // similar bugs because it represents a realistic real world situation —
787 + // namely, switching tabs repeatedly in an app that uses useDeferredValue.
788 + //
789 + // But don't worry too much about why this test is written the way it is.
790 +
791 + // Represents the browser's current location
792 + let browserPathname = '/path/a';
793 +
794 + let setPathname;
795 + function App({initialPathname}) {
796 + const [pathname, _setPathname] = React.useState('/path/a');
797 + setPathname = _setPathname;
798 +
799 + const deferredPathname = React.useDeferredValue(pathname);
800 +
801 + // Attach a popstate listener on mount. Normally this would be in the
802 + // in the router implementation.
803 + React.useEffect(() => {
804 + function onPopstate() {
805 + React.startTransition(() => {
806 + setPathname(browserPathname);
807 + });
808 + }
809 + window.addEventListener('popstate', onPopstate);
810 + return () => window.removeEventListener('popstate', onPopstate);
811 + }, []);
812 +
813 + return `Current: ${pathname}\nDeferred: ${deferredPathname}`;
814 + }
815 +
816 + const root = ReactDOMClient.createRoot(container);
817 + await act(async () => {
818 + root.render(<App initialPathname={browserPathname} />);
819 + });
820 +
821 + // Simulate a series of popstate events that toggle back and forth between
822 + // two locations. In the original regression case, a certain combination
823 + // of transition lanes would cause React to fall into an infinite deferral
824 + // loop — specifically, when the spawned by the useDeferredValue hook was
825 + // assigned a "higher" bit value than the one assigned to the "popstate".
826 +
827 + // For alignment reasons, call this once to advance the internal variable
828 + // that assigns transition lanes. Because this is a no-op update, it will
829 + // bump the counter, but it won't trigger the useDeferredValue hook.
830 + setPathname(browserPathname);
831 +
832 + // Trigger enough popstate events that the scenario occurs for every
833 + // possible transition lane.
834 + for (let i = 0; i < 50; i++) {
835 + await act(async () => {
836 + // Simulate a popstate event
837 + browserPathname = browserPathname === '/path/a' ? '/path/b' : '/path/a';
838 + const popStateEvent = new Event('popstate');
839 + window.event = popStateEvent;
840 + window.dispatchEvent(popStateEvent);
841 + await waitForMicrotasks();
842 + window.event = undefined;
843 + });
844 + }
845 + });
846 +
847 it('regression: infinite deferral loop caused by unstable useDeferredValue input', async () => {
848 function Text({text}) {
849 Scheduler.log(text);
packages/react-reconciler/src/ReactFiberHooks.js
+3 -1
@@ -3033,7 +3033,9 @@ function updateDeferredValueImpl<T>(
3033 return resultValue;
3034 }
3035
3036 - const shouldDeferValue = !includesOnlyNonUrgentLanes(renderLanes);
3036 + const shouldDeferValue =
3037 + !includesOnlyNonUrgentLanes(renderLanes) &&
3038 + !includesSomeLane(renderLanes, DeferredLane);
3039 if (shouldDeferValue) {
3040 // This is an urgent update. Since the value has changed, keep using the
3041 // previous value and spawn a deferred render to update it later.