-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: Network not updating when changing account connected the first time on a DAPP #9021
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
…directly from providerConfig
…ateUpdate of backgroundBridge
|
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.
Could you elaborate on how the network controller bump affected this logic?
Yes, the NetworkController changed the logic on how the state was being updated on version 12, and that seems to be impacting how the dapp receive data, what's happening is that the dapp is receiving data with the networkId and chainId on different times, because the networkId at certain point in time is null when the network changes (Because it's still being fetched) and because of that was not updating on the first change. On NetworkController V13 (Next Major version) the core repo will not expose the networkId in the network state, so where we need this |
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.
LGTM
|
Description
Updating the conditional case on
BackgroundBridge
onStateUpdate
function to trigger notifyChainChanged, when the chainId changes and when the network changes, it will allow for both properties being sent when they are called more than once and they are miss matching. That was happening on the first switch of the network and causing the bug.This fixes this issue: #9006
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
https://recordit.co/FEycCcnZ92
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist