Skip to content

[SDK] Fix showQrModal option not respected on desktop web #7711

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
Jul 25, 2025
Merged

[SDK] Fix showQrModal option not respected on desktop web #7711

Show file tree
Hide file tree
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
5 changes: 5 additions & 0 deletions .changeset/social-dogs-bet.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"thirdweb": patch
---

Fix showQrModal option not respected on desktop web
4 changes: 2 additions & 2 deletions apps/dashboard/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"posthog-js": "1.256.1",
"posthog-node": "^5.4.0",
"prettier": "3.6.2",
"qrcode": "^1.5.3",
"qrcode": "1.5.3",
"react": "19.1.0",
"react-children-utilities": "^2.10.0",
"react-day-picker": "^8.10.1",
Expand Down Expand Up @@ -99,7 +99,7 @@
"@types/node": "22.14.1",
"@types/papaparse": "^5.3.16",
"@types/pluralize": "^0.0.33",
"@types/qrcode": "^1.5.5",
"@types/qrcode": "1.5.5",
"@types/react": "19.1.8",
"@types/react-dom": "19.1.6",
"@types/react-table": "^7.7.20",
Expand Down
16 changes: 13 additions & 3 deletions apps/playground-web/src/app/wallets/sign-in/headless/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ function BuildCustomUISection() {
<CodeExample
code={`\
import { useConnect } from "thirdweb/react";
import { createWallet } from "thirdweb/wallets";
import { createWallet, injectedProvider } from "thirdweb/wallets";
import { shortenAddress } from "thirdweb/utils";

function App() {
const account = useActiveAccount();
const wallet = useActiveWallet();
const { connect, isConnecting, error } = useConnect();
const { connect, isConnecting, error, cancelConnection } = useConnect();
const { disconnect } = useDisconnect();

if (account) {
Expand All @@ -99,7 +99,17 @@ function App() {
connect(async () => {
// 500+ wallets supported with id autocomplete
const wallet = createWallet("io.metamask");
await wallet.connect({ client });
const isInstalled = !!injectedProvider("io.metamask");
await wallet.connect({ client, ...(isInstalled ? {} : {
walletConnect: {
// if not installed, show qr modal
showQrModal: true,
onCancel: () => {
cancelConnection();
}
}
})
});
return wallet;
})
}
Expand Down
13 changes: 12 additions & 1 deletion apps/playground-web/src/components/sign-in/hooks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
useDisconnect,
} from "thirdweb/react";
import { shortenAddress } from "thirdweb/utils";
import { createWallet } from "thirdweb/wallets";
import { createWallet, injectedProvider } from "thirdweb/wallets";
import { THIRDWEB_CLIENT } from "../../lib/client";
import { Button } from "../ui/button";

Expand All @@ -21,6 +21,17 @@ export function HooksPreview() {
const adminWallet = createWallet("io.metamask");
await adminWallet.connect({
client: THIRDWEB_CLIENT,
...(injectedProvider("io.metamask")
? {}
: {
walletConnect: {
showQrModal: true,
onCancel: () => {
console.log("onCancel");
connectMutation.cancelConnection();
},
},
}),
});
return adminWallet;
});
Expand Down
1 change: 0 additions & 1 deletion apps/playground-web/src/components/sign-in/modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export function ModalPreview({ enableAuth }: { enableAuth?: boolean }) {
auth: enableAuth ? playgroundAuth : undefined,
client: THIRDWEB_CLIENT,
});
console.log("connected", wallet);
return wallet;
};

Expand Down
28 changes: 25 additions & 3 deletions apps/portal/src/components/Document/AuthMethodsTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import type React from "react";
import { useState } from "react";
import { useMemo, useState } from "react";
import { getSocialIcon } from "thirdweb/wallets/in-app";
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
import {
Expand Down Expand Up @@ -819,7 +819,26 @@ const getUnrealSnippet = (authMethod: AuthMethod): string => {

function AuthMethodsTabsContent() {
const [selectedAuth, setSelectedAuth] = useState<AuthMethod>("email");
const platforms = getPlatformsForAuth(selectedAuth);
const [selectedPlatform, setSelectedPlatform] = useState<Platform | null>(
null,
);
const platforms = useMemo(
() => getPlatformsForAuth(selectedAuth),
[selectedAuth],
);

// Reset platform selection when platforms change
const currentPlatform = useMemo(() => {
const defaultPlatform = platforms[0]?.id || "typescript";
// If currently selected platform is not available in new platforms, reset to first available
if (
!selectedPlatform ||
!platforms.find((p) => p.id === selectedPlatform)
) {
return defaultPlatform;
}
return selectedPlatform;
}, [platforms, selectedPlatform]);

return (
<div className="space-y-6">
Expand Down Expand Up @@ -858,7 +877,10 @@ function AuthMethodsTabsContent() {
<h3 className="text-lg font-semibold mb-4 text-foreground">
2. Select Platform
</h3>
<Tabs defaultValue={platforms[0]?.id || "typescript"}>
<Tabs
value={currentPlatform}
onValueChange={(value) => setSelectedPlatform(value as Platform)}
>
<TabsList>
{platforms.map((platform) => {
const IconComponent = platform.icon;
Expand Down
2 changes: 2 additions & 0 deletions packages/thirdweb/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"ora": "8.2.0",
"ox": "0.7.0",
"prompts": "2.4.2",
"qrcode": "1.5.3",
"toml": "3.0.0",
"uqr": "0.1.2",
"viem": "2.28.1",
Expand All @@ -61,6 +62,7 @@
"@testing-library/user-event": "^14.6.1",
"@types/cross-spawn": "^6.0.6",
"@types/prompts": "2.4.9",
"@types/qrcode": "1.5.5",
"@types/react": "19.1.8",
"@viem/anvil": "0.0.10",
"@vitejs/plugin-react": "^4.6.0",
Expand Down
12 changes: 11 additions & 1 deletion packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,15 @@
[connect, options, setConnectionStatus],
);

return { connect: handleConnection, error, isConnecting } as const;
const cancelConnection = useCallback(() => {
setIsConnecting(false);
setConnectionStatus("disconnected");

Check warning on line 84 in packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts#L83-L84 

Added lines #L83 - L84 were not covered by tests
}, [setConnectionStatus]);

return {
connect: handleConnection,
error,
isConnecting,
cancelConnection,
} as const;
Comment on lines +87 to +92
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 25, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify no race conditions with ongoing connections.

The return object correctly exposes the new cancelConnection function. Ensure that calling cancelConnection during an active connection properly handles cleanup and doesn't cause race conditions with the ongoing handleConnection process.

🏁 Script executed:

#!/bin/bash
# Search for any cleanup logic in wallet connection flows that might need coordination with cancelConnection
ast-grep --pattern 'const $_ = useConnect($_)'

Length of output: 913

🏁 Script executed:

#!/bin/bash
# Locate cancelConnection definition
grep -R "cancelConnection" -n packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts

# Inspect handleConnection implementation for cancellation logic
grep -R "handleConnection" -n packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts

# Search for AbortController usage to manage cancellation
grep -R "AbortController" -n packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts

Length of output: 423

🏁 Script executed:

#!/bin/bash
# Display the relevant portion containing handleConnection and cancelConnection implementations
sed -n '1,120p' packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts

Length of output: 2792

Coordinate cancellation to avoid racing state updates

The current cancelConnection (lines 82–85) only flips the React state but doesn’t actually stop the in-flight promise in handleConnection (lines 53–76). This means a cancelled connection can still resolve or reject later and overwrite your status flags. To fix:

• In packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts
– Introduce an AbortController (or a ref-based isCancelled flag) at the hook level.
– Pass its signal into both walletOrFn() and connect(...) so that pending work can be aborted.
– In cancelConnection, call controller.abort() (or set the flag) before resetting state.

Example diff:

+ const controllerRef = useRef<AbortController>();
  const handleConnection = useCallback(
    async (walletOrFn: Wallet | (() => Promise<Wallet>)) => {
+     const controller = new AbortController();
+     controllerRef.current = controller;
      setError(null);
      setConnectionStatus("connecting");
+     if (controller.signal.aborted) return null;
      if (typeof walletOrFn !== "function") {
-       const account = await connect(walletOrFn, options);
+       const account = await connect(walletOrFn, { ...options, signal: controller.signal });
        setConnectionStatus("connected");
        return account;
      }

      setIsConnecting(true);
      try {
-       const w = await walletOrFn();
+       const w = await Promise.race([
+         walletOrFn(),
+         new Promise((_, rej) =>
+           controller.signal.addEventListener("abort", () => rej(new Error("cancelled")))
+         ),
+       ]);
-       const account = await connect(w, options);
+       const account = await connect(w, { ...options, signal: controller.signal });
        setConnectionStatus("connected");
        return account;
      } catch (e) {
        console.error(e);
        setError(e as Error);
        setConnectionStatus("disconnected");
      } finally {
        setIsConnecting(false);
      }
      return null;
    },
    [connect, options, setConnectionStatus],
  );

  const cancelConnection = useCallback(() => {
+   controllerRef.current?.abort();
    setIsConnecting(false);
    setConnectionStatus("disconnected");
  }, [setConnectionStatus]);

This ensures that calling cancelConnection truly aborts the ongoing flow and prevents late-settling promises from clobbering the UI.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return {
connect: handleConnection,
error,
isConnecting,
cancelConnection,
} as const;
// Introduce a ref to hold the current AbortController
const controllerRef = useRef<AbortController>();
const handleConnection = useCallback(
async (walletOrFn: Wallet | (() => Promise<Wallet>)) => {
// Create a fresh controller for this invocation
const controller = new AbortController();
controllerRef.current = controller;
setError(null);
setConnectionStatus("connecting");
// If already aborted, bail out
if (controller.signal.aborted) return null;
if (typeof walletOrFn !== "function") {
// Pass the abort signal into connect
const account = await connect(walletOrFn, { ...options, signal: controller.signal });
setConnectionStatus("connected");
return account;
}
setIsConnecting(true);
try {
// Race wallet factory against an abort event
const w = await Promise.race([
walletOrFn(),
new Promise<never>((_, rej) =>
controller.signal.addEventListener("abort", () => rej(new Error("cancelled")))
),
]);
const account = await connect(w, { ...options, signal: controller.signal });
setConnectionStatus("connected");
return account;
} catch (e) {
console.error(e);
setError(e as Error);
setConnectionStatus("disconnected");
} finally {
setIsConnecting(false);
}
return null;
},
[connect, options, setConnectionStatus],
);
const cancelConnection = useCallback(() => {
// Abort any in-flight connection attempt
controllerRef.current?.abort();
setIsConnecting(false);
setConnectionStatus("disconnected");
}, [setConnectionStatus]);
return {
connect: handleConnection,
error,
isConnecting,
cancelConnection,
} as const;
🤖 Prompt for AI Agents 
In packages/thirdweb/src/react/core/hooks/wallets/useConnect.ts around lines 53
to 92, the cancelConnection function only updates React state but does not stop
the ongoing async connection process, causing potential race conditions. To fix
this, add an AbortController or a ref-based cancellation flag at the hook level,
pass its signal or flag into walletOrFn() and connect() calls to allow aborting
the async operations, and modify cancelConnection to call controller.abort() or
set the cancellation flag before resetting state. This will ensure that
cancelling truly aborts the connection flow and prevents stale promise
resolutions from updating state.

}
109 changes: 85 additions & 24 deletions packages/thirdweb/src/wallets/create-wallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import { getInjectedProvider } from "./injected/index.js";
import type { Account, Wallet } from "./interfaces/wallet.js";
import { smartWallet } from "./smart/smart-wallet.js";
import type { QROverlay } from "./wallet-connect/qr-overlay.js";
import type { WCConnectOptions } from "./wallet-connect/types.js";
import { createWalletEmitter } from "./wallet-emitter.js";
import type {
Expand Down Expand Up @@ -299,30 +300,90 @@
"./wallet-connect/controller.js"
);

const [
connectedAccount,
connectedChain,
doDisconnect,
doSwitchChain,
] = await connectWC(
wcOptions,
emitter,
wallet.id as WCSupportedWalletIds | "walletConnect",
webLocalStorage,
sessionHandler,
);
// set the states
account = connectedAccount;
chain = connectedChain;
handleDisconnect = doDisconnect;
handleSwitchChain = doSwitchChain;
trackConnect({
chainId: chain.id,
client: wcOptions.client,
walletAddress: account.address,
walletType: id,
});
return account;
let qrOverlay: QROverlay | undefined;

Check warning on line 303 in packages/thirdweb/src/wallets/create-wallet.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/create-wallet.ts#L303 

Added line #L303 was not covered by tests

try {
const [
connectedAccount,
connectedChain,
doDisconnect,
doSwitchChain,
] = await connectWC(
{
...wcOptions,
walletConnect: {
...wcOptions.walletConnect,
onDisplayUri: wcOptions.walletConnect?.showQrModal
? async (uri) => {

Check warning on line 317 in packages/thirdweb/src/wallets/create-wallet.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/create-wallet.ts#L305-L317 

Added lines #L305 - L317 were not covered by tests
// Check if we're in a browser environment
if (
typeof window !== "undefined" &&
typeof document !== "undefined"
) {
try {
const { createQROverlay } = await import(
"./wallet-connect/qr-overlay.js"
);

Check warning on line 326 in packages/thirdweb/src/wallets/create-wallet.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/create-wallet.ts#L319-L326 

Added lines #L319 - L326 were not covered by tests

// Clean up any existing overlay
if (qrOverlay) {
qrOverlay.destroy();
}

Check warning on line 331 in packages/thirdweb/src/wallets/create-wallet.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/create-wallet.ts#L329-L331 

Added lines #L329 - L331 were not covered by tests

// Create new QR overlay
qrOverlay = createQROverlay(uri, {
theme:
wcOptions.walletConnect?.qrModalOptions
?.themeMode ?? "dark",
qrSize: 280,
showCloseButton: true,
onCancel: () => {
wcOptions.walletConnect?.onCancel?.();
},
});
} catch (error) {
console.error(
"Failed to create QR overlay:",
error,
);
}
}
}
: undefined,
},
},
emitter,
wallet.id as WCSupportedWalletIds | "walletConnect",
webLocalStorage,
sessionHandler,
);

Check warning on line 359 in packages/thirdweb/src/wallets/create-wallet.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/create-wallet.ts#L334-L359 

Added lines #L334 - L359 were not covered by tests

// Clean up QR overlay on successful connection
if (qrOverlay) {
qrOverlay.destroy();
qrOverlay = undefined;
}

Check warning on line 365 in packages/thirdweb/src/wallets/create-wallet.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/create-wallet.ts#L362-L365 

Added lines #L362 - L365 were not covered by tests

// set the states
account = connectedAccount;
chain = connectedChain;
handleDisconnect = doDisconnect;
handleSwitchChain = doSwitchChain;
trackConnect({
chainId: chain.id,
client: wcOptions.client,
walletAddress: account.address,
walletType: id,
});
return account;
} catch (error) {

Check warning on line 379 in packages/thirdweb/src/wallets/create-wallet.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/create-wallet.ts#L368-L379 

Added lines #L368 - L379 were not covered by tests
// Clean up QR overlay on connection error
if (qrOverlay) {
qrOverlay.destroy();
qrOverlay = undefined;
}
throw error;
}

Check warning on line 386 in packages/thirdweb/src/wallets/create-wallet.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/create-wallet.ts#L381-L386 

Added lines #L381 - L386 were not covered by tests
}

if (id === "walletConnect") {
Expand Down
Loading
Loading