Possible optimization to useSyncExternalStore's withSelector

[dutziworks]

https://github.com/facebook/react/blob/dd2d6522754f52c70d02c51db25eb7cbd5d1c8eb/packages/use-sync-external-store/src/useSyncExternalStoreWithSelector.js#L70-L91

Hey, isn't line 89 in block above supposed to be right before line 78?

I mean, if prevSnapshot and nextSnapshot are not equal, but prevSelection and nextSelection are equal, shouldn't we at least update memoizedSnapshot to be equal to nextSnapshot?

That way, in the next run of this callback, we at least don't run selector and isEqual again.

I may be missing something here, but I did make that change in our, pretty hefty, code base and all the tests passed and I got a nice performance boost.

const prevSnapshot: Snapshot = (memoizedSnapshot: any);
const prevSelection: Selection = (memoizedSelection: any);

if (is(prevSnapshot, nextSnapshot)) {
  // The snapshot is the same as last time. Reuse the previous selection.
  return prevSelection;
}

memoizedSnapshot = nextSnapshot;

// The snapshot has changed, so we need to compute a new selection.
const nextSelection = selector(nextSnapshot);

// If a custom isEqual function is provided, use that to check if the data
// has changed. If it hasn't, return the previous selection. That signals
// to React that the selections are conceptually equal, and we can bail
// out of rendering.
if (isEqual !== undefined && isEqual(prevSelection, nextSelection)) {
  return prevSelection;
}

memoizedSelection = nextSelection;
return nextSelection;

[karankulshrestha]

I will fix it please give me

[karankulshrestha]

please assign me

[jellevoost]

Next to the optimization mentioned, there is another issue this resolves: https://github.com/reduxjs/react-redux/issues/1981 If you use a shared store that changes often, with selectors that always return an equal result, it will keep copies of that old store alive as they are never updated.