[SDK] Fix wallet reconnection for previously connected wallets #8066
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: aaa7ea8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
WalkthroughRemoves runtime detection/augmentation of installed wallets from auto-connect; available wallets are now derived from provided wallets and lastConnectedWalletIds. Wallet manager persists only currently connected wallet IDs. Adds a changeset and updates tests and a chain constant usage. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant useAutoConnect
participant autoConnectCore
participant Manager
participant Storage
App->>useAutoConnect: init(props.wallets, storage, createWallet)
useAutoConnect->>autoConnectCore: start auto-connect
autoConnectCore->>Storage: read lastConnectedWalletIds & lastActiveWalletId
Note over autoConnectCore: Build availableWallets:\nfor each id in lastConnectedWalletIds:\n- use provided wallet if present\n- else create via createWalletFn(id)
autoConnectCore->>Manager: attempt connect (priority order)
alt success
Manager-->>autoConnectCore: connected wallet
autoConnectCore->>Storage: persist lastActiveWalletId
autoConnectCore-->>useAutoConnect: set active wallet
else failure
autoConnectCore-->>useAutoConnect: no connection established
end
rect rgba(220,240,255,0.4)
Note over Manager,Storage: Manager now saves only current connected wallet IDs (no merge with previous)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
joaquim-verges
force-pushed
the
_SDK_Fix_wallet_reconnection_for_previously_connected_wallets
branch
from
1625b15 to
748df0e
Compare
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
joaquim-verges
force-pushed
the
_SDK_Fix_wallet_reconnection_for_previously_connected_wallets
branch
from
748df0e to
382bd81
Compare
size-limit report 📦
|
![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
🧹 Nitpick comments (5)
packages/thirdweb/src/wallets/manager/index.ts (2)
244-247: Tiny allocation nit: avoid redundant spread into Set.Apply:
- stringify(Array.from(new Set([...ids]))), + stringify(Array.from(new Set(ids))),
332-344: Return type saysstring[] | nullbut implementation never returnsnull.Tighten the signature and doc to
Promise<string[]>for correctness and clearer consumers.Apply:
-export async function getStoredConnectedWalletIds( - storage: AsyncStorage, -): Promise<string[] | null> { +export async function getStoredConnectedWalletIds( + storage: AsyncStorage, +): Promise<string[]> {packages/thirdweb/src/transaction/prepare-transaction.test.ts (1)
25-37: Chain ID in authorization mismatches the chain under test.
authorization.request.chainIdis911867while the transaction now usesbaseSepolia(id84532). Align to avoid confusion/flaky future assertions.Apply:
- chainId: 911867, + chainId: baseSepolia.id,packages/thirdweb/src/wallets/connection/autoConnectCore.ts (2)
48-63: Add explicit return type to exported function.Project guidelines call for explicit return types on TS functions.
Apply:
-export const autoConnectCore = async (props: AutoConnectCoreProps) => { +export const autoConnectCore = async ( + props: AutoConnectCoreProps, +): Promise<boolean> => {
112-116: Early-return condition is ineffective and can leaveisAutoConnectingstuck.
getStoredConnectedWalletIdsreturns[], so!lastConnectedWalletIdsis never true. Also, if it ever is, we should reset the flag before returning.Apply:
- if (!lastConnectedWalletIds) { - return autoConnected; - } + if (!lastConnectedWalletIds?.length) { + manager.isAutoConnecting.setValue(false); + return autoConnected; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/clean-wings-repeat.md(1 hunks)packages/thirdweb/src/react/core/hooks/wallets/useAutoConnect.ts(0 hunks)packages/thirdweb/src/react/web/hooks/wallets/useAutoConnect.ts(0 hunks)packages/thirdweb/src/transaction/prepare-transaction.test.ts(2 hunks)packages/thirdweb/src/wallets/connection/autoConnect.ts(0 hunks)packages/thirdweb/src/wallets/connection/autoConnectCore.ts(1 hunks)packages/thirdweb/src/wallets/manager/index.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/thirdweb/src/wallets/connection/autoConnect.ts
- packages/thirdweb/src/react/core/hooks/wallets/useAutoConnect.ts
- packages/thirdweb/src/react/web/hooks/wallets/useAutoConnect.ts
🧰 Additional context used
📓 Path-based instructions (6)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/*.md: Each change inpackages/*must include a changeset for the appropriate package
Version bump rules: patch for non‑API changes; minor for new/modified public API
Files:
.changeset/clean-wings-repeat.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/wallets/manager/index.tspackages/thirdweb/src/transaction/prepare-transaction.test.tspackages/thirdweb/src/wallets/connection/autoConnectCore.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/wallets/manager/index.tspackages/thirdweb/src/transaction/prepare-transaction.test.tspackages/thirdweb/src/wallets/connection/autoConnectCore.ts
packages/thirdweb/src/wallets/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/wallets/**: UnifiedWalletandAccountinterfaces in wallet architecture
Support for in-app wallets (social/email login)
Smart wallets with account abstraction
EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Files:
packages/thirdweb/src/wallets/manager/index.tspackages/thirdweb/src/wallets/connection/autoConnectCore.ts
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/wallets/manager/index.tspackages/thirdweb/src/transaction/prepare-transaction.test.tspackages/thirdweb/src/wallets/connection/autoConnectCore.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/thirdweb/src/transaction/prepare-transaction.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Applied to files:
packages/thirdweb/src/wallets/manager/index.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Applied to files:
packages/thirdweb/src/wallets/manager/index.tspackages/thirdweb/src/transaction/prepare-transaction.test.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Smart wallets with account abstraction
Applied to files:
packages/thirdweb/src/wallets/manager/index.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to test/src/test-wallets.ts : Predefined test accounts are in `test/src/test-wallets.ts`
Applied to files:
packages/thirdweb/src/transaction/prepare-transaction.test.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to **/*.test.{ts,tsx} : Use `FORKED_ETHEREUM_CHAIN` for mainnet interactions and `ANVIL_CHAIN` for isolated tests
Applied to files:
packages/thirdweb/src/transaction/prepare-transaction.test.ts
🧬 Code graph analysis (3)
packages/thirdweb/src/wallets/manager/index.ts (1)
packages/thirdweb/src/exports/utils.ts (1)
stringify(189-189)
packages/thirdweb/src/transaction/prepare-transaction.test.ts (1)
packages/thirdweb/src/exports/chains.ts (1)
baseSepolia(18-18)
packages/thirdweb/src/wallets/connection/autoConnectCore.ts (1)
packages/thirdweb/src/exports/wallets.ts (1)
WalletId(139-139)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
joaquim-verges
force-pushed
the
_SDK_Fix_wallet_reconnection_for_previously_connected_wallets
branch
from
382bd81 to
560ae93
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/thirdweb/src/wallets/manager/index.ts (2)
400-407: EOA disconnect leaves stale EOA in connectedWallets; also remove the EOAWhen the personal EOA disconnects, only the smart wallet is removed. The EOA remains in
connectedWallets, causing staleCONNECTED_WALLET_IDSand unintended reconnection attempts.Apply this fix to clear both:
const disconnectUnsub = eoaWallet.subscribe("disconnect", () => { handleDisconnect(); }); const handleDisconnect = () => { disconnectUnsub(); - onWalletDisconnect(wallet); + // purge both the personal EOA and the smart wallet + onWalletDisconnect(eoaWallet); + onWalletDisconnect(wallet); };
158-163: Duplicate accountChanged subscriptions across reconnectionsRe-subscribing inside
handleConnectionon every account change accumulates listeners and re-triggers side effects multiple times.Use a per-wallet unsubscribe registry to de‑dupe:
+ // keep at most one accountChanged subscription per wallet id + const accountChangeUnsubs = new Map<WalletId, () => void>(); … - wallet.subscribe("accountChanged", async () => { + // replace prior subscription (if any) + accountChangeUnsubs.get(wallet.id)?.(); + const unsub = wallet.subscribe("accountChanged", async () => { // We reimplement connect here to prevent memory leaks const newWallet = await handleConnection(wallet, options); options?.onConnect?.(newWallet); }); + accountChangeUnsubs.set(wallet.id, unsub);Optionally clear this map entry inside
onWalletDisconnect(wallet)to avoid leaks.
🧹 Nitpick comments (2)
packages/thirdweb/src/wallets/manager/index.ts (2)
16-16: Use the shared WalletId type for the Map keyAligns with the codebase’s shared types guidance.
-type WalletIdToConnectedWalletMap = Map<string, Wallet>; +type WalletIdToConnectedWalletMap = Map<WalletId, Wallet>;
332-344: Tighten return type (function never returns null)Implementation always returns an array (empty on missing/error). Change signature to Promise<string[]> and update callers that rely on nullish/falsy checks.
-export async function getStoredConnectedWalletIds( - storage: AsyncStorage, -): Promise<string[] | null> { +export async function getStoredConnectedWalletIds( + storage: AsyncStorage, +): Promise<string[]> {Caller to update: packages/thirdweb/src/wallets/connection/autoConnectCore.ts — replace
if (!lastConnectedWalletIds)withif (lastConnectedWalletIds.length === 0)(only call site found).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.changeset/clean-wings-repeat.md(1 hunks)packages/thirdweb/src/react/core/hooks/wallets/useAutoConnect.ts(0 hunks)packages/thirdweb/src/react/web/hooks/wallets/useAutoConnect.ts(0 hunks)packages/thirdweb/src/transaction/prepare-transaction.test.ts(2 hunks)packages/thirdweb/src/wallets/connection/autoConnect.test.ts(0 hunks)packages/thirdweb/src/wallets/connection/autoConnect.ts(0 hunks)packages/thirdweb/src/wallets/connection/autoConnectCore.ts(1 hunks)packages/thirdweb/src/wallets/manager/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- packages/thirdweb/src/wallets/connection/autoConnect.test.ts
- packages/thirdweb/src/react/web/hooks/wallets/useAutoConnect.ts
- packages/thirdweb/src/wallets/connection/autoConnect.ts
- packages/thirdweb/src/react/core/hooks/wallets/useAutoConnect.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/clean-wings-repeat.md
- packages/thirdweb/src/wallets/connection/autoConnectCore.ts
- packages/thirdweb/src/transaction/prepare-transaction.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/wallets/manager/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/wallets/manager/index.ts
packages/thirdweb/src/wallets/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/wallets/**: UnifiedWalletandAccountinterfaces in wallet architecture
Support for in-app wallets (social/email login)
Smart wallets with account abstraction
EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Files:
packages/thirdweb/src/wallets/manager/index.ts
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/wallets/manager/index.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to test/src/test-wallets.ts : Predefined test accounts are in `test/src/test-wallets.ts`
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Applied to files:
packages/thirdweb/src/wallets/manager/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Build Packages
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/thirdweb/src/wallets/manager/index.ts (1)
244-247: Trim redundant Set when persisting connected wallet ids; confirm 'smart' persistence
- Replace the redundant dedupe with a direct stringify (or use a simpler Set call):
- storage.setItem( - CONNECTED_WALLET_IDS, - stringify(Array.from(new Set([...ids]))), - ); + storage.setItem( + CONNECTED_WALLET_IDS, + stringify(ids), + );Optional dedupe: stringify(Array.from(new Set(ids))).
- Confirm intent for "smart": manager avoids persisting "smart" as LAST_ACTIVE_EOA_ID (packages/thirdweb/src/wallets/manager/index.ts:215-220) but connected IDs are written at index.ts:242-247 and read via getStoredConnectedWalletIds (index.ts:332-339) used by autoConnect (packages/thirdweb/src/wallets/connection/autoConnectCore.ts:79-83). If "smart" should not trigger auto‑connect, filter it out (e.g. ids.filter(id => id !== "smart")) before persisting or explicitly exclude it during auto‑connect.
joaquim-verges
force-pushed
the
_SDK_Fix_wallet_reconnection_for_previously_connected_wallets
branch
from
560ae93 to
c9ad90f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx (5)
563-563: Avoid hard‑coding UI copy; assert via locale key.Use the locale-driven string (e.g., from
localeoren) instead of the literal "Manage Wallet" to prevent brittle tests if copy changes.
511-516: Restore theanimatestub after each test to avoid cross‑test leakage.Add an
afterEachto restore the original method.- beforeEach(() => { - // Mock the animate method - HTMLDivElement.prototype.animate = vi.fn().mockReturnValue({ - onfinish: vi.fn(), - }); - }); + beforeEach(() => { + // Mock the animate method + originalAnimate = HTMLDivElement.prototype.animate as any; + HTMLDivElement.prototype.animate = vi.fn().mockReturnValue({ + onfinish: vi.fn(), + }) as any; + }); + + afterEach(() => { + HTMLDivElement.prototype.animate = originalAnimate as any; + });Add this near the top of the "Details Modal" describe block:
let originalAnimate: any;
311-317: Consolidate module mocking foruseActiveWalletChain.Mock the module once and vary return values per test via
vi.mocked(useActiveWalletChain).mockReturnValue(...), orvi.spyOn(...). Repeatedvi.mock(...)calls can be fragile.Also applies to: 609-615
224-252: Rename test to match behavior.Title says address section should NOT render, but the assertion expects a rendered element with custom name. Rename for clarity.
-it("should NOT render the Address section if detailsButton.connectedAccountName is passed", () => { +it("should render connectedAccountName instead of the address when provided", () => {
37-38: Isolate react‑query state across tests.Sharing a single
QueryClientcan leak cache. Prefer creating a newQueryClientper test (orafterEach(() => queryClient.clear())where used).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.changeset/clean-wings-repeat.md(1 hunks)packages/thirdweb/src/react/core/hooks/wallets/useAutoConnect.ts(0 hunks)packages/thirdweb/src/react/web/hooks/wallets/useAutoConnect.ts(0 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx(1 hunks)packages/thirdweb/src/transaction/prepare-transaction.test.ts(2 hunks)packages/thirdweb/src/wallets/connection/autoConnect.test.ts(0 hunks)packages/thirdweb/src/wallets/connection/autoConnect.ts(0 hunks)packages/thirdweb/src/wallets/connection/autoConnectCore.ts(1 hunks)packages/thirdweb/src/wallets/manager/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- packages/thirdweb/src/wallets/connection/autoConnect.ts
- packages/thirdweb/src/wallets/connection/autoConnect.test.ts
- packages/thirdweb/src/react/web/hooks/wallets/useAutoConnect.ts
- packages/thirdweb/src/react/core/hooks/wallets/useAutoConnect.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/clean-wings-repeat.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/thirdweb/src/wallets/manager/index.ts
- packages/thirdweb/src/transaction/prepare-transaction.test.ts
- packages/thirdweb/src/wallets/connection/autoConnectCore.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to test/src/test-wallets.ts : Predefined test accounts are in `test/src/test-wallets.ts`
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
joaquim-verges
force-pushed
the
_SDK_Fix_wallet_reconnection_for_previously_connected_wallets
branch
from
c9ad90f to
aaa7ea8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/live-stats.tsx (1)
32-49: Handle RPC errors and invalid payloads to avoid NaN/false-success statesIf the RPC returns non-OK or lacks result.number, this path proceeds and may render NaN while not flagging isError. Throw on bad responses so React Query sets isError and the UI shows N/A.
- const res = await fetch(rpcUrl, { - body: JSON.stringify({ + const res = await fetch(rpcUrl, { + method: "POST", + headers: { + "content-type": "application/json", + }, + body: JSON.stringify({ id: 1, jsonrpc: "2.0", method: "eth_getBlockByNumber", params: ["latest", false], - }), - method: "POST", - }); - - const json = await res.json(); + }), + }); + if (!res.ok) { + throw new Error(`RPC error: ${res.status}`); + } + const json = await res.json(); + if (!json?.result?.number) { + throw new Error("Invalid RPC response: missing result.number"); + } const latency = (performance.now() - startTimeStamp).toFixed(0); const blockNumber = Number.parseInt(json.result.number, 16);
🧹 Nitpick comments (4)
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/live-stats.tsx (4)
187-201: Align tooltip copy with new "Available/Not Available" wordingUI text changed below, but tooltips still say "Enabled/Disabled". Make them consistent.
- <ToolTipLabel label="Enabled"> + <ToolTipLabel label="Available"> <CircleCheckIcon className="size-4 text-success-text" /> </ToolTipLabel> ) : ( - <ToolTipLabel label="Disabled"> + <ToolTipLabel label="Not Available"> <XIcon className="size-4 text-destructive-text" /> </ToolTipLabel> ) ) : eip7702Support.isError ? ( - <ToolTipLabel label="Disabled"> + <ToolTipLabel label="Not Available"> <XIcon className="size-4 text-destructive-text" /> </ToolTipLabel> ) : null
206-211: Copy update LGTM; consider consistency with surrounding sections"Available"/"Not Available" looks good. For visual consistency with the Latency/Block sections, consider wrapping these strings in a styled
like those sections.
81-102: Set JSON content-type on EIP‑7702 probe for stricter RPCsSome RPC gateways require content-type; adding it is harmless and improves compatibility.
- const res = await fetch(rpcUrl, { - body: JSON.stringify({ + const res = await fetch(rpcUrl, { + headers: { + "content-type": "application/json", + }, + body: JSON.stringify({ id: 1, jsonrpc: "2.0", method: "eth_estimateGas", params: [ {
106-110: Comment wordingThe code returns isSupported; prefer “supported/available” over “enabled” in the comment for consistency with UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
.changeset/clean-wings-repeat.md(1 hunks)apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/live-stats.tsx(1 hunks)packages/thirdweb/src/react/core/hooks/wallets/useAutoConnect.ts(0 hunks)packages/thirdweb/src/react/web/hooks/wallets/useAutoConnect.ts(0 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx(1 hunks)packages/thirdweb/src/transaction/prepare-transaction.test.ts(2 hunks)packages/thirdweb/src/wallets/connection/autoConnect.test.ts(0 hunks)packages/thirdweb/src/wallets/connection/autoConnect.ts(0 hunks)packages/thirdweb/src/wallets/connection/autoConnectCore.ts(1 hunks)packages/thirdweb/src/wallets/manager/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- packages/thirdweb/src/react/web/hooks/wallets/useAutoConnect.ts
- packages/thirdweb/src/wallets/connection/autoConnect.ts
- packages/thirdweb/src/wallets/connection/autoConnect.test.ts
- packages/thirdweb/src/react/core/hooks/wallets/useAutoConnect.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/clean-wings-repeat.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/thirdweb/src/transaction/prepare-transaction.test.ts
- packages/thirdweb/src/wallets/manager/index.ts
- packages/thirdweb/src/react/web/ui/ConnectWallet/Details.test.tsx
- packages/thirdweb/src/wallets/connection/autoConnectCore.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/live-stats.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/live-stats.tsx
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/live-stats.tsx
apps/{dashboard,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/{dashboard,playground}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/_(e.g., Button, Input, Tabs, Card)
UseNavLinkfor internal navigation to get active state handling
Use Tailwind CSS for styling; no inline styles
Merge class names withcn()from@/lib/utilsfor conditional classes
Stick to design tokens (e.g., bg-card, border-border, text-muted-foreground)
Server Components must start withimport "server-only"; usenext/headers, server‑only env, heavy data fetching, andredirect()where appropriate
Client Components must start with'use client'; handle interactivity with hooks and browser APIs
Server-side data fetching: callgetAuthToken()from cookies, sendAuthorization: Bearer <token>header, and return typed results (avoidany)
Client-side data fetching: wrap calls in React Query with descriptive, stablequeryKeysand set sensiblestaleTime/cacheTime(≥ 60s default); keep tokens secret via internal routes or server actions
Do not importposthog-jsin server components (client-side only)
Files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/live-stats.tsx
apps/{dashboard,playground}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Expose a
classNameprop on the root element of every component
Files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/(chainPage)/components/client/live-stats.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: joaquim-verges
PR: thirdweb-dev/js#7268
File: packages/thirdweb/src/wallets/in-app/core/wallet/in-app-core.ts:210-216
Timestamp: 2025-06-03T23:44:40.243Z
Learning: EIP7702 wallets do not need special handling for switching chains, unlike EIP4337 wallets which require reconnection when switching chains. In the switchChain method condition, EIP7702 should be intentionally excluded from the reconnection logic.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8066 +/- ##
==========================================
- Coverage 56.53% 56.34% -0.20%
==========================================
Files 904 906 +2
Lines 58873 59168 +295
Branches 4165 4173 +8
==========================================
+ Hits 33283 33337 +54
- Misses 25484 25725 +241
Partials 106 106
🚀 New features to boost your workflow:
|
joaquim-verges
deleted the
_SDK_Fix_wallet_reconnection_for_previously_connected_wallets
branch
PR-Codex overview
This PR focuses on improving wallet connection handling and updating UI elements in the
thirdwebpackage. It includes changes to modal texts, wallet availability checks, and the removal of unused functions related to installed wallets.Detailed summary
getInstalledWalletsfunction from various files.Summary by CodeRabbit
New Features
Bug Fixes
UI
Chores