Skip to content

Commit

Permalink
refactor: connector listeners (#3788)
Browse files Browse the repository at this point in the history
* refactor: connector listeners

* chore: changeset

* refactor: listener
  • Loading branch information
tmm authored Apr 4, 2024
1 parent f85b83a commit 42ad380
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 50 deletions.
6 changes: 6 additions & 0 deletions .changeset/long-dogs-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@wagmi/connectors": patch
"@wagmi/core": patch
---

Refactored connectors to remove unnecessarily event listeners.
57 changes: 46 additions & 11 deletions packages/connectors/src/coinbaseWallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import {
type CoinbaseWalletProvider,
type CoinbaseWalletSDK,
} from '@coinbase/wallet-sdk'
import { ChainNotConfiguredError, createConnector } from '@wagmi/core'
import {
ChainNotConfiguredError,
type Connector,
createConnector,
} from '@wagmi/core'
import type { Evaluate, Mutable, Omit } from '@wagmi/core/internal'
import {
type ProviderRpcError,
Expand Down Expand Up @@ -48,6 +52,10 @@ export function coinbaseWallet(parameters: CoinbaseWalletParameters) {
let sdk: CoinbaseWalletSDK | undefined
let walletProvider: Provider | undefined

let accountsChanged: Connector['onAccountsChanged'] | undefined
let chainChanged: Connector['onChainChanged'] | undefined
let disconnect: Connector['onDisconnect'] | undefined

return createConnector<Provider, Properties>((config) => ({
id: 'coinbaseWalletSDK',
name: 'Coinbase Wallet',
Expand All @@ -61,9 +69,18 @@ export function coinbaseWallet(parameters: CoinbaseWalletParameters) {
})) as string[]
).map((x) => getAddress(x))

provider.on('accountsChanged', this.onAccountsChanged)
provider.on('chainChanged', this.onChainChanged)
provider.on('disconnect', this.onDisconnect.bind(this))
if (!accountsChanged) {
accountsChanged = this.onAccountsChanged.bind(this)
provider.on('accountsChanged', accountsChanged)
}
if (!chainChanged) {
chainChanged = this.onChainChanged.bind(this)
provider.on('chainChanged', chainChanged)
}
if (!disconnect) {
disconnect = this.onDisconnect.bind(this)
provider.on('disconnect', disconnect)
}

// Switch to chain if provided
let currentChainId = await this.getChainId()
Expand All @@ -89,9 +106,18 @@ export function coinbaseWallet(parameters: CoinbaseWalletParameters) {
async disconnect() {
const provider = await this.getProvider()

provider.removeListener('accountsChanged', this.onAccountsChanged)
provider.removeListener('chainChanged', this.onChainChanged)
provider.removeListener('disconnect', this.onDisconnect.bind(this))
if (accountsChanged) {
provider.removeListener('accountsChanged', accountsChanged)
accountsChanged = undefined
}
if (chainChanged) {
provider.removeListener('chainChanged', chainChanged)
chainChanged = undefined
}
if (disconnect) {
provider.removeListener('disconnect', disconnect)
disconnect = undefined
}

provider.disconnect()
provider.close()
Expand Down Expand Up @@ -193,7 +219,7 @@ export function coinbaseWallet(parameters: CoinbaseWalletParameters) {
}
},
onAccountsChanged(accounts) {
if (accounts.length === 0) config.emitter.emit('disconnect')
if (accounts.length === 0) this.onDisconnect()
else
config.emitter.emit('change', {
accounts: accounts.map((x) => getAddress(x)),
Expand All @@ -207,9 +233,18 @@ export function coinbaseWallet(parameters: CoinbaseWalletParameters) {
config.emitter.emit('disconnect')

const provider = await this.getProvider()
provider.removeListener('accountsChanged', this.onAccountsChanged)
provider.removeListener('chainChanged', this.onChainChanged)
provider.removeListener('disconnect', this.onDisconnect.bind(this))
if (accountsChanged) {
provider.removeListener('accountsChanged', accountsChanged)
accountsChanged = undefined
}
if (chainChanged) {
provider.removeListener('chainChanged', chainChanged)
chainChanged = undefined
}
if (disconnect) {
provider.removeListener('disconnect', disconnect)
disconnect = undefined
}
},
}))
}
18 changes: 15 additions & 3 deletions packages/connectors/src/safe.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { type SafeAppProvider } from '@safe-global/safe-apps-provider'
import { type Opts } from '@safe-global/safe-apps-sdk'
import { ProviderNotFoundError, createConnector } from '@wagmi/core'
import {
type Connector,
ProviderNotFoundError,
createConnector,
} from '@wagmi/core'
import type { Evaluate } from '@wagmi/core/internal'
import { getAddress } from 'viem'

Expand Down Expand Up @@ -28,6 +32,8 @@ export function safe(parameters: SafeParameters = {}) {

let provider_: Provider | undefined

let disconnect: Connector['onDisconnect'] | undefined

return createConnector<Provider, Properties, StorageItem>((config) => ({
id: 'safe',
name: 'Safe',
Expand All @@ -39,7 +45,10 @@ export function safe(parameters: SafeParameters = {}) {
const accounts = await this.getAccounts()
const chainId = await this.getChainId()

provider.on('disconnect', this.onDisconnect.bind(this))
if (!disconnect) {
disconnect = this.onDisconnect.bind(this)
provider.on('disconnect', disconnect)
}

// Remove disconnected shim if it exists
if (shimDisconnect) await config.storage?.removeItem('safe.disconnected')
Expand All @@ -50,7 +59,10 @@ export function safe(parameters: SafeParameters = {}) {
const provider = await this.getProvider()
if (!provider) throw new ProviderNotFoundError()

provider.removeListener('disconnect', this.onDisconnect.bind(this))
if (disconnect) {
provider.removeListener('disconnect', disconnect)
disconnect = undefined
}

// Add shim signalling connector is disconnected
if (shimDisconnect)
Expand Down
122 changes: 93 additions & 29 deletions packages/core/src/connectors/injected.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
type Address,
type EIP1193EventMap,
type EIP1193Provider,
type ProviderConnectInfo,
ProviderRpcError,
Expand All @@ -13,6 +14,7 @@ import {
withTimeout,
} from 'viem'

import type { Connector } from '../createConfig.js'
import { ChainNotConfiguredError } from '../errors/config.js'
import { ProviderNotFoundError } from '../errors/connector.js'
import { type Evaluate } from '../types/utils.js'
Expand Down Expand Up @@ -124,6 +126,11 @@ export function injected(parameters: InjectedParameters = {}) {
[_ in 'injected.connected' | `${string}.disconnected`]: true
}

let accountsChanged: Connector['onAccountsChanged'] | undefined
let chainChanged: Connector['onChainChanged'] | undefined
let connect: Connector['onConnect'] | undefined
let disconnect: Connector['onDisconnect'] | undefined

return createConnector<Provider, Properties, StorageItem>((config) => ({
get icon() {
return getTarget().icon
Expand All @@ -138,8 +145,19 @@ export function injected(parameters: InjectedParameters = {}) {
async setup() {
const provider = await this.getProvider()
// Only start listening for events if `target` is set, otherwise `injected()` will also receive events
if (provider && parameters.target)
provider.on('connect', this.onConnect.bind(this))
if (provider && parameters.target) {
if (!connect) {
connect = this.onConnect.bind(this)
provider.on('connect', connect)
}

// We shouldn't need to listen for `'accountsChanged'` here since the `'connect'` event should suffice (and wallet shouldn't be connected yet).
// Some wallets, like MetaMask, do not implement the `'connect'` event and overload `'accountsChanged'` instead.
if (!accountsChanged) {
accountsChanged = this.onAccountsChanged.bind(this)
provider.on('accountsChanged', accountsChanged)
}
}
},
async connect({ chainId, isReconnecting } = {}) {
const provider = await this.getProvider()
Expand Down Expand Up @@ -176,10 +194,24 @@ export function injected(parameters: InjectedParameters = {}) {
accounts = requestedAccounts.map((x) => getAddress(x))
}

provider.removeListener('connect', this.onConnect.bind(this))
provider.on('accountsChanged', this.onAccountsChanged.bind(this))
provider.on('chainChanged', this.onChainChanged)
provider.on('disconnect', this.onDisconnect.bind(this))
// Manage EIP-1193 event listeners
// https://eips.ethereum.org/EIPS/eip-1193#events
if (connect) {
provider.removeListener('connect', connect)
connect = undefined
}
if (!accountsChanged) {
accountsChanged = this.onAccountsChanged.bind(this)
provider.on('accountsChanged', accountsChanged)
}
if (!chainChanged) {
chainChanged = this.onChainChanged.bind(this)
provider.on('chainChanged', chainChanged)
}
if (!disconnect) {
disconnect = this.onDisconnect.bind(this)
provider.on('disconnect', disconnect)
}

// Switch to chain if provided
let currentChainId = await this.getChainId()
Expand Down Expand Up @@ -213,13 +245,19 @@ export function injected(parameters: InjectedParameters = {}) {
const provider = await this.getProvider()
if (!provider) throw new ProviderNotFoundError()

provider.removeListener(
'accountsChanged',
this.onAccountsChanged.bind(this),
)
provider.removeListener('chainChanged', this.onChainChanged)
provider.removeListener('disconnect', this.onDisconnect.bind(this))
provider.on('connect', this.onConnect.bind(this))
// Manage EIP-1193 event listeners
if (chainChanged) {
provider.removeListener('chainChanged', chainChanged)
chainChanged = undefined
}
if (disconnect) {
provider.removeListener('disconnect', disconnect)
disconnect = undefined
}
if (!connect) {
connect = this.onConnect.bind(this)
provider.on('connect', connect)
}

// Add shim signalling connector is disconnected
if (shimDisconnect) {
Expand Down Expand Up @@ -348,11 +386,16 @@ export function injected(parameters: InjectedParameters = {}) {
method: 'wallet_switchEthereumChain',
params: [{ chainId: numberToHex(chainId) }],
}),
new Promise<void>((resolve) =>
config.emitter.once('change', ({ chainId: currentChainId }) => {
if (currentChainId === chainId) resolve()
}),
),
new Promise<void>((resolve) => {
const listener: EIP1193EventMap['chainChanged'] = (data) => {
console.log('[injected] switchChain.listener', { data, chainId })
if (Number(data) === chainId) {
provider.removeListener('chainChanged', listener)
resolve()
}
}
provider.on('chainChanged', listener)
}),
])
return chain
} catch (err) {
Expand Down Expand Up @@ -407,6 +450,7 @@ export function injected(parameters: InjectedParameters = {}) {
}
},
async onAccountsChanged(accounts) {
console.log('[injected] onAccountsChanged', accounts)
// Disconnect if there are no accounts
if (accounts.length === 0) this.onDisconnect()
// Connect if emitter is listening for connect event (e.g. is disconnected and connects through wallet interface)
Expand All @@ -424,6 +468,7 @@ export function injected(parameters: InjectedParameters = {}) {
})
},
onChainChanged(chain) {
console.log('[injected] onChainChanged', chain)
const chainId = Number(chain)
config.emitter.emit('change', { chainId })
},
Expand All @@ -434,12 +479,25 @@ export function injected(parameters: InjectedParameters = {}) {
const chainId = Number(connectInfo.chainId)
config.emitter.emit('connect', { accounts, chainId })

// Manage EIP-1193 event listeners
const provider = await this.getProvider()
if (provider) {
provider.removeListener('connect', this.onConnect.bind(this))
provider.on('accountsChanged', this.onAccountsChanged.bind(this))
provider.on('chainChanged', this.onChainChanged)
provider.on('disconnect', this.onDisconnect.bind(this))
if (connect) {
provider.removeListener('connect', connect)
connect = undefined
}
if (!accountsChanged) {
accountsChanged = this.onAccountsChanged.bind(this)
provider.on('accountsChanged', accountsChanged)
}
if (!chainChanged) {
chainChanged = this.onChainChanged.bind(this)
provider.on('chainChanged', chainChanged)
}
if (!disconnect) {
disconnect = this.onDisconnect.bind(this)
provider.on('disconnect', disconnect)
}
}
},
async onDisconnect(error) {
Expand All @@ -456,14 +514,20 @@ export function injected(parameters: InjectedParameters = {}) {
// actually disconnected and we don't need to simulate it.
config.emitter.emit('disconnect')

// Manage EIP-1193 event listeners
if (provider) {
provider.removeListener(
'accountsChanged',
this.onAccountsChanged.bind(this),
)
provider.removeListener('chainChanged', this.onChainChanged)
provider.removeListener('disconnect', this.onDisconnect.bind(this))
provider.on('connect', this.onConnect.bind(this))
if (chainChanged) {
provider.removeListener('chainChanged', chainChanged)
chainChanged = undefined
}
if (disconnect) {
provider.removeListener('disconnect', disconnect)
disconnect = undefined
}
if (!connect) {
connect = this.onConnect.bind(this)
provider.on('connect', connect)
}
}
},
}))
Expand Down
20 changes: 16 additions & 4 deletions packages/core/src/createConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export function createConfig<
function getInitialState() {
return {
chainId: chains.getState()[0].id,
connections: new Map(),
connections: new Map<string, Connection>(),
current: undefined,
status: 'disconnected',
} satisfies State
Expand Down Expand Up @@ -319,6 +319,14 @@ export function createConfig<
store.setState((x) => {
const connector = connectors.getState().find((x) => x.uid === data.uid)
if (!connector) return x

if (connector.emitter.listenerCount('connect'))
connector.emitter.off('connect', change)
if (!connector.emitter.listenerCount('change'))
connector.emitter.on('change', change)
if (!connector.emitter.listenerCount('disconnect'))
connector.emitter.on('disconnect', disconnect)

return {
...x,
connections: new Map(x.connections).set(data.uid, {
Expand All @@ -335,9 +343,13 @@ export function createConfig<
store.setState((x) => {
const connection = x.connections.get(data.uid)
if (connection) {
connection.connector.emitter.off('change', change)
connection.connector.emitter.off('disconnect', disconnect)
connection.connector.emitter.on('connect', connect)
const connector = connection.connector
if (connector.emitter.listenerCount('change'))
connection.connector.emitter.off('change', change)
if (connector.emitter.listenerCount('disconnect'))
connection.connector.emitter.off('disconnect', disconnect)
if (!connector.emitter.listenerCount('connect'))
connection.connector.emitter.on('connect', connect)
}

x.connections.delete(data.uid)
Expand Down
Loading

0 comments on commit 42ad380

Please sign in to comment.