Skip to content

Commit

Permalink
Fix useSyncExternalStore dropped update when state is dispatched in r…
Browse files Browse the repository at this point in the history
…ender phase (#25578)

Fix facebook/react#25565
  • Loading branch information
jerrydev0927 committed Dec 3, 2022
1 parent acb4a92 commit 397a93d
Show file tree
Hide file tree
Showing 3 changed files with 33 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 @@ -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;
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 @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -36,6 +37,7 @@ describe('useSyncExternalStore', () => {
useImperativeHandle = React.useImperativeHandle;
forwardRef = React.forwardRef;
useRef = React.useRef;
useState = React.useState;
useSyncExternalStore = React.useSyncExternalStore;
startTransition = React.startTransition;

Expand Down Expand Up @@ -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']);
});
});

0 comments on commit 397a93d

Please sign in to comment.