-
Notifications
You must be signed in to change notification settings - Fork 7
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
Re-renders everywhere Context api limitation? #11
Comments
I'm not sure I understand the "different store" bit - could you please provide a simple test case? (e.g. on codesandbox) |
First issue: Filters generate layout with checkboxes, radios, etc. Any change to filters (ex. make checkbox selected) re-renders whole view tree which is correct behavior but there's no way to Memoize layout my current workaround to use useMemo instead of React.memo. Here's the second issue (Basically the same as previous) To address performance Basically it will cause issue when application I'm working on is bigger. Every change in any store will make every component depending on outstated store re-render. |
Ah, got it. This is indeed expected behavior and beyond using |
What comes from top of my mind is instead of using single Provider what can be done is dynamic creation of Providers for each store. ATM I don't have much time but I will look into this on the weekend. This will solve unnecessary re-renders for component which depend on different store |
That's exactly what unstated-next does :) |
Unstated doesn't do it automatically. You have to manually wrap Root with Providers you need. I much prefer outstated api so I will try to implement this weekend. I can also make a pull request if you might be interested in something like that. |
That does sound interesting and if you can make it work - I'd be more than happy to accept a PR! |
Related answer from Dan: facebook/react#15156 (comment) |
@yamalight I use option 3 ATM for dynamically generated checkboxes but I don't want to wrap everything inside useMemo. |
Well, as I've mentioned before - if you can figure out how to auto-split stores into contexts, I'd be more than happy to accept a PR with it :) |
Thanks @yamalight ! |
I have tried Option 2 and Option 3 but they are not working correctly - check my test here facebook/react#15156 (comment) |
@marrkeri I looked at your test but you aren't using option 3. Try using useMemo instead of React.memo the component will be recreated but UI will not. |
@marrkeri here's a little demo: https://codesandbox.io/s/outstated-rerenders-test-34p82 |
@yamalight if you take a look on picture from my post you will see in console that it is correct, but dev tools draws re-render for both inputs. I have posted there also other solution without context and that example draws correct re-render in dev tools. facebook/react#15156 (comment) |
@marrkeri fair enough. seems like there is something weird going on indeed 🤔 |
@yamalight OK I drafted first implementation. Can you look into the code if I'm going in the direction you may allow on this repo? The new state code is in state.js https://snack.expo.io/@alexandrius/outstated-issue
|
@alexandrius yep, that looks pretty good! 👍 |
@alexandrius v3.0 is released with your PR included. thanks for your work! ❤️ I'll close this issue, feel free to re-open or create a new one if there's something else. |
Hello.
Although I'm big fan of this library. I found one issue which causes lots of unnecessary re-renders. Basically if any store changes i triggers re-renders to components which rely on different store.
Another problem is that I can't use React.memo if I'm using store inside that component or parent.
Could you help me?
The text was updated successfully, but these errors were encountered: