Skip to content

Commit c48e0c9

Browse files
committed
[SDK] Fix: Disconnect smart wallet when signer is disconnected (#5617)
<!-- start pr-codex --> ## PR-Codex overview This PR focuses on improving error handling and connection management for smart wallets within the `thirdweb` package, enhancing user experience by ensuring proper disconnection and validation of address inputs. ### Detailed summary - Added error handling in `AccountProvider` for missing addresses. - Implemented disconnection logic for smart wallets in `handleSmartWalletConnection`. - Updated tests to cover new error scenarios and connection handling. - Refactored `DetailsModal` to manage modal state based on active accounts. - Improved test coverage for `DetailsModal` and connection manager functionalities. > The following files were skipped due to too many changes: `pnpm-lock.yaml` > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex -->
1 parent 625febb commit c48e0c9

File tree

9 files changed

+1291
-619
lines changed

9 files changed

+1291
-619
lines changed

.changeset/strong-meals-remain.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"thirdweb": patch
3+
---
4+
5+
Fix: Disconnect smart account when account signer is disconnected

packages/thirdweb/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@
309309
"test:watch": "vitest -c ./test/vitest.config.ts dev",
310310
"test": "vitest run -c ./test/vitest.config.ts --coverage",
311311
"test:cov": "vitest dev -c ./test/vitest.config.ts --coverage",
312+
"test:ui": "vitest dev -c ./test/vitest.config.ts --coverage --ui",
312313
"test:dev": "vitest run -c ./test/vitest.config.ts",
313314
"test:react": "vitest run -c ./test/vitest.config.ts dev --ui src/react",
314315
"typedoc": "bun run scripts/typedoc.mjs",
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import {
3+
fireEvent,
4+
render,
5+
screen,
6+
waitFor,
7+
} from "../../../../../test/src/react-render.js";
8+
import { TEST_CLIENT } from "../../../../../test/src/test-clients.js";
9+
import { useActiveAccount } from "../../../core/hooks/wallets/useActiveAccount.js";
10+
import { DetailsModal } from "./Details.js";
11+
import { getConnectLocale } from "./locale/getConnectLocale.js";
12+
13+
vi.mock("../../../core/hooks/wallets/useActiveAccount.js", () => ({
14+
useActiveAccount: vi.fn(),
15+
}));
16+
17+
const mockDetailsModalOptions = {};
18+
const mockSupportedTokens = {};
19+
const mockSupportedNFTs = {};
20+
// biome-ignore lint/suspicious/noExplicitAny: Mock
21+
const mockChains: any[] = [];
22+
const mockDisplayBalanceToken = {};
23+
const mockConnectOptions = {};
24+
// biome-ignore lint/suspicious/noExplicitAny: Mock
25+
const mockAssetTabs: any[] = [];
26+
const mockOnDisconnect = vi.fn();
27+
28+
describe("Details Component", () => {
29+
beforeEach(() => {
30+
// Mock the animate method
31+
HTMLDivElement.prototype.animate = vi.fn().mockReturnValue({
32+
onfinish: vi.fn(),
33+
});
34+
});
35+
36+
it("should close the modal when activeAccount is falsy", async () => {
37+
const closeModalMock = vi.fn();
38+
const locale = await getConnectLocale("en_US");
39+
40+
vi.mocked(useActiveAccount).mockReturnValue(undefined);
41+
42+
render(
43+
<DetailsModal
44+
client={TEST_CLIENT}
45+
locale={locale}
46+
detailsModal={mockDetailsModalOptions}
47+
theme="light"
48+
supportedTokens={mockSupportedTokens}
49+
supportedNFTs={mockSupportedNFTs}
50+
closeModal={closeModalMock}
51+
onDisconnect={mockOnDisconnect}
52+
chains={mockChains}
53+
displayBalanceToken={mockDisplayBalanceToken}
54+
connectOptions={mockConnectOptions}
55+
assetTabs={mockAssetTabs}
56+
/>,
57+
);
58+
59+
await waitFor(() => {
60+
expect(closeModalMock).toHaveBeenCalled();
61+
});
62+
});
63+
64+
it("should render the DetailsModal with default props", async () => {
65+
const closeModalMock = vi.fn();
66+
const locale = await getConnectLocale("en_US");
67+
68+
render(
69+
<DetailsModal
70+
client={TEST_CLIENT}
71+
locale={locale}
72+
theme="light"
73+
closeModal={closeModalMock}
74+
onDisconnect={mockOnDisconnect}
75+
chains={mockChains}
76+
connectOptions={mockConnectOptions}
77+
/>,
78+
);
79+
80+
// Add assertions to check if the modal is rendered correctly
81+
expect(screen.getByText("Connect Modal")).toBeInTheDocument();
82+
});
83+
84+
it("should call closeModal when the close button is clicked", async () => {
85+
const closeModalMock = vi.fn();
86+
const locale = await getConnectLocale("en_US");
87+
88+
render(
89+
<DetailsModal
90+
client={TEST_CLIENT}
91+
locale={locale}
92+
theme="light"
93+
closeModal={closeModalMock}
94+
onDisconnect={mockOnDisconnect}
95+
chains={mockChains}
96+
connectOptions={mockConnectOptions}
97+
/>,
98+
);
99+
100+
// Simulate clicking the close button
101+
fireEvent.click(screen.getByRole("button", { name: /close/i }));
102+
103+
await waitFor(() => {
104+
expect(closeModalMock).toHaveBeenCalled();
105+
});
106+
});
107+
});

packages/thirdweb/src/react/web/ui/ConnectWallet/Details.tsx

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
TextAlignJustifyIcon,
1010
} from "@radix-ui/react-icons";
1111
import { useQuery } from "@tanstack/react-query";
12-
import { type JSX, useContext, useEffect, useState } from "react";
12+
import { type JSX, useCallback, useContext, useEffect, useState } from "react";
1313
import { trackPayEvent } from "../../../../analytics/track/pay.js";
1414
import type { Chain } from "../../../../chains/types.js";
1515
import type { ThirdwebClient } from "../../../../client/client.js";
@@ -291,7 +291,10 @@ export const ConnectedWalletDetails: React.FC<{
291291
);
292292
};
293293

294-
function DetailsModal(props: {
294+
/**
295+
* @internal
296+
*/
297+
export function DetailsModal(props: {
295298
client: ThirdwebClient;
296299
locale: ConnectLocale;
297300
detailsModal?: ConnectButton_detailsModalOptions;
@@ -330,19 +333,25 @@ function DetailsModal(props: {
330333
wallets: activeWallet ? [activeWallet] : [],
331334
});
332335

333-
function closeModal() {
336+
const closeModal = useCallback(() => {
334337
setIsOpen(false);
335338
onModalUnmount(() => {
336339
props.closeModal();
337340
});
338-
}
341+
}, [props.closeModal]);
339342

340343
function handleDisconnect(info: { wallet: Wallet; account: Account }) {
341344
setIsOpen(false);
342345
props.closeModal();
343346
props.onDisconnect(info);
344347
}
345348

349+
useEffect(() => {
350+
if (!activeAccount) {
351+
closeModal();
352+
}
353+
}, [activeAccount, closeModal]);
354+
346355
const networkSwitcherButton = (
347356
<MenuButton
348357
type="button"
@@ -691,20 +700,6 @@ function DetailsModal(props: {
691700
<Text color="primaryText">{props.locale.manageWallet.title}</Text>
692701
</MenuButton>
693702

694-
{/* Switch to Personal Wallet */}
695-
{/* {personalWallet &&
696-
!props.detailsModal?.hideSwitchToPersonalWallet && (
697-
<AccountSwitcher
698-
wallet={personalWallet}
699-
name={locale.personalWallet}
700-
/>
701-
)} */}
702-
703-
{/* Switch to Smart Wallet */}
704-
{/* {smartWallet && (
705-
<AccountSwitcher name={locale.smartWallet} wallet={smartWallet} />
706-
)} */}
707-
708703
{/* Request Testnet funds */}
709704
{(props.detailsModal?.showTestnetFaucet ?? false) &&
710705
(chainFaucetsQuery.faucets.length > 0 ||
@@ -993,12 +988,11 @@ function DetailsModal(props: {
993988
}
994989
}}
995990
>
996-
<AccountProvider
997-
address={activeAccount?.address || ""}
998-
client={client}
999-
>
1000-
{content}
1001-
</AccountProvider>
991+
{activeAccount?.address && (
992+
<AccountProvider address={activeAccount.address} client={client}>
993+
{content}
994+
</AccountProvider>
995+
)}
1002996
</Modal>
1003997
</ScreenSetupContext.Provider>
1004998
</WalletUIStatesProvider>

packages/thirdweb/src/react/web/ui/prebuilt/Account/provider.test.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,20 @@ describe.runIf(process.env.TW_SECRET_KEY)("AccountProvider component", () => {
3535
}),
3636
).toBeInTheDocument();
3737
});
38+
39+
it("should throw an error if no address is provided", () => {
40+
expect(() => {
41+
render(
42+
<AccountProvider
43+
// biome-ignore lint/suspicious/noExplicitAny: testing invalid input
44+
address={undefined as any}
45+
client={TEST_CLIENT}
46+
>
47+
<AccountAddress />
48+
</AccountProvider>,
49+
);
50+
}).toThrowError(
51+
"AccountProvider: No address passed. Ensure an address is always provided to the AccountProvider",
52+
);
53+
});
3854
});

packages/thirdweb/src/react/web/ui/prebuilt/Account/provider.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ const AccountProviderContext = /* @__PURE__ */ createContext<
4848
export function AccountProvider(
4949
props: React.PropsWithChildren<AccountProviderProps>,
5050
) {
51+
if (!props.address) {
52+
throw new Error(
53+
"AccountProvider: No address passed. Ensure an address is always provided to the AccountProvider",
54+
);
55+
}
5156
return (
5257
<AccountProviderContext.Provider value={props}>
5358
{props.children}

0 commit comments

Comments
 (0)