Skip to content

chore: updates web3-react, adds key for changing connector order #4085

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
vm merged 15 commits into from
Jul 13, 2022
Merged

chore: updates web3-react, adds key for changing connector order #4085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d34d5cf
fix connectors changing
vm Jul 12, 2022
230860f
update package
vm Jul 12, 2022
386816c
add connection name
vm Jul 12, 2022
b81bf2d
rename file
vm Jul 12, 2022
5f8127c
de-dupe
vm Jul 12, 2022
d3dd1bf
cb wallet fix
vm Jul 13, 2022
b0eb028
fix
vm Jul 13, 2022
3bd224c
yarn change
vm Jul 13, 2022
659895c
Merge branch 'main' into w3r-connectors
vm Jul 13, 2022
3f80992
log the key
vm Jul 13, 2022
8ad7c95
re-order connections
vm Jul 13, 2022
5489d33
memoize the key
vm Jul 13, 2022
c350c02
some updates
vm Jul 13, 2022
6e81dba
rm console
vm Jul 13, 2022
a293818
prevent memory leak
NoahZinsmeister Jul 13, 2022
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
20 changes: 10 additions & 10 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,16 +134,16 @@
"@uniswap/v3-periphery": "^1.1.1",
"@uniswap/v3-sdk": "^3.9.0",
"@walletconnect/ethereum-provider": "1.7.1",
"@web3-react/coinbase-wallet": "^8.0.33-beta.0",
"@web3-react/core": "^8.0.33-beta.0",
"@web3-react/eip1193": "^8.0.25-beta.0",
"@web3-react/empty": "^8.0.19-beta.0",
"@web3-react/gnosis-safe": "^8.0.5-beta.0",
"@web3-react/metamask": "^8.0.26-beta.0",
"@web3-react/network": "^8.0.26-beta.0",
"@web3-react/types": "^8.0.19-beta.0",
"@web3-react/url": "^8.0.24-beta.0",
"@web3-react/walletconnect": "^8.0.34-beta.0",
"@web3-react/coinbase-wallet": "^8.0.34-beta.0",
"@web3-react/core": "^8.0.35-beta.0",
"@web3-react/eip1193": "^8.0.26-beta.0",
"@web3-react/empty": "^8.0.20-beta.0",
"@web3-react/gnosis-safe": "^8.0.6-beta.0",
"@web3-react/metamask": "^8.0.27-beta.0",
"@web3-react/network": "^8.0.27-beta.0",
"@web3-react/types": "^8.0.20-beta.0",
"@web3-react/url": "^8.0.25-beta.0",
"@web3-react/walletconnect": "^8.0.35-beta.0",
"ajv": "^6.12.3",
"array.prototype.flat": "^1.2.4",
"array.prototype.flatmap": "^1.2.4",
Expand Down
8 changes: 0 additions & 8 deletions src/components/AccountDetails/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Trans } from '@lingui/macro'
import { useWeb3React } from '@web3-react/core'
import CopyHelper from 'components/AccountDetails/Copy'
import { coinbaseWalletConnection } from 'connection'
import { getConnection, getConnectionName, getIsCoinbaseWallet, getIsMetaMask } from 'connection/utils'
import { useCallback, useContext } from 'react'
import { ExternalLink as LinkIcon } from 'react-feather'
Expand Down Expand Up @@ -247,13 +246,6 @@ export default function AccountDetails({
onClick={() => {
if (connector.deactivate) {
connector.deactivate()

// Coinbase Wallet SDK does not emit a disconnect event to the provider,
// which is what web3-react uses to reset state. As a workaround we manually
// reset state.
if (connector === coinbaseWalletConnection.connector) {
connector.resetState()
}
} else {
connector.resetState()
}
Expand Down
20 changes: 15 additions & 5 deletions src/components/Web3Provider/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
import { Web3ReactProvider } from '@web3-react/core'
import useConnectors from 'hooks/useConnectors'
import { Web3ReactHooks, Web3ReactProvider } from '@web3-react/core'
import { Connector } from '@web3-react/types'
import { Connection } from 'connection'
import { getConnectionName } from 'connection/utils'
import useEagerlyConnect from 'hooks/useEagerlyConnect'
import { ReactNode } from 'react'
import useOrderedConnections from 'hooks/useOrderedConnections'
import { ReactNode, useMemo } from 'react'

export default function Web3Provider({ children }: { children: ReactNode }) {
useEagerlyConnect()
const connectors = useConnectors()
const connections = useOrderedConnections()
const connectors: [Connector, Web3ReactHooks][] = connections.map(({ hooks, connector }) => [connector, hooks])

return <Web3ReactProvider connectors={connectors}>{children}</Web3ReactProvider>
const key = useMemo(() => connections.map(({ type }: Connection) => getConnectionName(type)).join('-'), [connections])

return (
<Web3ReactProvider connectors={connectors} key={key}>
Copy link
Copy Markdown
Contributor

@zzmp zzmp Jul 12, 2022

Choose a reason for hiding this comment

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

Why does this need a key?

Copy link
Copy Markdown
Contributor

@NoahZinsmeister NoahZinsmeister Jul 13, 2022

Choose a reason for hiding this comment

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

sorry i should have surfaced the change in web3-react that requires this more broadly - i introduced it here: Uniswap/web3-react@a792857

the root of the issue is was that in the widget codebase, we started dynamically adding a provider to the connectors prop, which in turn is passed to getPriorityConnector, and finally threaded through to getSelectedConnector, which mounts hooks for each array element. so, if the length of the array changes, we change the number of hooks rendered mid-flight, which breaks the rules of hooks and was causing serious issues in the widget (hooks were returning invalid data)

so, there 100% needs to be a key at least on connectors.length. what i'm less sure about is whether changing the ordering of connectors without changing the length would also cause issues - admittedly i haven't dug into this enough to be sure, but i figured it was better to be safe than sorry, so i started enforcing referential equality for connectors to be safe, hence the need for a key

{children}
</Web3ReactProvider>
)
}
17 changes: 8 additions & 9 deletions src/connection/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,6 @@ import {
walletConnectConnection,
} from 'connection'

const CONNECTIONS = [
coinbaseWalletConnection,
fortmaticConnection,
injectedConnection,
networkConnection,
walletConnectConnection,
gnosisSafeConnection,
]

export function getIsInjected(): boolean {
return Boolean(window.ethereum)
}
Expand All @@ -30,6 +21,14 @@ export function getIsCoinbaseWallet(): boolean {
return window.ethereum?.isCoinbaseWallet ?? false
}

const CONNECTIONS = [
gnosisSafeConnection,
injectedConnection,
coinbaseWalletConnection,
walletConnectConnection,
fortmaticConnection,
networkConnection,
]
export function getConnection(c: Connector | ConnectionType) {
if (c instanceof Connector) {
const connection = CONNECTIONS.find((connection) => connection.connector === c)
Expand Down
10 changes: 2 additions & 8 deletions src/hooks/useConnectors.ts → src/hooks/useOrderedConnections.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { Web3ReactHooks } from '@web3-react/core'
import { Connector } from '@web3-react/types'
import { ConnectionType } from 'connection'
import { getConnection } from 'connection/utils'
import { useMemo } from 'react'
Expand All @@ -8,7 +6,7 @@ import { useAppSelector } from 'state/hooks'

const SELECTABLE_WALLETS = [...BACKFILLABLE_WALLETS, ConnectionType.FORTMATIC]

export default function useConnectors() {
export default function useOrderedConnections() {
const selectedWallet = useAppSelector((state) => state.user.selectedWallet)
return useMemo(() => {
const orderedConnectionTypes: ConnectionType[] = []
Expand All @@ -25,10 +23,6 @@ export default function useConnectors() {
// Add network connection last as it should be the fallback.
orderedConnectionTypes.push(ConnectionType.NETWORK)

// Convert to web3-react's representation of connectors.
const web3Connectors: [Connector, Web3ReactHooks][] = orderedConnectionTypes
.map(getConnection)
.map(({ connector, hooks }) => [connector, hooks])
return web3Connectors
return orderedConnectionTypes.map(getConnection)
Comment thread
vm marked this conversation as resolved.
}, [selectedWallet])
}
11 changes: 9 additions & 2 deletions src/lib/hooks/useBlockNumber.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,30 @@ export function BlockNumberProvider({ children }: { children: ReactNode }) {

const windowVisible = useIsWindowVisible()
useEffect(() => {
let stale = false

if (provider && activeChainId && windowVisible) {
// If chainId hasn't changed, don't clear the block. This prevents re-fetching still valid data.
setChainBlock((chainBlock) => (chainBlock.chainId === activeChainId ? chainBlock : { chainId: activeChainId }))

provider
.getBlockNumber()
.then(onBlock)
.then((block) => {
if (!stale) onBlock(block)
})
.catch((error) => {
console.error(`Failed to get block number for chainId ${activeChainId}`, error)
})

provider.on('block', onBlock)

return () => {
stale = true
provider.removeListener('block', onBlock)
}
}
return undefined

return void 0
}, [activeChainId, provider, onBlock, setChainBlock, windowVisible])

const value = useMemo(
Expand Down
2 changes: 1 addition & 1 deletion src/state/connection/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ConnectionType } from 'connection'

export const BACKFILLABLE_WALLETS = [
Copy link
Copy Markdown
Contributor Author

@vm vm Jul 13, 2022

Choose a reason for hiding this comment

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

this doesn't really matter but i just re-ordered based on the order in the wallet modal

ConnectionType.INJECTED,
ConnectionType.COINBASE_WALLET,
ConnectionType.WALLET_CONNECT,
ConnectionType.INJECTED,
]
Loading