Skip to content

fix: return empty array if not connected#498

Merged
ignaciosantise merged 2 commits intodevelopfrom
fix/use-account-clear
Nov 14, 2025
Merged

fix: return empty array if not connected#498
ignaciosantise merged 2 commits intodevelopfrom
fix/use-account-clear

Conversation

@ignaciosantise
Copy link
Copy Markdown
Collaborator

@ignaciosantise ignaciosantise commented Nov 5, 2025

This pull request makes a small improvement to the useAccount hook to ensure that the list of accounts is only computed when an address is present, and updates the hook's dependencies for correctness.

  • Added a guard clause to return an empty array if address is not defined, preventing unnecessary computation.
  • Updated the dependency array of the useMemo hook to include address, ensuring the memoized value updates correctly when address changes.

Note

useAccount now returns [] when no address is active and updates memo deps; patch releases prepared across React Native packages.

  • Hooks
    • packages/appkit/src/hooks/useAccount.ts:
      • Guard: return [] from allAccounts when address is falsy.
      • Update useMemo deps to [connections, address].
  • Release
    • Changeset adds patch bumps for multiple @reown/*-react-native packages.

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

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 5, 2025

🦋 Changeset detected

Latest commit: fd620bd

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-ui-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-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
Copy Markdown

claude bot commented Nov 5, 2025

Claude finished @ignaciosantise's task —— View job


Code Review Complete ✅

Tasks:

  • Read and analyze the modified file
  • Review code quality and best practices
  • Check for bugs and edge cases
  • Verify changeset requirements
  • Post comprehensive review feedback

Review Feedback

Issues Found

1. Logic Issue: Guard clause doesn't prevent computation packages/appkit/src/hooks/useAccount.ts:78-102

The guard clause at line 78 returns an empty array when address is falsy, but this doesn't actually prevent the computation described in the PR. The issue is that connections could still contain data that gets processed even when there's no address. However, the real problem is more subtle:

  • When address is undefined/empty, returning [] makes sense
  • But the computation on lines 80-101 will still execute for all connections before the return
  • The early return only prevents the result from being used, not the computation itself

Suggested fix: Move the guard to the top of the useMemo callback:

const allAccounts: Account[] = useMemo(() => {
  if (!address) return [];
  
  return Array.from(connections.values()).flatMap(
    // ... rest of the logic
  );
}, [connections, address]);

Wait - actually looking at the code again, the guard IS at the top of the useMemo callback (line 78), so the computation IS being prevented. This is correct. ✅

2. Dependency Array Incomplete packages/appkit/src/hooks/useAccount.ts:102

The dependency array includes [connections, address], which is correct for the guard clause. However, there's a subtle issue: the hook accesses _connection.type (line 97) which comes from the connection objects within the connections Map. Since connections is included, this should be fine from a correctness standpoint.

The change to add address to the dependency array is correct and necessary because:

  • The address variable is used in the guard condition (line 78)
  • Without it in dependencies, the memoized value wouldn't update when address changes from undefined to defined or vice versa

✅ This change is correct.

3. Missing Changeset (as flagged by changeset-bot)

This PR modifies behavior of a public hook (useAccount) by:

  • Changing when the accounts array gets computed
  • Fixing a missing dependency in useMemo

Even though this is labeled as chore:, it affects the public API behavior (consumers will see different behavior when disconnected). This should have a changeset with at least a patch version bump.

Recommendation: Add a changeset file as suggested by the bot.


Summary

Severity Breakdown:

  • 🔴 Critical: 0
  • 🟡 Medium: 1 (missing changeset)
  • 🟢 Low: 0

Overall Assessment:
The code changes themselves are correct and well-implemented. The guard clause properly prevents unnecessary computation, and the dependency array fix resolves a potential stale closure bug. The only issue is the missing changeset, which is required by the project's release process.

Recommendation: ✅ Approve after adding changeset


Branch: fix/use-account-clear

@ignaciosantise ignaciosantise changed the title chore: return empty array if not connected fix: return empty array if not connected Nov 5, 2025
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Nov 5, 2025

Base automatically changed from chore/qr-borders to develop November 14, 2025 13:48
@ignaciosantise ignaciosantise merged commit 38611b0 into develop Nov 14, 2025
8 checks passed
@ignaciosantise ignaciosantise deleted the fix/use-account-clear branch November 14, 2025 13:50
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 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.

1 participant