-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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 #25967
Comments
I've included the fix as a PR that was also included in the codesandbox that should resolve the issue. |
This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment! |
The real solution is to drop redux. |
This issue is still present |
This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment! |
…n change (#25968) ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> A proposed fix for the bug described in #25967 ## How did you test this change? See the issue linked above, test scenario included in the code sandbox: https://codesandbox.io/s/fervent-ives-0vm9es?file=/src/App.jsx <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. -->
Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you! |
Fixed in #25968 |
…n change (facebook#25968) ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> A proposed fix for the bug described in facebook#25967 ## How did you test this change? See the issue linked above, test scenario included in the code sandbox: https://codesandbox.io/s/fervent-ives-0vm9es?file=/src/App.jsx <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. -->
…n change (facebook#25968) ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> A proposed fix for the bug described in facebook#25967 ## How did you test this change? See the issue linked above, test scenario included in the code sandbox: https://codesandbox.io/s/fervent-ives-0vm9es?file=/src/App.jsx <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. -->
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:
Link to code example:
https://codesandbox.io/s/fervent-ives-0vm9es?file=/src/App.jsx
The current behavior
The expected behavior
useSyncExternalStoreWithSelector
but should/can reduce the memory footprint of applications using this.The text was updated successfully, but these errors were encountered: