fix: downstream referential equality errors #144
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -7,7 +7,7 @@ import { Connector, Provider as Eip1193Provider, Web3ReactStore } from '@web3-re | |||
| import { WalletConnect } from '@web3-react/walletconnect' | |||
| import { SupportedChainId } from 'constants/chains' | |||
| import { atom, useAtom } from 'jotai' | |||
| import { PropsWithChildren, useEffect, useMemo } from 'react' | |||
| import { PropsWithChildren, useEffect, useMemo, useRef } from 'react' | |||
| import JsonRpcConnector from 'utils/JsonRpcConnector' | |||
|
|
|||
| export type Web3Connection = [Connector, Web3ReactHooks] | |||
There was a problem hiding this comment.
connections is potentially invalid if it changes over time. Does this export need to be reconsidered?
There was a problem hiding this comment.
so, while i'm still not thrilled about this export, i checked and all the other components that use connections are children of Web3ReactProvider, and since the whole point of this pr is ensuring that this component re-renders whenever connections changes, there shouldn't be any immediate issues
my vote would be to merge this and consider a refactor in a later pr? can make an issue if there's agreement
There was a problem hiding this comment.
If this is the case, could there just be a hook to expose connections within the React lifecycle? I'm fine with doing this as a follow up, but it seems brittle and not future-proof, so I may just do it today.
There was a problem hiding this comment.
tracking in #145
| walletConnectConnectionQR, | ||
| walletConnectConnectionPopup, | ||
| networkConnection, | ||
| ].filter((connection): connection is Web3Connection => Boolean(connection)) |
There was a problem hiding this comment.
Nit: I think you can add the type to the template parameters and just use Boolean, so that the actual code remains simpler.
There was a problem hiding this comment.
hm i tried this and i don't think it's possible - similar issue (without a fix) described here: microsoft/TypeScript#16069
| @@ -7,7 +7,7 @@ import { Connector, Provider as Eip1193Provider, Web3ReactStore } from '@web3-re | |||
| import { WalletConnect } from '@web3-react/walletconnect' | |||
| import { SupportedChainId } from 'constants/chains' | |||
| import { atom, useAtom } from 'jotai' | |||
| import { PropsWithChildren, useEffect, useMemo } from 'react' | |||
| import { PropsWithChildren, useEffect, useMemo, useRef } from 'react' | |||
| import JsonRpcConnector from 'utils/JsonRpcConnector' | |||
|
|
|||
| export type Web3Connection = [Connector, Web3ReactHooks] | |||
There was a problem hiding this comment.
If this is the case, could there just be a hook to expose connections within the React lifecycle? I'm fine with doing this as a follow up, but it seems brittle and not future-proof, so I may just do it today.
|
🎉 This PR is included in version 2.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.