-
Notifications
You must be signed in to change notification settings - Fork 408
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
fix: fallback to chainId
of wallet
#1587
Conversation
Branch preview✅ Deploy successful! https://fallback_wallet_chainid--webcore.review-web-core.5afe.dev |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
src/hooks/useChainId.ts
Outdated
? wallet.chainId | ||
: defaultChainId | ||
|
||
return urlChainId || session.lastChainId || fallbackChainId |
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.
I would define the wallet chainId as a separate thing and fall back in this order:
return urlChainId || walletChainId || session.lastChainId || fallbackChainId
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.
Is fallbackChainId
and walletChainId
the same thing?
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.
I've inverted the order and added tests to cover it.
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.
Is fallbackChainId and walletChainId the same thing?
In my previous commit, fallbackChainId
was either that of the wallet
or from defaultChainId
. I've changed it in accordance with the first suggestion. Wdyt?
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.
Thanks! 👍
Tested, it correctly sets the app network with the current wallet network if you're on a page w/o a Safe address or On pages with a Safe address or the chain query param, it works the same as before. |
What it solves
Resolves #1554
How this PR fixes it
useChainId
returns thechainId
of the connected wallet if nochainId
is present in the URL or cached from the last session.How to test it
chainId
" URL in an Incognito window (e.g./welcome
).chainId
" URL in an Incognito window (e.g./welcome
).