-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: introduce ChainSyncManager to keep chain state up to date #15
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
refactor: introduce ChainSyncManager to keep chain state up to date #15
Conversation
| const SUPPORTED_CHAIN_IDS = SUPPORTED_CHAINS.map((chain) => chain.id); | ||
|
|
||
| export function UnsupportedChain() { | ||
| const { isConnected, chainId } = useUserStore(); |
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.
chainId from store may be different from account's chainId
| const newChainSlug = SUPPORTED_CHAINS.find( | ||
| (chain) => chain.id === Number(value) | ||
| )?.slug; | ||
| const pathParts = location.pathname.split('/').filter(Boolean); | ||
| const newPath = | ||
| pathParts.length > 1 | ||
| ? `/${newChainSlug}/${pathParts.slice(1).join('/')}` | ||
| : `/${newChainSlug}`; | ||
|
|
||
| navigate({ to: newPath }); |
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.
removed side effect in the component implementation, chain navigation is handled by a single component
| if (!chainId) { | ||
| throw Error('Missing chainId') | ||
| } |
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.
chainId initial state is undefined
| adapters: [wagmiAdapter], | ||
| networks: networks, | ||
| projectId, | ||
| defaultNetwork: bellecour, |
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.
default network was removed to avoid switching to the default chain before switching to the selected chain
scenario:
- user disconnected, arbitrum-sepolia is active
- user connects with an account on ethereum mainnet
- connection modal force to switch to bellecour 🐛
ErwanDecoster
left a comment
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.
Everything seems fine to me, I just have one remark:
Imagine I am on "http://localhost:5173/bellecour". If I modify the URL to "http://localhost:5173/arbitrum-sepolia", for a brief moment, the URL reverts to "http://localhost:5173/bellecour" before finally becoming "http://localhost:5173/arbitrum-sepolia" again.
I find this behavior a bit strange, but it doesn't really pose a problem.
This reflects the state of the account:
Depending on the wallet used, step 4.a. may not require user confirmation and create this "glitchy" view, but if the wallet delegates the confirmation, step 4.b. occurs and must be handled properly. In my opinion, it is not safe to skip the state synchronization by omitting step 2 because the outcome of step 4 is unknown. |
This PR introduces a new component
<ChainSyncManager>to keep the chain state up to date between the global store, URL, and user account provider.Tested scenarios (platforms: brave browser linux, chrome metamask linux):
/or/<unknown-chain>/*the default chain (bellecour) is selected/<known-chain>/*the known chain is selected