-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: implement lazy-loading for Coinbase Wallet SDK #5152
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| --- | ||
| '@reown/appkit-adapter-ethers5': patch | ||
| '@reown/appkit-adapter-ethers': patch | ||
| '@reown/appkit-adapter-wagmi': patch | ||
| 'pay-test-exchange': patch | ||
| '@reown/appkit-adapter-bitcoin': patch | ||
| '@reown/appkit-adapter-solana': patch | ||
| '@reown/appkit': patch | ||
| '@reown/appkit-utils': patch | ||
| '@reown/appkit-cdn': patch | ||
| '@reown/appkit-cli': patch | ||
| '@reown/appkit-codemod': patch | ||
| '@reown/appkit-common': patch | ||
| '@reown/appkit-controllers': patch | ||
| '@reown/appkit-core': patch | ||
| '@reown/appkit-experimental': patch | ||
| '@reown/appkit-pay': patch | ||
| '@reown/appkit-polyfills': patch | ||
| '@reown/appkit-scaffold-ui': patch | ||
| '@reown/appkit-siwe': patch | ||
| '@reown/appkit-siwx': patch | ||
| '@reown/appkit-testing': patch | ||
| '@reown/appkit-ui': patch | ||
| '@reown/appkit-universal-connector': patch | ||
| '@reown/appkit-wallet': patch | ||
| '@reown/appkit-wallet-button': patch | ||
| --- | ||
|
|
||
| Lazy load Coinbase SDK |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,7 @@ export class EthersAdapter extends AdapterBlueprint { | |||||||||||
| private ethersConfig?: ProviderType | ||||||||||||
| private balancePromises: Record<string, Promise<AdapterBlueprint.GetBalanceResult>> = {} | ||||||||||||
| private universalProvider?: UniversalProvider | ||||||||||||
| private options?: AppKitOptions | ||||||||||||
|
|
||||||||||||
| constructor() { | ||||||||||||
| super({ | ||||||||||||
|
|
@@ -45,6 +46,34 @@ export class EthersAdapter extends AdapterBlueprint { | |||||||||||
| }) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| private async getCoinbaseProviderLazy(): Promise<Provider | undefined> { | ||||||||||||
| try { | ||||||||||||
| const { createCoinbaseWalletSDK } = await import('@coinbase/wallet-sdk') | ||||||||||||
| if (typeof window === 'undefined') { | ||||||||||||
| return undefined | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const networks = this.getCaipNetworks()?.filter( | ||||||||||||
| n => n.chainNamespace === CommonConstantsUtil.CHAIN.EVM | ||||||||||||
| ) | ||||||||||||
| const coinbaseSdk = createCoinbaseWalletSDK({ | ||||||||||||
| appName: this.options?.metadata?.name, | ||||||||||||
| appLogoUrl: this.options?.metadata?.icons?.[0], | ||||||||||||
| appChainIds: networks?.map(n => n.id as number) || [1, 84532], | ||||||||||||
| preference: { | ||||||||||||
| options: this.options?.coinbasePreference ?? 'all' | ||||||||||||
| } | ||||||||||||
| }) | ||||||||||||
|
|
||||||||||||
| return coinbaseSdk.getProvider() as unknown as Provider | ||||||||||||
| } catch (error) { | ||||||||||||
| // eslint-disable-next-line no-console | ||||||||||||
| console.error('Failed to import Coinbase Wallet SDK:', error) | ||||||||||||
|
|
||||||||||||
| return undefined | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| private async createEthersConfig(options: AppKitOptions) { | ||||||||||||
| if (!options.metadata) { | ||||||||||||
| return undefined | ||||||||||||
|
|
@@ -85,43 +114,15 @@ export class EthersAdapter extends AdapterBlueprint { | |||||||||||
| return provider | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| async function getCoinbaseProvider() { | ||||||||||||
| try { | ||||||||||||
| const { createCoinbaseWalletSDK } = await import('@coinbase/wallet-sdk') | ||||||||||||
| if (typeof window === 'undefined') { | ||||||||||||
| return undefined | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const coinbaseSdk = createCoinbaseWalletSDK({ | ||||||||||||
| appName: options?.metadata?.name, | ||||||||||||
| appLogoUrl: options?.metadata?.icons[0], | ||||||||||||
| appChainIds: options.networks?.map(caipNetwork => caipNetwork.id as number) || [1, 84532], | ||||||||||||
| preference: { | ||||||||||||
| options: options.coinbasePreference ?? 'all' | ||||||||||||
| } | ||||||||||||
| }) | ||||||||||||
|
|
||||||||||||
| return coinbaseSdk.getProvider() | ||||||||||||
| } catch (error) { | ||||||||||||
| // eslint-disable-next-line no-console | ||||||||||||
| console.error('Failed to import Coinbase Wallet SDK:', error) | ||||||||||||
|
|
||||||||||||
| return undefined | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const providers: ProviderType = { metadata: options.metadata } | ||||||||||||
|
|
||||||||||||
| if (options.enableInjected !== false) { | ||||||||||||
| providers.injected = getInjectedProvider() | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if (options.enableCoinbase !== false) { | ||||||||||||
| const coinbaseProvider = await getCoinbaseProvider() | ||||||||||||
|
|
||||||||||||
| if (coinbaseProvider) { | ||||||||||||
| providers.coinbase = coinbaseProvider | ||||||||||||
| } | ||||||||||||
| // Register Coinbase connector without instantiating SDK yet (lazy-load on connect) | ||||||||||||
| providers.coinbase = undefined | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if (CoreHelperUtil.isSafeApp()) { | ||||||||||||
|
|
@@ -277,6 +278,7 @@ export class EthersAdapter extends AdapterBlueprint { | |||||||||||
| } | ||||||||||||
|
|
||||||||||||
| override async syncConnectors(options: AppKitOptions): Promise<void> { | ||||||||||||
| this.options = options | ||||||||||||
| this.ethersConfig = await this.createEthersConfig(options) | ||||||||||||
|
|
||||||||||||
| if (this.ethersConfig?.EIP6963) { | ||||||||||||
|
|
@@ -450,7 +452,21 @@ export class EthersAdapter extends AdapterBlueprint { | |||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const selectedProvider = connector?.provider as Provider | ||||||||||||
| let selectedProvider = connector?.provider as Provider | undefined | ||||||||||||
|
|
||||||||||||
| // Lazy-load Coinbase SDK only when user selects it | ||||||||||||
| if ( | ||||||||||||
| !selectedProvider && | ||||||||||||
| (connector?.id === CommonConstantsUtil.CONNECTOR_ID.COINBASE_SDK || | ||||||||||||
| connector?.id === 'coinbase') | ||||||||||||
| ) { | ||||||||||||
| selectedProvider = await this.getCoinbaseProviderLazy() | ||||||||||||
| if (selectedProvider) { | ||||||||||||
| // Persist on the connector so listeners work normally | ||||||||||||
| // eslint-disable-next-line no-param-reassign | ||||||||||||
| connector.provider = selectedProvider | ||||||||||||
|
Comment on lines
+466
to
+467
|
||||||||||||
| // eslint-disable-next-line no-param-reassign | |
| connector.provider = selectedProvider | |
| // Use a new connector object to preserve immutability | |
| const updatedConnector = { ...connector, provider: selectedProvider } | |
| connector = updatedConnector |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -38,6 +38,7 @@ export class Ethers5Adapter extends AdapterBlueprint { | |||||||||||||
| private ethersConfig?: ProviderType | ||||||||||||||
| private balancePromises: Record<string, Promise<AdapterBlueprint.GetBalanceResult>> = {} | ||||||||||||||
| private universalProvider?: UniversalProvider | ||||||||||||||
| private options?: AppKitOptions | ||||||||||||||
|
|
||||||||||||||
| constructor() { | ||||||||||||||
| super({ | ||||||||||||||
|
|
@@ -46,6 +47,34 @@ export class Ethers5Adapter extends AdapterBlueprint { | |||||||||||||
| }) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private async getCoinbaseProviderLazy(): Promise<Provider | undefined> { | ||||||||||||||
| try { | ||||||||||||||
| const { createCoinbaseWalletSDK } = await import('@coinbase/wallet-sdk') | ||||||||||||||
| if (typeof window === 'undefined') { | ||||||||||||||
| return undefined | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| const networks = this.getCaipNetworks()?.filter( | ||||||||||||||
| n => n.chainNamespace === CommonConstantsUtil.CHAIN.EVM | ||||||||||||||
| ) | ||||||||||||||
| const coinbaseSdk = createCoinbaseWalletSDK({ | ||||||||||||||
| appName: this.options?.metadata?.name, | ||||||||||||||
| appLogoUrl: this.options?.metadata?.icons?.[0], | ||||||||||||||
| appChainIds: networks?.map(n => n.id as number) || [1, 84532], | ||||||||||||||
| preference: { | ||||||||||||||
| options: this.options?.coinbasePreference ?? 'all' | ||||||||||||||
| } | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| return coinbaseSdk.getProvider() as unknown as Provider | ||||||||||||||
| } catch (error) { | ||||||||||||||
| // eslint-disable-next-line no-console | ||||||||||||||
| console.error('Failed to import Coinbase Wallet SDK:', error) | ||||||||||||||
|
|
||||||||||||||
| return undefined | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private async createEthersConfig(options: AppKitOptions) { | ||||||||||||||
| if (!options.metadata) { | ||||||||||||||
| return undefined | ||||||||||||||
|
|
@@ -87,44 +116,15 @@ export class Ethers5Adapter extends AdapterBlueprint { | |||||||||||||
| return provider | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| async function getCoinbaseProvider() { | ||||||||||||||
| try { | ||||||||||||||
| const { createCoinbaseWalletSDK } = await import('@coinbase/wallet-sdk') | ||||||||||||||
|
|
||||||||||||||
| if (typeof window === 'undefined') { | ||||||||||||||
| return undefined | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| const coinbaseSdk = createCoinbaseWalletSDK({ | ||||||||||||||
| appName: options?.metadata?.name, | ||||||||||||||
| appLogoUrl: options?.metadata?.icons[0], | ||||||||||||||
| appChainIds: options.networks?.map(caipNetwork => caipNetwork.id as number) || [1, 84532], | ||||||||||||||
| preference: { | ||||||||||||||
| options: options.coinbasePreference ?? 'all' | ||||||||||||||
| } | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| return coinbaseSdk.getProvider() | ||||||||||||||
| } catch (error) { | ||||||||||||||
| // eslint-disable-next-line no-console | ||||||||||||||
| console.error('Failed to import Coinbase Wallet SDK:', error) | ||||||||||||||
|
|
||||||||||||||
| return undefined | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| const providers: ProviderType = { metadata: options.metadata } | ||||||||||||||
|
|
||||||||||||||
| if (options.enableInjected !== false) { | ||||||||||||||
| providers.injected = getInjectedProvider() | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if (options.enableCoinbase !== false) { | ||||||||||||||
| const coinbaseProvider = await getCoinbaseProvider() | ||||||||||||||
|
|
||||||||||||||
| if (coinbaseProvider) { | ||||||||||||||
| providers.coinbase = coinbaseProvider | ||||||||||||||
| } | ||||||||||||||
| // Register Coinbase connector without instantiating SDK yet (lazy-load on connect) | ||||||||||||||
| providers.coinbase = undefined | ||||||||||||||
|
Comment on lines
+126
to
+127
|
||||||||||||||
| // Register Coinbase connector without instantiating SDK yet (lazy-load on connect) | |
| providers.coinbase = undefined | |
| // Register Coinbase connector with a lazy provider wrapper (lazy-load on connect) | |
| providers.coinbase = { | |
| getProvider: () => this.getCoinbaseProviderLazy() | |
| } |
Copilot
AI
Oct 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly mutating the connector parameter violates immutability principles. Consider using a more functional approach or documenting why this mutation is necessary for the architecture.
| // Persist on the connector so listeners work normally | |
| // eslint-disable-next-line no-param-reassign | |
| connector.provider = selectedProvider | |
| // Create a new connector object with the updated provider to preserve immutability | |
| connector = { ...connector, provider: selectedProvider } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting providers.coinbase to undefined may cause issues downstream. Consider using a placeholder object or lazy provider wrapper instead of undefined to maintain consistent provider interface expectations.