frontend: useAccountSync hook#4121
Merged
Merged
Conversation
5 tasks
Collaborator
Author
|
This hook could probably be used in various places but I'd like to get feedback before continuing. |
📝 WalkthroughWalkthroughThis pull request introduces a reusable ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontends/web/src/api/market.ts (1)
83-91: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider addressing the TODO to improve type safety.
The
errorMessagefield is currently typed asstring, butbtcdirect.tsx(line 214) already checks for the specific value'syncInProgress'. To improve type safety and make the error code explicit, consider using a discriminated union:✨ Proposed type-safe approach
export type TBTCDirectInfoResponse = { success: true; url: string; apiKey: string; address?: string; } | { success: false; - errorMessage: string; // TODO: add 'syncInProgress' + errorCode?: 'syncInProgress'; + errorMessage?: string; };Then update the usage in
btcdirect.tsxto checkerrorCodeinstead:const syncInProgress = !btcdirectInfo?.success && btcdirectInfo?.errorCode === 'syncInProgress';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/web/src/api/market.ts` around lines 83 - 91, The TBTCDirectInfoResponse union uses a generic errorMessage:string which hides specific error cases; update the error branch of TBTCDirectInfoResponse to use a discriminated errorCode (e.g., errorCode: 'syncInProgress' | 'other' or a string literal union) plus an optional human-readable message (e.g., message?: string) so callers can pattern-match safely; then update consumers (notably the btcdirect component usage that currently checks for 'syncInProgress') to test btcdirectInfo?.errorCode === 'syncInProgress' instead of comparing errorMessage. Ensure the exported type name TBTCDirectInfoResponse and the consumer check in btcdirect are updated together.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontends/web/src/hooks/accounts.ts`:
- Around line 15-28: The hook currently swallows API errors and leaves callers
in a perpetual loading state; update the hook (the callApi callback and the
hook's return value) to surface errors: add an error state (e.g. const [error,
setError] = useState<Error|undefined>(undefined)), in callApi clear error on
start, setError(error) inside the catch, and ensure you still guard with
isMounted/current requestId before updating state; change the hook's return
signature from just result to an object like { data: result, error } (or
similar) so callers can show a toast/alert or fallback UI; also ensure you type
the caught error (Error|unknown) and normalize it before storing.
In `@frontends/web/src/routes/market/btcdirect.tsx`:
- Line 10: Remove the now-unused import useLoad from the module import list (the
line "import { useLoad } from '@/hooks/api';") since the code was migrated to
useAccountSynced; delete that import and ensure no remaining references to
useLoad exist in the file (verify usage in functions/components like the
component that previously called useLoad and confirm it uses useAccountSynced
instead).
---
Outside diff comments:
In `@frontends/web/src/api/market.ts`:
- Around line 83-91: The TBTCDirectInfoResponse union uses a generic
errorMessage:string which hides specific error cases; update the error branch of
TBTCDirectInfoResponse to use a discriminated errorCode (e.g., errorCode:
'syncInProgress' | 'other' or a string literal union) plus an optional
human-readable message (e.g., message?: string) so callers can pattern-match
safely; then update consumers (notably the btcdirect component usage that
currently checks for 'syncInProgress') to test btcdirectInfo?.errorCode ===
'syncInProgress' instead of comparing errorMessage. Ensure the exported type
name TBTCDirectInfoResponse and the consumer check in btcdirect are updated
together.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0ecf86f3-1fc4-4d5c-9944-6ec7d0f7bd54
📒 Files selected for processing (5)
frontends/web/src/api/market.tsfrontends/web/src/components/aopp/aopp.tsxfrontends/web/src/hooks/accounts.tsfrontends/web/src/locales/en/app.jsonfrontends/web/src/routes/market/btcdirect.tsx
thisconnect
force-pushed
the
frontend-sync-hook
branch
from
2be1c32 to
226d40e
Compare
Fixed BTC Direct widget loading forever if accounts are not fully synced yet. How to test: 1) unlock device 2) stop make servewallet-mainnet 3) rm -rf appfolder.dev/cache/account-v0-* 4) rm appfolder.dev/accounts.json 5) make servewallet-mainnet 6) navigate to Market / BTC Direct 7) iframe should load after account is synced
thisconnect
force-pushed
the
frontend-sync-hook
branch
from
226d40e to
c67ac8b
Compare
Collaborator
Author
|
rebased |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontends/web/src/api/market.ts`:
- Around line 89-91: The failure branch of the API response type (the object
with success: false and errorMessage: string) should expose a typed
discriminator instead of relying on free-form messages: replace or augment
errorMessage with a field like errorCode: 'syncInProgress' | 'otherError' (or a
broader enum) and update the response union so the success=true and
success=false variants form a discriminated union; then update any consumers/UI
code that parse errorMessage to switch on response.errorCode (handling
'syncInProgress' explicitly) and keep errorMessage only for human-readable text.
Ensure you modify the response type declaration (the false-variant where
success: false is defined) and all call sites that inspect errorMessage.
In `@frontends/web/src/hooks/accounts.ts`:
- Line 5: The import in accounts.ts uses a relative path; update the import of
useMountedRef to use the repository path alias by replacing "import {
useMountedRef } from './mount';" with the alias-based import (e.g. import {
useMountedRef } from '@/hooks/mount') so it follows the frontends/web/src/ `@/`
mapping and matches other internal imports.
- Around line 15-24: The callApi flow leaves previous successful result in state
causing stale data to be visible if a subsequent fetch fails; modify callApi
(where apiRequestId.current is incremented) to clear result by calling
setResult(undefined) at the start of a new request (after computing requestId)
and only when the component is mounted and requestId === apiRequestId.current
(use isMounted.current and requestId checks) so you reset state for the current
request but avoid touching state for unmounted/obsolete requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 02f1df84-41df-4b26-8f1c-3375c551dd3a
📒 Files selected for processing (5)
frontends/web/src/api/market.tsfrontends/web/src/components/aopp/aopp.tsxfrontends/web/src/hooks/accounts.tsfrontends/web/src/locales/en/app.jsonfrontends/web/src/routes/market/btcdirect.tsx
thisconnect
force-pushed
the
frontend-sync-hook
branch
from
c67ac8b to
43e1722
Compare
In case the account is still syncing show a 'syncing account' message instead of just 'loading' to give the user better feedback. Ensure that the account is properly synced and call BTC Direct info once it is synced. This fixes a bug where BTC Direct hangs with loading spinner in case the account was not fully synced.
thisconnect
force-pushed
the
frontend-sync-hook
branch
from
43e1722 to
645e7f6
Compare
thisconnect
force-pushed
the
frontend-sync-hook
branch
from
645e7f6 to
cb0e309
Compare
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontends/web/src/routes/market/bitrefill.tsx (1)
195-226:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle unsuccessful
bitrefillInfostates explicitly.Line 223 now keeps the loader visible whenever
iframeLoadedis false, but the iframe is only mounted on Line 226 whenbitrefillInfo?.successis true. IfuseAccountSynced()resolves to an unsuccessful result (for example while the account is still syncing), this route never clears the spinner and never shows a fallback state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/web/src/routes/market/bitrefill.tsx` around lines 195 - 226, The spinner can hang forever because the iframe is only rendered when bitrefillInfo?.success is true; update the render logic around bitrefillInfo, iframeLoaded and Spinner so unsuccessful or non-success states are handled explicitly: inside the agreedTerms branch (where bitrefillInfo and iframeLoaded are checked) add a clear fallback/error UI (e.g., an error message or retry) when bitrefillInfo exists but bitrefillInfo.success is false, and ensure Spinner is only shown while waiting for either iframeLoaded or bitrefillInfo to be undefined/pending; reference the bitrefillInfo variable, the iframeLoaded state, the Spinner component and the BitrefillTerms branch to locate where to add the fallback rendering and adjust the spinner condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontends/web/src/hooks/account.test.ts`:
- Around line 6-8: Replace the relative imports in account.test.ts with repo
path aliases: change the import of useAccountSynced (currently from './account')
to use the alias '@/hooks/account' and change the import of utils (currently
from './mount') to use '@/hooks/mount' so the file follows the
frontends/web/src/**/*.{ts,tsx} rule that requires '@/...' mappings to src/*.
In `@frontends/web/src/routes/market/pocket.tsx`:
- Around line 319-345: The spinner is shown only based on iframeLoaded, so when
pocketInfo?.success === false (no iframe mounts) the spinner never disappears;
update the Spinner render condition to also require that pocketInfo?.success is
not false (e.g., render Spinner when !iframeLoaded && pocketInfo?.success !==
false) so the Spinner hides when a failed pocket URL is returned; adjust any
related blocking overlay logic if needed and keep onIframeLoad/iframeRef
unchanged.
---
Outside diff comments:
In `@frontends/web/src/routes/market/bitrefill.tsx`:
- Around line 195-226: The spinner can hang forever because the iframe is only
rendered when bitrefillInfo?.success is true; update the render logic around
bitrefillInfo, iframeLoaded and Spinner so unsuccessful or non-success states
are handled explicitly: inside the agreedTerms branch (where bitrefillInfo and
iframeLoaded are checked) add a clear fallback/error UI (e.g., an error message
or retry) when bitrefillInfo exists but bitrefillInfo.success is false, and
ensure Spinner is only shown while waiting for either iframeLoaded or
bitrefillInfo to be undefined/pending; reference the bitrefillInfo variable, the
iframeLoaded state, the Spinner component and the BitrefillTerms branch to
locate where to add the fallback rendering and adjust the spinner condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 59104196-f01a-474f-bc60-e3cd016b5bb4
📒 Files selected for processing (8)
frontends/web/src/api/market.tsfrontends/web/src/components/aopp/aopp.tsxfrontends/web/src/hooks/account.test.tsfrontends/web/src/hooks/account.tsfrontends/web/src/locales/en/app.jsonfrontends/web/src/routes/market/bitrefill.tsxfrontends/web/src/routes/market/btcdirect.tsxfrontends/web/src/routes/market/pocket.tsx
thisconnect
commented
May 13, 2026
| } | { | ||
| success: false; | ||
| errorMessage: string; | ||
| errorMessage: string; // TODO: add 'syncInProgress' |
Collaborator
Author
There was a problem hiding this comment.
Once the backend gets cleaned up it would be nice to explicitly type all the known errors here. cc @bznein
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before asking for reviews, here is a check list of the most common things you might need to consider: