From bd081376665f5f081dcf4bf72f06b7e563c8046d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 20 Apr 2022 23:30:46 -0400 Subject: [PATCH] Fix: useDeferredValue should reuse previous value (#24413) During an urgent update, useDeferredValue should reuse the previous value. The regression test I added shows that it was reverting to the initial value instead. The cause of the bug was trivial: the update path doesn't update the hook's `memoizedState` field. Only the mount path. None of the existing tests happened to catch this because to trigger the bug, you have to do an urgent update that isn't the first update after initial render. In all of the existing tests that included an urgent update, it was the first update, so the "previous" value and the initial value happened to be the same thing. --- .../src/ReactFiberHooks.new.js | 1 + .../src/ReactFiberHooks.old.js | 1 + .../src/__tests__/ReactDeferredValue-test.js | 75 +++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index b7799d1dedccc..1742d00ae1580 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -1993,6 +1993,7 @@ function updateDeferredValueImpl(hook: Hook, prevValue: T, value: T): T { markWorkInProgressReceivedUpdate(); } + hook.memoizedState = value; return value; } } diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 03360bd6e96ce..20717bde1cd74 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -1993,6 +1993,7 @@ function updateDeferredValueImpl(hook: Hook, prevValue: T, value: T): T { markWorkInProgressReceivedUpdate(); } + hook.memoizedState = value; return value; } } diff --git a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js index 9cd07c066e4cc..a799a53c81ca8 100644 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js @@ -221,4 +221,79 @@ describe('ReactDeferredValue', () => { , ); }); + + it('regression test: during urgent update, reuse previous value, not initial value', async () => { + function App({value: propValue}) { + const [value, setValue] = useState(null); + if (value !== propValue) { + setValue(propValue); + } + + const deferredValue = useDeferredValue(value); + + const child = useMemo(() => , [ + value, + ]); + + const deferredChild = useMemo( + () => , + [deferredValue], + ); + + return ( +
+
{child}
+
{deferredChild}
+
+ ); + } + + const root = ReactNoop.createRoot(); + + // Initial render + await act(async () => { + root.render(); + expect(Scheduler).toFlushUntilNextPaint(['Original: 1', 'Deferred: 1']); + expect(root).toMatchRenderedOutput( +
+
Original: 1
+
Deferred: 1
+
, + ); + }); + + await act(async () => { + startTransition(() => { + root.render(); + }); + expect(Scheduler).toFlushUntilNextPaint(['Original: 2', 'Deferred: 2']); + expect(root).toMatchRenderedOutput( +
+
Original: 2
+
Deferred: 2
+
, + ); + }); + + await act(async () => { + root.render(); + // In the regression, the memoized value was not updated during non-urgent + // updates, so this would flip the deferred value back to the initial + // value (1) instead of reusing the current one (2). + expect(Scheduler).toFlushUntilNextPaint(['Original: 3']); + expect(root).toMatchRenderedOutput( +
+
Original: 3
+
Deferred: 2
+
, + ); + expect(Scheduler).toFlushUntilNextPaint(['Deferred: 3']); + expect(root).toMatchRenderedOutput( +
+
Original: 3
+
Deferred: 3
+
, + ); + }); + }); });