Skip to content

Conversation

@ignaciosantise
Copy link
Collaborator

@ignaciosantise ignaciosantise commented Oct 23, 2025

Enhancement to the useAccount hook:

  • The useAccount hook in packages/appkit/src/hooks/useAccount.ts now exposes an allAccounts property, which provides a flattened list of all accounts from the current connections, including their addresses, namespaces, chain IDs, and types. [1] [2]

Note

Expose all connected accounts via useAccount (with typed Account), normalize chain types, and re-export types; add changeset for patch releases.

  • Hooks (packages/appkit/src/hooks/useAccount.ts):
    • Add Account interface and expose allAccounts aggregated from all connections (parsed from CAIP-10; logs invalid entries).
    • Normalize return types: chainId now string; chain typed as AppKitNetwork.
  • Public API (packages/appkit/src/index.ts):
    • Re-export useAccount and type Account as UseAccountReturn.
  • Release:
    • Add changeset with patch bumps across related React Native packages.

Written by Cursor Bugbot for commit 5366d16. This will update automatically on new commits. Configure here.

Copilot AI review requested due to automatic review settings October 23, 2025 17:17
@changeset-bot
Copy link

changeset-bot bot commented Oct 23, 2025

🦋 Changeset detected

Latest commit: 5366d16

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@reown/appkit-react-native Patch
@reown/appkit-bitcoin-react-native Patch
@reown/appkit-coinbase-react-native Patch
@reown/appkit-common-react-native Patch
@reown/appkit-core-react-native Patch
@reown/appkit-ethers-react-native Patch
@reown/appkit-solana-react-native Patch
@reown/appkit-ui-react-native Patch
@reown/appkit-wagmi-react-native Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@claude
Copy link

claude bot commented Oct 23, 2025

Claude finished @ignaciosantise's task —— View job


Code Review Complete ✅

Tasks

  • Read repository guidelines (CLAUDE.md)
  • Review modified useAccount.ts file
  • Analyze code quality and best practices
  • Check type safety and error handling
  • Post review findings

Review Findings

🔴 Critical Issue: Missing TypeScript Type Definition

Location: packages/appkit/src/hooks/useAccount.ts:18-31

The new allAccounts property lacks an explicit type definition. The inferred type is correct, but for a public API hook, an explicit interface should be defined and exported.

Recommendation:

export interface AccountInfo {
  address: string;
  namespace: string;
  chainId: string;
  type?: AccountType;
}

Then update the return type:

return {
  allAccounts, // Type will be AccountInfo[]
  // ... rest
};

🟡 Type Safety Issue: Potential Undefined Value

Location: packages/appkit/src/hooks/useAccount.ts:27

The type property from _connection.type is optional in the Connection interface (type?: AccountType). This means allAccounts can contain objects where type is undefined.

Current behavior:

type: _connection.type  // Can be 'eoa' | 'smartAccount' | undefined

Recommendation: Consider whether undefined is acceptable, or if you should:

  1. Filter out connections without a type
  2. Provide a default value
  3. Document that type can be undefined

🟡 CAIP Standard Parsing Issue

Location: packages/appkit/src/hooks/useAccount.ts:21

The code assumes CAIP addresses follow the format namespace:chainId:address, but there's no validation or error handling if the format is incorrect.

Current code:

const [namespace, chainId, plainAddress] = account.split(':');

Potential issues:

  • If account has more or fewer than 3 parts, the destructuring will succeed but produce incorrect values
  • plainAddress could be undefined if the account format is malformed

Recommendation:

const parts = account.split(':');
if (parts.length !== 3) {
  // Log error or skip invalid accounts
  continue; // or return early
}
const [namespace, chainId, plainAddress] = parts;

Or at minimum, add JSDoc documentation noting the expected format and behavior with malformed data.


📝 Documentation Missing

Location: packages/appkit/src/hooks/useAccount.ts:7-49

The hook and the new allAccounts property lack JSDoc documentation explaining:

  • What allAccounts returns (all accounts across all connections, not just active ones)
  • The structure of each account object
  • When this might be empty vs. populated
  • How it differs from the singular address property

Recommendation:

/**
 * Returns all connected accounts across all active connections.
 * 
 * @returns An array of account objects, each containing:
 *   - address: The plain address (without CAIP prefix)
 *   - namespace: The blockchain namespace (e.g., 'eip155', 'solana')
 *   - chainId: The chain ID from the CAIP address
 *   - type: The account type ('eoa' | 'smartAccount' | undefined)
 */
const allAccounts = useMemo(() => {
  // ...
}, [connections]);

✅ Positives

  • Proper use of useMemo with correct dependencies for performance optimization
  • Follows React Native hooks patterns
  • Code is clean and readable
  • Changeset properly configured

Summary

The implementation is mostly sound, but needs:

  1. Type definition for the account objects (critical for TypeScript users)
  2. Error handling or validation for CAIP address parsing
  3. Documentation explaining the new API
  4. Clarification on whether type: undefined is intentional

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the useAccount hook to expose all connected accounts across different connections. The hook now provides a flattened list of accounts with their addresses, namespaces, chain IDs, and connection types.

Key Changes:

  • Added allAccounts property to the useAccount hook return value
  • Implemented a memoized computation that extracts and flattens account information from all active connections
  • Updated changeset to document this enhancement across all affected packages

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/appkit/src/hooks/useAccount.ts Added allAccounts computed property that flattens and exposes all accounts from connections with their metadata
.changeset/stupid-cloths-admire.md Added changeset documenting the export of account list on useAccount hook

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Account Parsing Fails on Incomplete Strings

The account.split(':') destructuring assumes account strings always have three colon-separated parts. If an account string has fewer parts, plainAddress becomes undefined, leading to address: undefined in the allAccounts objects.

Fix in Cursor Fix in Web

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@sonarqubecloud
Copy link

@ignaciosantise ignaciosantise merged commit 6114f4f into develop Oct 30, 2025
14 of 15 checks passed
@ignaciosantise ignaciosantise deleted the chore/all-accounts branch October 30, 2025 14:54
@github-actions github-actions bot locked and limited conversation to collaborators Oct 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants