-
Notifications
You must be signed in to change notification settings - Fork 5k
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
make network-controller methods overwrite rather than merge provider state #18127
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. |
Builds ready [8e756f4]
Page Load Metrics (1725 ± 51 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
Codecov Report
@@ Coverage Diff @@
## develop #18127 +/- ##
========================================
Coverage 63.93% 63.93%
========================================
Files 908 908
Lines 35399 35399
Branches 8976 8976
========================================
Hits 22632 22632
Misses 12767 12767
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Builds ready [5c33af2]
Page Load Metrics (1534 ± 36 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
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!
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!
This changes several places in the
NetworkController
where we callupdateState
- which merges partial state - on anObservableStore
instance, toputState
- which wholly overwrites state in that store. This pattern of immutable state mutation is safer and avoids confusing state scenarios where, for instance, arollbackToPreviousProvider
call results in a provider state with some of the state of the provider we failed to connect alongside the the details of the provider we rolled back to.