-
Notifications
You must be signed in to change notification settings - Fork 61
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
broken in react 18 #100
Comments
If you put the console.log statement in useEffect, it shows just once, right? If that's the case, it's expected behavior, React 18 disables useReducer early bailout and it's how it works. If this behavior causes a performance issue in your app, the solution is to use useMemo to avoid repeating computation. |
I remember something. If you have a stable selector, it can be optimized. |
oops, my bad. optimization with stable selector is about calling the selector. so, it doesn't apply in this case. |
@dai-shi thanks for the quick response. You're right, a The behavior brings to mind two worries: the first is that it could confuse engineers who encounter an unexpected "rerender" and spend time trying to track it down, and the second is a perf concern with more computation now done inline in the function body. So for example a simple unmemoized inline loop (say Regardless I appreciate the explanation and the work done here. For the time being I think we will monitor performance and apply memoization where necessary. |
(I feel like I answered to a similar question somewhere. pmndrs/valtio#638 (reply in thread) is it...) The first concern is valid, but people misunderstand how react optimizes renders. I would suggest, if they want to debug extra re-renders with
Correct. The motivation of this library is to make it compatible (as much as possible in userland) with concurrency. I run experiment here: https://github.com/dai-shi/will-this-react-global-state-work-in-concurrent-rendering It's kind of opt-in, if you use btw, with React 17, it works as expected, right? If you want something for legacy react (no concurrency), creating use-context-selector API compatible library is fairly easy (using Especially, if we require stable selectors (always use useCallback), it can be implemented without There are such libraries out there, but if you are interested, I can show the rough idea. |
yes the behavior was as expected in 17. in 18 we'll continue to monitor our app and memoize as necessary. while we haven't adopted much concurrency yet, switching back to a legacy library which could introduce potential tearing may be something we regret later. thanks for the response |
In retrospect, this library is developed for concurrency (it's a spin-off from react-tracked), and in v1.3, it moved to a new implementation that requires With that, let's close this. On side note: The goal of this library is not performance. If people are trying to use this library for global state solution, and they care performance, I'd recommend trying global state libraries, which could be more performant. Here's some of my projects: jotai, valtio, and zustand. |
hi there. I love
use-context-selector
but after recently upgrading a large codebase to react 18, we noticed that it stopped behaving as expected. subscribers to the context are rerendering, regardless of whether their selector returns a changed value or not. for example in this case:clicking the button changes a part of the state that is not used by any of the consumers of the context. however they all still rerender anyway. this seems to defeat the purpose of the library, which is to prevent unnecessary rerenders. any guidance you have would be appreciated here
The text was updated successfully, but these errors were encountered: