Bugfix: useDeferredValue loop during popstate transition (#27559)
During a popstate event, we attempt to render updates synchronously even if they are transitions, to preserve scroll position if possible. We do this by entangling the transition lane with the Sync lane. However, if rendering the transition spawns additional transition updates (e.g. a setState inside useEffect), there's no reason to render those synchronously, too. We should use the normal transition behavior. This fixes an issue where useDeferredValue during a popstate event would spawn a transition update that was itself also synchronous.
Andrew Clark committed
Oct 21, 2023 at 09:11 UTC
6db7f4209e6f32ebde298a0b7451710dd6aa3e19
2 files changed
+85
-1
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+22
-1
@@ -583,8 +583,29 @@ export function getCurrentEventPriority(): EventPriority {
583
return getEventPriority(currentEvent.type);
584
}
585
586
+let currentPopstateTransitionEvent: Event | null = null;
587
export function shouldAttemptEagerTransition(): boolean {
587
- return window.event && window.event.type === 'popstate';
588
+ const event = window.event;
589
+ if (event && event.type === 'popstate') {
590
+ // This is a popstate event. Attempt to render any transition during this
591
+ // event synchronously. Unless we already attempted during this event.
592
+ if (event === currentPopstateTransitionEvent) {
593
+ // We already attempted to render this popstate transition synchronously.
594
+ // Any subsequent attempts must have happened as the result of a derived
595
+ // update, like startTransition inside useEffect, or useDV. Switch back to
596
+ // the default behavior for all remaining transitions during the current
597
+ // popstate event.
598
+ return false;
599
+ } else {
600
+ // Cache the current event in case a derived transition is scheduled.
601
+ // (Refer to previous branch.)
602
+ currentPopstateTransitionEvent = event;
603
+ return true;
604
+ }
605
+ }
606
+ // We're not inside a popstate event.
607
+ currentPopstateTransitionEvent = null;
608
+ return false;
609
}
610
611
export const isPrimaryRenderer = true;
packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js
+63
@@ -746,5 +746,68 @@ describe('ReactDOMFiberAsync', () => {
746
});
747
assertLog([]);
748
expect(div.textContent).toBe('/path/b');
749
+ await act(() => {
750
+ root.unmount();
751
+ });
752
+ });
753
+
754
+ it('regression: infinite deferral loop caused by unstable useDeferredValue input', async () => {
755
+ function Text({text}) {
756
+ Scheduler.log(text);
757
+ return text;
758
+ }
759
+
760
+ let i = 0;
761
+ function App() {
762
+ const [pathname, setPathname] = React.useState('/path/a');
763
+ // This is an unstable input, so it will always cause a deferred render.
764
+ const {value: deferredPathname} = React.useDeferredValue({
765
+ value: pathname,
766
+ });
767
+ if (i++ > 100) {
768
+ throw new Error('Infinite loop detected');
769
+ }
770
+ React.useEffect(() => {
771
+ function onPopstate() {
772
+ React.startTransition(() => {
773
+ setPathname('/path/b');
774
+ });
775
+ }
776
+ window.addEventListener('popstate', onPopstate);
777
+ return () => window.removeEventListener('popstate', onPopstate);
778
+ }, []);
779
+
780
+ return <Text text={deferredPathname} />;
781
+ }
782
+
783
+ const root = ReactDOMClient.createRoot(container);
784
+ await act(() => {
785
+ root.render(<App />);
786
+ });
787
+ assertLog(['/path/a']);
788
+ expect(container.textContent).toBe('/path/a');
789
+
790
+ // Simulate a popstate event
791
+ await act(async () => {
792
+ const popStateEvent = new Event('popstate');
793
+
794
+ // Simulate a popstate event
795
+ window.event = popStateEvent;
796
+ window.dispatchEvent(popStateEvent);
797
+ await waitForMicrotasks();
798
+ window.event = undefined;
799
+
800
+ // The transition lane is attempted synchronously (in a microtask).
801
+ // Because the input to useDeferredValue is referentially unstable, it
802
+ // will spawn a deferred task at transition priority. However, even
803
+ // though it was spawned during a transition event, the spawned task
804
+ // not also be upgraded to sync.
805
+ assertLog(['/path/a']);
806
+ });
807
+ assertLog(['/path/b']);
808
+ expect(container.textContent).toBe('/path/b');
809
+ await act(() => {
810
+ root.unmount();
811
+ });
812
});
813
});