Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Stale selectors keep old store snapshots alive in useSyncExternalStoreWithSelector #3

Open
Lucier opened this issue Jan 27, 2023 · 0 comments

Comments

@Lucier
Copy link
Owner

Lucier commented Jan 27, 2023

Within useSyncExternalStoreWithSelector there is currently a bug that will keep old references to the used store alive if you use an immutable store in combination with selectors that always result in the same result. This can lead to excessive memory usage while this is not needed. I've noticed this behavior in combination with react-redux, but also managed to reproduce it without react-redux to figure out exactly what was going on. (I've reported this at the react-redux repo as well reduxjs/react-redux#1981)
React version: 18.2.0
Steps To Reproduce
Since the reproduction is fairly complicated I've created a sandbox with details on how to reproduce including a minimal working sample that also includes the reproduction steps within that exact example.
But in summary, it is reproducible using the following steps:
1. You will need an immutable store that you will use with your selector (new copy every store update)
2. You will need a stable custom isEqual function and selector that are not created in-line
3. The store needs some property (e.g. a string) that is easily found within memory snapshots. (It helps to include a timestamp within this property that updates on store change)
4. You will need to print a value from the store within your main component, to show the most recent result of the store.
5. You will need a component that uses a stable selector to select a stale value (something that never changes) from the store
6. Next up you need to update the store and add an additional copy of the previous component (the one created in step 5.)
7. Repeat this a few times
8. Take a memory snapshot and notice there are multiple copies of the store present in memoizedSnapshots of the different components. You can see this by searching on the property you defined in step 3.
Link to code example:
https://codesandbox.io/s/fervent-ives-0vm9es?file=/src/App.jsx
The current behavior
• Whenever the result of getSnapshot() is changed, but the result of the selector() has not, the memoized reference to the old result of getSnapshot() is not updated, resulting in unnecessary copies of the store used alive. Whenever you have a fairly large store that is shared between quite a bunch of selectors, especially with components that are mounted at a later timestamp and use selectors that have stale data, you could end up with an ever increasing amount of store references resulting in high memory usage.
The expected behavior
• Whenever the result of getSnapshot() is changed, but the result of the selector() has not, the memoized reference to the old result of getSnapshot() is updated correctly, preventing unnecessary copies from being kept alive. This should not impact the behavior of useSyncExternalStoreWithSelector but should/can reduce the memory footprint of applications using this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant