Fix useSyncExternalStore dropped update when state is dispatched in r…

…ender phase (#25578) Fix #25565
facebook · Nov 8, 2022 · 1e3e30d · 1e3e30d
1 parent 18dff79
commit 1e3e30d
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 2 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js
@@ -1615,7 +1615,7 @@ function updateSyncExternalStore<T>(
       }
     }
   }
-  const prevSnapshot = hook.memoizedState;
+  const prevSnapshot = (currentHook || hook).memoizedState;
   const snapshotChanged = !is(prevSnapshot, nextSnapshot);
   if (snapshotChanged) {
     hook.memoizedState = nextSnapshot;

diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js
@@ -1615,7 +1615,7 @@ function updateSyncExternalStore<T>(
       }
     }
   }
-  const prevSnapshot = hook.memoizedState;
+  const prevSnapshot = (currentHook || hook).memoizedState;
   const snapshotChanged = !is(prevSnapshot, nextSnapshot);
   if (snapshotChanged) {
     hook.memoizedState = nextSnapshot;

diff --git a/packages/react-reconciler/src/__tests__/useSyncExternalStore-test.js b/packages/react-reconciler/src/__tests__/useSyncExternalStore-test.js
@@ -18,6 +18,7 @@ let useLayoutEffect;
 let forwardRef;
 let useImperativeHandle;
 let useRef;
+let useState;
 let startTransition;
 
 // This tests the native useSyncExternalStore implementation, not the shim.
@@ -36,6 +37,7 @@ describe('useSyncExternalStore', () => {
     useImperativeHandle = React.useImperativeHandle;
     forwardRef = React.forwardRef;
     useRef = React.useRef;
+    useState = React.useState;
     useSyncExternalStore = React.useSyncExternalStore;
     startTransition = React.startTransition;
 
@@ -173,4 +175,33 @@ describe('useSyncExternalStore', () => {
       });
     },
   );
+
+  test('next value is correctly cached when state is dispatched in render phase', async () => {
+    const store = createExternalStore('value:initial');
+
+    function App() {
+      const value = useSyncExternalStore(store.subscribe, store.getState);
+      const [sameValue, setSameValue] = useState(value);
+      if (value !== sameValue) setSameValue(value);
+      return <Text text={value} />;
+    }
+
+    const root = ReactNoop.createRoot();
+    act(() => {
+      // Start a render that reads from the store and yields value
+      root.render(<App />);
+    });
+    expect(Scheduler).toHaveYielded(['value:initial']);
+
+    await act(() => {
+      store.set('value:changed');
+    });
+    expect(Scheduler).toHaveYielded(['value:changed']);
+
+    // If cached value was updated, we expect a re-render
+    await act(() => {
+      store.set('value:initial');
+    });
+    expect(Scheduler).toHaveYielded(['value:initial']);
+  });
 });