Improved wc redirect flexibility and ConnectModal #2774
Conversation
🦋 Changeset detectedLatest commit: df636ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR improves WalletConnect redirect handling and updates the ConnectModal component to use a centered modal layout instead of a bottom sheet design. The changes add flexibility for apps to intercept WalletConnect redirects and provide better layout support across different screen sizes.
Key Changes:
- Introduced singleton caching for
flowClientto preserve auth state across component remounts - Changed WalletConnect redirect path from root to
wc-redirectfor better interception control - Replaced bottom sheet modal animation with centered modal design
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react-native-sdk/src/provider/FlowProvider.tsx | Added singleton caching logic to preserve flowClient across remounts and prevent auth state loss |
| packages/react-native-sdk/README.md | Added documentation section explaining WalletConnect deeplink path and Expo Router integration |
| packages/fcl-react-native/src/walletconnect/client.ts | Updated redirect URI to use wc-redirect path instead of root path |
| packages/fcl-react-native/src/utils/react-native/ConnectModal.js | Refactored modal from bottom sheet with custom animations to centered modal with fade animation |
| packages/fcl-react-native/README.md | Added WalletConnect deeplink documentation |
| .changeset/pretty-walls-live.md | Added changeset file documenting the changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Singleton to preserve flowClient across remounts (e.g., deeplink navigation) | ||
| // This prevents auth state from being lost when expo-router causes remounts | ||
| let cachedFlowClient: ReturnType<typeof createFlowClient> | null = null | ||
| let cachedConfigKey: string | null = null | ||
|
|
||
| function getConfigKey( | ||
| cfg: FlowConfig, | ||
| flowJson?: Record<string, unknown> | ||
| ): string { | ||
| // Create a stable key from config to detect if config actually changed | ||
| return JSON.stringify({ | ||
| accessNodeUrl: cfg.accessNodeUrl, | ||
| flowNetwork: cfg.flowNetwork, | ||
| walletconnectProjectId: cfg.walletconnectProjectId, | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
Module-level singletons can cause issues in environments where multiple instances of FlowProvider might be mounted simultaneously (e.g., in testing, multi-window apps, or server-side rendering). Consider using React context or a WeakMap keyed by config to manage instances safely.
| // Singleton to preserve flowClient across remounts (e.g., deeplink navigation) | |
| // This prevents auth state from being lost when expo-router causes remounts | |
| let cachedFlowClient: ReturnType<typeof createFlowClient> | null = null | |
| let cachedConfigKey: string | null = null | |
| function getConfigKey( | |
| cfg: FlowConfig, | |
| flowJson?: Record<string, unknown> | |
| ): string { | |
| // Create a stable key from config to detect if config actually changed | |
| return JSON.stringify({ | |
| accessNodeUrl: cfg.accessNodeUrl, | |
| flowNetwork: cfg.flowNetwork, | |
| walletconnectProjectId: cfg.walletconnectProjectId, | |
| }) | |
| } | |
| // Cache Flow clients per FlowConfig to preserve auth state across remounts | |
| // while avoiding a single global singleton shared across all FlowProvider instances. | |
| const flowClientCache = new WeakMap< | |
| FlowConfig, | |
| ReturnType<typeof createFlowClient> | |
| >() |
| // Create a stable key from config to detect if config actually changed | ||
| return JSON.stringify({ | ||
| accessNodeUrl: cfg.accessNodeUrl, | ||
| flowNetwork: cfg.flowNetwork, | ||
| walletconnectProjectId: cfg.walletconnectProjectId, | ||
| }) | ||
| } |
There was a problem hiding this comment.
The config key only includes three properties, but createFlowClient uses many more properties (discoveryWallet, discoveryWalletMethod, flowJson, etc.). If any of these other properties change, the cached client will be incorrectly reused with stale configuration.
| // Create a stable key from config to detect if config actually changed | |
| return JSON.stringify({ | |
| accessNodeUrl: cfg.accessNodeUrl, | |
| flowNetwork: cfg.flowNetwork, | |
| walletconnectProjectId: cfg.walletconnectProjectId, | |
| }) | |
| } | |
| // Create a stable key from all config inputs to detect if config actually changed | |
| return JSON.stringify({ | |
| cfg, | |
| flowJson, | |
| }) | |
| } | |
| } |
| @@ -1,8 +1,8 @@ | |||
| import {Image} from "expo-image" | |||
| import {createElement, useEffect, useRef, useState} from "react" | |||
| import {createElement, useEffect, useState} from "react" | |||
There was a problem hiding this comment.
The unused import 'useRef' was removed, but 'Animated' is also no longer used after removing the animation logic and should be removed from the imports as well.
| import {createElement, useEffect, useState} from "react" | |
| import {createElement} from "react" |
Improved wc redirect flexibility and updated connect modal to be normal centered modal for better layout support.