-
Notifications
You must be signed in to change notification settings - Fork 688
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: realtime balance fetcher for recent transactions #1717
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
useRefetchBalance
hookuseRefetchBalance
hook
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.
Looks like we also need to add docs. Want to create a section below "Advanced" called "Hooks", bump the Modal Hooks doc there as "Modals", and create a new doc for "Balances"?
Noticed some merge conflicts too with v2 |
I think we could simplify this:
|
chore: remove console.log chore: remove console.log
26a1edd
to
52d1e3f
Compare
useRefetchBalance
hookTransactionStoreContext
useEffect(() => { | ||
if (store && address && chainId) { | ||
return store.onTransactionStatus(onTransactionStatus); | ||
} | ||
}, [store, address, chainId, onTransactionStatus]); |
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.
We could pass this logic in here which would be less work, but that would cause a lot of re-renders + make unnecessary RPC calls since it'll happen pretty often due to how our transaction store provider works.
We'll call refetch whenever is necessary to update our balance (e.g when a tx has successfully passed).
chore: revert index.tx change
721478e
to
3d21571
Compare
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.
Everything else looks good; just need that 1 balance fetch optimization I think
query: { | ||
enabled: false, | ||
}, | ||
}); |
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.
@magiziz Should we move this logic to useProfile
so that we're caching it and reduce the number of balance fetches? Currently I believe this hook would be called even if showBalance is disabled, and would cause duplicate balance fetch calls when rendering Profile -> Transaction UI
TransactionStoreContext
Changes
useRefetchBalance
hook toTransactionStoreContext
to support real-time balance supportScreen recordings / screenshots
realtime_transaction_with_store.mov
What to test
address
andvalue
here and try sending the transaction to yourself