less store 13#29093
Conversation
There was a problem hiding this comment.
Pull request overview
This PR continues the “zustand-store-pruning” effort by removing the wallets zustand store and migrating a couple of users-store RPC helpers into the feature/components that invoke them, while updating the pruning checklist documentation accordingly.
Changes:
- Removed the
walletszustand store and its unit tests; wallets screens now load/delete accounts via direct RPC calls and pass required data via route params. - Removed
reportUser/setUserBlocksdispatch helpers from theusersstore and updated call sites to invoke the underlying RPCs directly. - Updated the store-pruning checklist statuses/notes for
team-building,tracker,users, andwallets.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| skill/zustand-store-pruning/references/store-checklist.md | Updates pruning progress notes/statuses for several stores. |
| shared/wallets/index.tsx | Loads wallet accounts via direct RPC into component state; passes needed params to modals. |
| shared/wallets/remove-account.tsx | Removes wallets-store dependency; uses route params for name/balance display. |
| shared/wallets/really-remove-account.tsx | Removes wallets-store dependency; deletes account via direct RPC and uses route params. |
| shared/stores/wallets.tsx | Deletes the wallets zustand store implementation. |
| shared/stores/tests/wallets.test.ts | Deletes wallets-store unit tests. |
| shared/stores/users.tsx | Removes reportUser and setUserBlocks dispatch helpers from the users store. |
| shared/chat/blocking/block-modal.tsx | Calls report/block RPCs directly instead of via users-store dispatch helpers. |
| shared/teams/team/rows/member-row.tsx | Uses direct userSetUserBlocks RPC instead of users-store dispatch. |
| shared/teams/channel/rows.tsx | Uses direct userSetUserBlocks RPC instead of users-store dispatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type Account = { | ||
| accountID: string | ||
| balanceDescription: string | ||
| deviceReadOnly: boolean | ||
| isDefault: boolean | ||
| name: string | ||
| } |
There was a problem hiding this comment.
Account duplicates fields that already exist on the RPC type (T.RPCStellar.WalletAccountLocal). To avoid drift when the protocol type changes, consider defining this as a Pick (or alias) of the generated RPC type instead of re-declaring the shape here.
| type Account = { | |
| accountID: string | |
| balanceDescription: string | |
| deviceReadOnly: boolean | |
| isDefault: boolean | |
| name: string | |
| } | |
| type Account = Pick< | |
| T.RPCStellar.WalletAccountLocal, | |
| 'accountID' | 'balanceDescription' | 'deviceReadOnly' | 'isDefault' | 'name' | |
| > |
| const Container = () => { | ||
| const [accounts, setAccounts] = React.useState<Array<Account>>([]) | ||
| const [acceptedDisclaimer, setAcceptedDisclaimer] = React.useState(false) | ||
| const checkDisclaimer = C.useRPC(T.RPCStellar.localHasAcceptedDisclaimerLocalRpcPromise) | ||
|
|
||
| const load = useWalletsState(s => s.dispatch.load) | ||
| const loadAccounts = C.useRPC(T.RPCStellar.localGetWalletAccountsLocalRpcPromise) | ||
|
|
||
| C.Router2.useSafeFocusEffect( | ||
| () => { | ||
| load() | ||
| loadAccounts( | ||
| [undefined, loadAccountsWaitingKey], | ||
| res => { | ||
| setAccounts( | ||
| (res ?? []).map(a => ({ | ||
| accountID: a.accountID, | ||
| balanceDescription: a.balanceDescription, | ||
| deviceReadOnly: a.deviceReadOnly, | ||
| isDefault: a.isDefault, | ||
| name: a.name, | ||
| })) | ||
| ) | ||
| }, | ||
| () => { | ||
| setAccounts([]) | ||
| } | ||
| ) |
There was a problem hiding this comment.
The previous wallets store had unit tests for account loading/removal, but this screen now owns the RPC + mapping logic directly (and the store tests were removed). Consider adding replacement tests for the localGetWalletAccountsLocal result mapping / sorting and the navigation params used by the remove-account flow so this behavior stays covered.
| reportUserRPC( | ||
| [ | ||
| { | ||
| comment: report.extraNotes, | ||
| convID: conversationIDKey, | ||
| includeTranscript: report.includeTranscript && !!conversationIDKey, | ||
| reason: report.reason, | ||
| username, | ||
| }, | ||
| S.waitingKeyUsersReportUser, | ||
| ], | ||
| () => {}, | ||
| () => {} | ||
| ) |
There was a problem hiding this comment.
These RPC calls intentionally ignore both success and error callbacks (() => {}), which makes failures silent and hard to debug (and can leave the modal in an unclear state if a report/block fails). Consider at least logging errors and/or surfacing a user-visible error/feedback path on failure.
| navigateUp() | ||
| deleteAccount([{accountID, userAcknowledged: 'yes'}, loadAccountsWaitingKey], () => { | ||
| navigateUp() | ||
| }, () => {}) |
There was a problem hiding this comment.
The delete-account RPC error callback is currently a no-op, so failures will be silent and the user won’t know why the account wasn’t removed. Consider handling the error (e.g., show a banner/toast and/or log) so this flow is diagnosable.
| }, () => {}) | |
| }, err => { | |
| console.error('Error deleting wallet account', err) | |
| }) |
chrisnojima
force-pushed
the
nojima/ZCLIENT-less-store-13
branch
from
125aaec to
f833bc9
Compare
chrisnojima
force-pushed
the
nojima/ZCLIENT-less-store-12
branch
from
323fa1c to
a35bca9
Compare
No description provided.