Skip to content
New issue

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

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

Already on GitHub? # to your account

Unstable selectors causes infinite loop in React #2160

Open
1 task done
theKashey opened this issue Apr 25, 2024 · 0 comments
Open
1 task done

Unstable selectors causes infinite loop in React #2160

theKashey opened this issue Apr 25, 2024 · 0 comments

Comments

@theKashey
Copy link

What version of React, ReactDOM/React Native, Redux, and React Redux are you using?

  • React: 18.2.0
  • ReactDOM/React Native: 18.2.0
  • Redux: 3.7.2 (I know)
  • React Redux: 8.1.3

What is the current behavior?

React-redux provides an extra option to check selector stability, but that check:

  • has to be explicitly enabled
  • is not covering connect function
  • is a subject of a design flaw

The problem is that selector stability must be enforced

What is the expected behavior?

Redux hard breaks for unstable selectors.


Let me explain problem in details.

Recently we started migration from React-redux v5 to v8. The process was almost frictionless, but later we started experiencing product outages (full or partial page crashes) due to endless updates somewhere deep in React.

One of the problems here - while debugging in production was always letting us find and eliminate the problem, we were never able to reproduce it in unit (jest) or integration(Cypress) tests.

Long story short - every time the problem leading to a crash was unstable mapStateToProps that was one way or another creating endless update loop in React.

Lets check the docs:

a selector that returns a different result reference when called again with the same inputs will cause unnecessary rerenders.... causes the component to rerender after every action is dispatched
https://react-redux.js.org/api/hooks#selector-result-stability

This description is matching react-redux v5 behavior and is different in v8 due to useSyncExternalStore used underneath. The expected behavior comes from this underlaying hook:

React will re-render the component if getSnapshot return value is different from the last time. This is why, if you always return a different value, you will enter an infinite loop and get this error.
https://react.dev/reference/react/useSyncExternalStore#im-getting-an-error-the-result-of-getsnapshot-should-be-cached

➡️ In our experience we are getting into infinite loop, but are NOT getting this error.

In fact - React has a "double render"-based check for getSnapshot stability, but react-redux effectively bypasses it via memoization here and here


The outcome of the problem is the following:

  • connect function is missing stabilityCheck. It's not as ease to implement as it was done for useSelector, but this gap should be closed
  • due to internal implementation of useSyncExternalStore - stability MUST BE enforced, or React can enter endless loops due to various but yet unknown reasons.
  • useSelector dont have any extra caching layers, and yet the necessity to enforce stability is somewhat unclear. Really wondering why you had to implement stabilityCheck while React should detect the absence of memoization and at least throw an error.

Which browser and OS are affected by this issue?

No response

Did this work in previous versions of React Redux?

  • Yes
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant