Skip to content

Commit

Permalink
Fix production-only updateSyncExternalStore() crash when doing setSta…
Browse files Browse the repository at this point in the history
…te in render (facebook#23150)

* Update ReactFiberHooks.new.js

* Add regression test + replace-fork

* Prettier

Co-authored-by: Dan Abramov <dan.abramov@me.com>
  • Loading branch information
2 people authored and zhengjitf committed Apr 15, 2022
1 parent 6fa197f commit 4268962
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2471,7 +2471,7 @@ const HooksDispatcherOnRerender: Dispatcher = {
useDeferredValue: rerenderDeferredValue,
useTransition: rerenderTransition,
useMutableSource: updateMutableSource,
useSyncExternalStore: mountSyncExternalStore,
useSyncExternalStore: updateSyncExternalStore,
useId: updateId,

unstable_isNewReconciler: enableNewReconciler,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2471,7 +2471,7 @@ const HooksDispatcherOnRerender: Dispatcher = {
useDeferredValue: rerenderDeferredValue,
useTransition: rerenderTransition,
useMutableSource: updateMutableSource,
useSyncExternalStore: mountSyncExternalStore,
useSyncExternalStore: updateSyncExternalStore,
useId: updateId,

unstable_isNewReconciler: enableNewReconciler,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Text text={derivedText} />;
}

const container = document.createElement('div');
const root = createRoot(container);
await act(() => root.render(<App />));

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 () => {
Expand Down

0 comments on commit 4268962

Please sign in to comment.