Skip to content

[SDK] Rename otherWallets to allConnectedWallets in onConnect callback #8407

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 12, 2025
+3 −3
Merged

[SDK] Rename otherWallets to allConnectedWallets in onConnect callback #8407

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/thirdweb/src/wallets/connection/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ export type AutoConnectProps = {
*
* ```tsx
* <AutoConnect
* onConnect={(activeWallet, otherWallets) => {
* onConnect={(activeWallet, allConnectedWallets) => {
* console.log("auto connected to", activeWallet)
* console.log("other wallets that were also connected", otherWallets)
* console.log("all connected wallets", allConnectedWallets)
* }}
* />
* ```
*/
onConnect?: (activeWallet: Wallet, otherWallets: Wallet[]) => void;
onConnect?: (activeWallet: Wallet, allConnectedWallets: Wallet[]) => void;
Comment on lines +104 to +111
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify semantic accuracy and breaking change impact.

This rename from otherWallets to allConnectedWallets is a breaking change for TypeScript users. The new name implies the array contains ALL connected wallets, but it's unclear whether this includes the activeWallet or only the other wallets (excluding the active one).

Please clarify:

  1. Does allConnectedWallets include the activeWallet parameter, or does it contain only the other connected wallets (excluding the active one)?
  2. If it includes the active wallet, consider adding a JSDoc comment to clarify this behavior.
  3. Ensure all call sites across the codebase have been updated to use the new parameter name.

🏁 Script executed:

#!/bin/bash
# Verify all references to the old parameter name have been updated

# Search for any remaining references to 'otherWallets' in the context of onConnect
rg -n --type ts --type tsx -C 3 'otherWallets'

# Search for all implementations/call sites of the onConnect callback
ast-grep --pattern $'onConnect: ($_, $_) => {
  $$$
}'

# Search for all usages of AutoConnectProps to ensure consistency
rg -n --type ts --type tsx -C 3 'AutoConnectProps'

Length of output: 55

🏁 Script executed:

#!/bin/bash
# Verify all references to the old parameter name have been updated
# Using glob patterns instead of --type flags

# Search for any remaining references to 'otherWallets'
rg -n -C 3 'otherWallets' -g '*.ts' -g '*.tsx'

# Search for onConnect callback usages and implementations
rg -n -C 5 'onConnect' -g '*.ts' -g '*.tsx' | head -100

# Search for AutoConnectProps usages
rg -n -C 3 'AutoConnectProps' -g '*.ts' -g '*.tsx'

Length of output: 21928

🏁 Script executed:

#!/bin/bash
# Find where onConnect callback is actually invoked in autoConnectCore.ts

rg -n -C 10 'props.onConnect' -g '*.ts' -g '*.tsx' | head -200

Length of output: 20554

🏁 Script executed:

#!/bin/bash
# Search for onConnect callback invocation specifically in autoConnectCore.ts
# Look for where the callback is actually called

rg -n 'manager\.onConnect\|props\.onConnect' packages/thirdweb/src/wallets/connection/autoConnectCore.ts -C 15

# Also check the manager onConnect method definition
rg -n 'onConnect' packages/thirdweb/src/wallets/manager/index.ts -C 5

Length of output: 1262

🏁 Script executed:

#!/bin/bash
# Find where onConnect callback is actually invoked in autoConnectCore.ts
rg -n -A 5 -B 5 'handleWalletConnection' packages/thirdweb/src/wallets/connection/autoConnectCore.ts | head -80

# Also check the handleWalletConnection function signature
rg -n -A 10 'const handleWalletConnection' packages/thirdweb/src/wallets/connection/autoConnectCore.ts

Length of output: 1119

Update variable naming in autoConnectCore.ts to match new parameter semantics.

The type definition parameter was renamed from otherWallets to allConnectedWallets, but the internal variable in autoConnectCore.ts (line 172) is still named otherWallets. While the actual callback receives the correct data (all connected wallets) through manager.connect(), this naming mismatch creates confusion.

Rename otherWallets to better reflect that it contains wallets other than the active one being connected, or document why this distinction is maintained internally. The parameter name change to allConnectedWallets should be consistently reflected throughout the codebase for clarity.

🤖 Prompt for AI Agents 
In packages/thirdweb/src/wallets/connection/autoConnectCore.ts around line 172,
rename the internal variable currently named otherWallets to allConnectedWallets
(or another clearer name like otherConnectedWallets if you want to emphasize
"other than active") and update every reference/usage in that file so it matches
the onConnect type signature in types.ts; ensure the value passed into the
onConnect callback and any local documentation/comments reflect the new name to
avoid confusion.


/**
* Optional chain to autoconnect to
Expand Down
Loading