Skip to content

Commit 6a7650c

Browse files
authored
[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.
1 parent efb22d8 commit 6a7650c

File tree

2 files changed

+71
-1
lines changed

2 files changed

+71
-1
lines changed

packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js

+68
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,74 @@ describe('ReactDOMFiberAsync', () => {
776776
});
777777
});
778778

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+
779847
it('regression: infinite deferral loop caused by unstable useDeferredValue input', async () => {
780848
function Text({text}) {
781849
Scheduler.log(text);

packages/react-reconciler/src/ReactFiberHooks.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -3033,7 +3033,9 @@ function updateDeferredValueImpl<T>(
30333033
return resultValue;
30343034
}
30353035

3036-
const shouldDeferValue = !includesOnlyNonUrgentLanes(renderLanes);
3036+
const shouldDeferValue =
3037+
!includesOnlyNonUrgentLanes(renderLanes) &&
3038+
!includesSomeLane(renderLanes, DeferredLane);
30373039
if (shouldDeferValue) {
30383040
// This is an urgent update. Since the value has changed, keep using the
30393041
// previous value and spawn a deferred render to update it later.

0 commit comments

Comments
 (0)