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
Return useAccountBalance hook #829
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very great to me!!
It makes the Balance and FauceButton components a lot simpler and cleaner.
Maybe call this new component with an explicit name like useWatchedBalance?
And maybe remove the watch parameter, because if you don't want it watched you can just use the regular Wagmi useBalance component, right?
Thought about something like that, but with
For me looks strange to use two different hooks for two slightly different cases, and hence for me is better to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @rin-st ! And all for the discussion. I think having this makes sense, since wagmi removed the watch prop in most hooks. In #700 we abstracted for builders in our hooks so they can still use Regarding the name, I don't have a strong opinion, but If I had to vote, I'd say: 🥇 🥈 But I also like the current name Anything works in my mind tho! |
Yup makes sense and less confusion, so let's remove acceptance of |
I'd vote for |
…se-2 into return-use-account-balance
Renamed to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rin-st !!!
Thanks Shiv! Regarding ee10269 but I intentionally added new type. I can't find a proper article to explain why I prefer that way, so I just copy-paste ChatGpt answer. I don't say it's the right way, just something to consider question: answer:
In essence, adding a type alias like |
Ohh, I thought 95% you just removed But yeah my bad should have asked before removing, and also the points make sense!! Thanks for sharing!! |
Description
Basically
useAccountBalance
isuseBalance
with addedwatch: boolean
property, so it's not the same as it was before.useAccountBalance
was removed less than a month ago, but I think we should return it. Reason -watch
property was removed fromuseBalance
hook. So whenever we need to use watched balance outside ofBalance
component, we need to dance with all those query/block number hooks etc. It adds complexity to the code and is also not so obvious to newcomers how to configure it properly.notes:
watch
value istrue
since I think it is needed in most casesAdditional Information