From e12a9dfc96be12ea8e5c759986041ee5308e8e06 Mon Sep 17 00:00:00 2001 From: Douglas Armstrong Date: Thu, 20 Jan 2022 07:22:17 -0800 Subject: [PATCH] Fix production-only updateSyncExternalStore() crash when doing setState in render (#23150) * Update ReactFiberHooks.new.js * Add regression test + replace-fork * Prettier Co-authored-by: Dan Abramov --- .../src/ReactFiberHooks.new.js | 2 +- .../src/ReactFiberHooks.old.js | 2 +- .../useSyncExternalStoreShared-test.js | 27 +++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index cf8086b075aa2..7747a25c858ed 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -2471,7 +2471,7 @@ const HooksDispatcherOnRerender: Dispatcher = { useDeferredValue: rerenderDeferredValue, useTransition: rerenderTransition, useMutableSource: updateMutableSource, - useSyncExternalStore: mountSyncExternalStore, + useSyncExternalStore: updateSyncExternalStore, useId: updateId, unstable_isNewReconciler: enableNewReconciler, diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index ff2aff6720e11..d7137e8cd7f66 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -2471,7 +2471,7 @@ const HooksDispatcherOnRerender: Dispatcher = { useDeferredValue: rerenderDeferredValue, useTransition: rerenderTransition, useMutableSource: updateMutableSource, - useSyncExternalStore: mountSyncExternalStore, + useSyncExternalStore: updateSyncExternalStore, useId: updateId, unstable_isNewReconciler: enableNewReconciler, diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js index 5af9e9767ab0c..cb3a40aa13ab7 100644 --- a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js +++ b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js @@ -745,6 +745,33 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); }); + test('regression test for #23150', async () => { + const store = createExternalStore('Initial'); + + function App() { + const text = useSyncExternalStore(store.subscribe, store.getState); + const [derivedText, setDerivedText] = useState(text); + useEffect(() => {}, []); + if (derivedText !== text.toUpperCase()) { + setDerivedText(text.toUpperCase()); + } + return ; + } + + const container = document.createElement('div'); + const root = createRoot(container); + await act(() => root.render()); + + expect(Scheduler).toHaveYielded(['INITIAL']); + expect(container.textContent).toEqual('INITIAL'); + + await act(() => { + store.set('Updated'); + }); + expect(Scheduler).toHaveYielded(['UPDATED']); + expect(container.textContent).toEqual('UPDATED'); + }); + // The selector implementation uses the lazy ref initialization pattern // @gate !(enableUseRefAccessWarning && __DEV__) test('compares selection to rendered selection even if selector changes', async () => {