-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
feat: Shallow equality option for useStoreState() #354
Conversation
Codecov Report
@@ Coverage Diff @@
## master #354 +/- ##
=======================================
Coverage 97.76% 97.76%
=======================================
Files 20 20
Lines 581 581
Branches 113 113
=======================================
Hits 568 568
Misses 11 11
Partials 2 2 Continue to review full report at Codecov.
|
@ctrlplusb anything I can do to push this along? |
Hi @joelmoss - shall endeavour to review this week. I have had a very busy past few weeks. |
No worries bud. Thx
Joel Moss
Develop with Style
DevelopWithStyle.com
07791 503309
https://www.linkedin.com/in/joelkmoss/
https://github.com/joelmoss
https://twitter.com/joelmoss
…On 23 Nov 2019, 09:15 +0000, Sean Matheson ***@***.***>, wrote:
Hi @joelmoss - shall endeavour to review this week. I have had a very busy past few weeks.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@joelmoss I've had a look. Thanks so much for this. Will be an awesome addition. I'd like to request that we change the API somewhat. Instead of the configuration object could we instead accept an equality function as the second argument to useStoreState. This would very much be in line with the Redux useSelector hook then. We could include a default shallow equality fn that user's could leverage. This provides a lot more flexibility going forward IMO. |
ok, yeah I like that. Leave it with me. |
By the way, I noticed that the However, I'm not seeing why that package is included and where it used. |
Actually, no matter. The |
done! |
Amazing thanks! |
@joelmoss Thanks for this feature. If possible please squash your commits into one and update the docs/example found in this commit 1d29a67, since it no longer matches the final implementation. This might be very useful to @ctrlplusb if he decides to use that in the documentation. |
Currently. useStoreState() performs a strict equality check (`oldState === newState`) on its state, which is great for most cases. But sometimes you need to work with more complex state and return something that will always fail a strict check. This PR adds support for providing a custom equality function as follows: ```javascript const store = createStore({ count: 1, firstName: null, lastName: null }); // In your component const { count, firstName } = useStoreState( state => ({ count: state.count, firstName: state.firstName, }), (prevState, nextState) => { // perform some equality comparison here return shallowEqual(prevState, nextState) } ); ``` In the above case, if either the `count` or `firstName` state vars change, the equality check with be true, and the component will re-render. And if any other state is changed; for example, the `lastName`, the equality check will be true, and the component will not re-render. An exported `shallowEqual()` function is provided to allow you to run shallow equality checks: `useStoreState(map, shallowEqual)`. See ctrlplusb#275
e302140
to
8c534dc
Compare
Done! |
@joelmoss Cool. I think |
Not familiar with TS, but will give it a go |
@joelmoss - if you aren't comfortable with doing the TS don't stress. Let me know and I will pick that up for ya. |
Yeah, sorry. Not familiar enough with TS to give this time. So any help would be appreciated, thx
…On 9 Dec 2019, 17:59 +0000, Sean Matheson ***@***.***>, wrote:
@joelmoss - if you aren't comfortable with doing the TS don't stress. Let me know and I will pick that up for ya.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@ctrlplusb Just had a thought about this PR in the context of computed properties; do they also have a strict equality check? If so, we should probably allow this new option with computed properties too. |
…354) Currently. useStoreState() performs a strict equality check (`oldState === newState`) on its state, which is great for most cases. But sometimes you need to work with more complex state and return something that will always fail a strict check. This PR adds support for providing a custom equality function as follows: ```javascript const store = createStore({ count: 1, firstName: null, lastName: null }); // In your component const { count, firstName } = useStoreState( state => ({ count: state.count, firstName: state.firstName, }), (prevState, nextState) => { // perform some equality comparison here return shallowEqual(prevState, nextState) } ); ``` In the above case, if either the `count` or `firstName` state vars change, the equality check with be true, and the component will re-render. And if any other state is changed; for example, the `lastName`, the equality check will be true, and the component will not re-render. An exported `shallowEqual()` function is provided to allow you to run shallow equality checks: `useStoreState(map, shallowEqual)`. See #275 Co-authored-by: Sean Matheson <[email protected]>
Currently. useStoreState() performs a strict equality check (
oldState === newState
) on its state, which is great for most cases. But sometimes you need to work with more complex state and return something that will always fail a strict check.This PR adds support for a shallow equality check as follows:
In the above case, if either the
count
orfirstName
state vars change, the equality check with be true, and the component will re-render. And if any other state is changed; for example, thelastName
, the equality check will be true, and the component will not re-render.The
shallowEquality
option is false by default, thus maintaining existing functionality.See #275