-
Notifications
You must be signed in to change notification settings - Fork 9
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
Performance Tradeoffs? #17
Comments
Hi @RobMaple sorry, I missed your comment somehow. Many smaller apps in the react world will actually render the entire app every update which is possible because react's v-dom diffing means it only updates the dom nodes that actually change. I wouldn't do this though - instead I'd use So to answer your question: no, no tradeoffs, the performance story is excellent. The most simple use of this library gives you efficiency you have to fight for with many other approaches. By the way, I've just released a new version which gives you a |
Thanks, will take another look. Out of interest, is there any equality checks that could be built in to the pure-store hook so as to only fire 'setState' when necessary? |
We also have a test to make sure that re-renders don't occur unnecessarily - https://github.com/gunn/pure-store/blob/eea8a8e14c81/src/react.test.tsx#L217 |
Ah ok, that makes sense. Heres a sandbox illustrating the issue: https://codesandbox.io/s/condescending-browser-9hihf?fontsize=14&hidenavigation=1&theme=dark |
Hi Rob, thanks for investigating and making a nice demo, I really appreciate it. I looked into it a bit, and the cause is wrapping the app in The idea seems to be to double render to catch bugs from render side-effects - https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects |
Ah! Sorry, completely missed that. Just fired up a codesandbox default react app, didn't realised it used Strict Mode. Thanks for taking a look! |
Are there any performance tradeoffs in using this library? At first glance it seems like wrapping your app with the subscribe function will rerender the whole app on any update which as far as I'm aware means it will at least miss out on any perf benefits of using React's native state (batched updates for instance).
The text was updated successfully, but these errors were encountered: