-
Notifications
You must be signed in to change notification settings - Fork 396
Adrienne / Added useBalance hook in API package #9770
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
Adrienne / Added useBalance hook in API package #9770
Conversation
Co-authored-by: Sergei Baranovski <120570511+sergei-deriv@users.noreply.github.com>
Co-authored-by: Sergei Baranovski <120570511+sergei-deriv@users.noreply.github.com>
Co-authored-by: Sergei Baranovski <120570511+sergei-deriv@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| * If set as an account id, return the balance of that account. | ||
| * Default is set to 'all'. | ||
| */ | ||
| const useBalance = (account?: string) => { |
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.
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.
@adrienne-deriv No need for this, We will always fetch all. We will be using the useBalance internally in the useAccountList.
|
A production App ID was automatically generated for this PR. (log)
Click here to copy & paste above information. |
|
🚨 Lighthouse report for the changes in this PR:
Lighthouse ran with https://deriv-app-git-fork-adrienne-deriv-api-use-balance.binary.sx/ |
| * If set as an account id, return the balance of that account. | ||
| * Default is set to 'all'. | ||
| */ | ||
| const useBalance = (account?: string) => { |
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.
@adrienne-deriv No need for this, We will always fetch all. We will be using the useBalance internally in the useAccountList.
| */ | ||
| const useBalance = (account?: string) => { | ||
| const { data: balance_data, ...rest } = useFetch('balance', { | ||
| payload: { account: account ?? 'all' }, |
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.
@adrienne-deriv Maybe use option to add a fetch interval until we have a subscription? 🤔
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.
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.
Clarified with @yashim-deriv, when all is passed as payload to account, it is not subscrible-able, but when a loginid is passed instead, the endpoint is subscribe-able
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 can only subscribe once to each endpoint, Otherwise, we will get an error from BE that you are already subscribed. Since the client store is in the background subscribing to balance we can't subscribe again here. Instead, we will do a periodic fetch for now until we can serve the wallet without the core package.
|
Kudos, SonarCloud Quality Gate passed!
|










Changes:
Please provide a summary of the change.
Screenshots:
Please provide some screenshots of the change.