Remove providerConfig from NetworkController #4254
Conversation
mcmire
force-pushed
the
remove-provider-config
branch
2 times, most recently
from
9b3cc0a
to
730b264
Compare
mcmire
force-pushed
the
remove-provider-config
branch
2 times, most recently
from
ca0fc05
to
310a6b3
Compare
mcmire
force-pushed
the
remove-provider-config
branch
from
310a6b3
to
cd8a065
Compare
Historically, the `providerConfig` property in NetworkController has been used to track the currently selected network as well as provide access to information about that network. The selected network is now tracked via `selectedNetworkClientId`, and information about that network can be retrieved by looking at the `networkConfigurations` property or the `configuration` property on the NetworkClient interface. This means that we no longer need `providerConfig` and we can remove this redundant state.
mcmire
force-pushed
the
remove-provider-config
branch
from
cd8a065
to
65fa9a8
Compare
|
Conflicts resolved. |
packages/selected-network-controller/tests/SelectedNetworkController.test.ts
Show resolved
Hide resolved
|
Okay! Tests should pass again. |
| autoManagedNetworkClient = | ||
| builtInNetworkClientRegistry[networkClientId as BuiltInNetworkClientId]; | ||
| if (!autoManagedNetworkClient) { | ||
| // This is impossible to reach |
There was a problem hiding this comment.
How about mocking isInfuraNetworkType to return true?
There was a problem hiding this comment.
The test we'd need to write here is not what happens when isInfuraNetworkType returns true or false, but what happens when isInfuraNetworkType returns true and yet possibleAutoManagedNetworkClient is undefined. If that happens, it would mean that there is no network client registered for the networkClientId when it refers to an Infura network.
The reason why this is impossible is that autoManagedNetworkClientRegistry[NetworkClientType.Infura] is automatically populated with all of the Infura networks that we know about, and there is no way to stop that from happening. So if networkClientId refers to an Infura network, we know that it matches a network client in autoManagedNetworkClientRegistry[NetworkClientType.Infura].
However, this will no longer be true in #4286, because that integrates Infura networks into the networkConfigurationsByChainId state property, and so we will now be able to control whether a network client gets created for an Infura network or not, making this scenario testable.
All this to say... this istanbul ignore will go away in #4286.
| import { | ||
| buildCustomNetworkClientConfiguration, | ||
| buildInfuraNetworkClientConfiguration, | ||
| } from './helpers'; | ||
|
|
||
| jest.mock('../src/create-network-client'); | ||
|
|
There was a problem hiding this comment.
Curious to know, why we have a separate test folder which is uncommon when we compared with other controllers?
There was a problem hiding this comment.
Good question. This change was made by another team mistakenly and wasn't caught, and has now persisted. I agree that we should move this to src/ however.
| const fakeNetworkClient = buildFakeClient(fakeProvider); | ||
| createNetworkClientMock.mockReturnValue(fakeNetworkClient); | ||
| }, | ||
| infuraProjectId: 'some-infura-project-id', |
There was a problem hiding this comment.
Just to understand, even the custom network client will have a infura project id?
Explanation
Historically, the
providerConfigproperty in NetworkController has been used to track the currently selected network as well as provide access to information about that network. The selected network is now tracked viaselectedNetworkClientId, and information about that network can be retrieved by looking at thenetworkConfigurationsproperty or theconfigurationproperty on the NetworkClient interface. This means that we no longer needproviderConfigand we can remove this redundant state.References
Fixes #4185.
Changelog
(Updated in PR)
Checklist