From 95881df333ad49cfa31ddad860e4844f00071c8f Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 27 Jul 2023 13:28:42 -0230 Subject: [PATCH] refactor: Simplify `handleNetworkSwitch` helper (#6878) The `handleNetworkSwitch` helper function no longer requires the `frequentRpcList` state as a parameter. Instead it takes a reference to the `PreferencesController`, and gets the RPC list state from that. Typically it would be preferrable to reference controller state from Redux rather than accesing it directly from the Engine global. However, this function will be called from non-UI contexts, so it has to get controller state from global context regardless. This change was done to simplify PR #6872, which is part of https://github.com/MetaMask/mobile-planning/issues/798 --- app/components/Nav/App/index.js | 7 ++---- .../Views/SendFlow/AddressTo/AddressTo.tsx | 12 ++++++---- app/components/Views/SendFlow/SendTo/index.js | 17 +++++++------- app/core/DeeplinkManager.js | 12 +++++----- app/util/networks/handleNetworkSwitch.ts | 23 +++++++++++-------- app/util/networks/index.test.ts | 20 ++++++++-------- 6 files changed, 48 insertions(+), 43 deletions(-) diff --git a/app/components/Nav/App/index.js b/app/components/Nav/App/index.js index c0af52ae5b1..1ccc6779fe1 100644 --- a/app/components/Nav/App/index.js +++ b/app/components/Nav/App/index.js @@ -34,7 +34,7 @@ import Logger from '../../../util/Logger'; import { trackErrorAsAnalytics } from '../../../util/analyticsV2'; import { routingInstrumentation } from '../../../util/sentryUtils'; import Analytics from '../../../core/Analytics/Analytics'; -import { connect, useSelector, useDispatch } from 'react-redux'; +import { connect, useDispatch } from 'react-redux'; import { CURRENT_APP_VERSION, EXISTING_USER, @@ -86,7 +86,6 @@ import EditAccountName from '../../Views/EditAccountName/EditAccountName'; import WC2Manager, { isWC2Enabled, } from '../../../../app/core/WalletConnect/WalletConnectV2'; -import { selectFrequentRpcList } from '../../../selectors/preferencesController'; const clearStackNavigatorOptions = { headerShown: false, @@ -232,7 +231,6 @@ const App = ({ userLoggedIn }) => { dispatch(setCurrentBottomNavRoute(route)); } }; - const frequentRpcList = useSelector(selectFrequentRpcList); useEffect(() => { if (prevNavigator.current || !navigator) return; @@ -315,7 +313,6 @@ const App = ({ userLoggedIn }) => { navigator.dispatch?.(CommonActions.navigate(params)); }, }, - frequentRpcList, dispatch, }); if (!prevNavigator.current) { @@ -336,7 +333,7 @@ const App = ({ userLoggedIn }) => { } prevNavigator.current = navigator; } - }, [dispatch, handleDeeplink, frequentRpcList, navigator]); + }, [dispatch, handleDeeplink, navigator]); useEffect(() => { const initAnalytics = async () => { diff --git a/app/components/Views/SendFlow/AddressTo/AddressTo.tsx b/app/components/Views/SendFlow/AddressTo/AddressTo.tsx index 9d1f179562c..d1543b4729e 100644 --- a/app/components/Views/SendFlow/AddressTo/AddressTo.tsx +++ b/app/components/Views/SendFlow/AddressTo/AddressTo.tsx @@ -10,7 +10,6 @@ import { NetworkSwitchErrorType } from '../../../../constants/error'; import Routes from '../../../../constants/navigation/Routes'; import Engine from '../../../../core/Engine'; import { selectNetwork } from '../../../../selectors/networkController'; -import { selectFrequentRpcList } from '../../../../selectors/preferencesController'; import { handleNetworkSwitch } from '../../../../util/networks'; import { AddressTo } from '../../../UI/AddressInputs'; import { createQRScannerNavDetails } from '../../QRScanner'; @@ -34,16 +33,19 @@ const SendFlowAddressTo = ({ const network = useSelector(selectNetwork); - const frequentRpcList = useSelector(selectFrequentRpcList); - const showAlertAction = (config: any) => dispatch(showAlert(config)); const onHandleNetworkSwitch = (chain_id: string) => { try { - const { NetworkController, CurrencyRateController } = Engine.context; - const networkSwitch = handleNetworkSwitch(chain_id, frequentRpcList, { + const { + NetworkController, + CurrencyRateController, + PreferencesController, + } = Engine.context; + const networkSwitch = handleNetworkSwitch(chain_id, { networkController: NetworkController, currencyRateController: CurrencyRateController, + preferencesController: PreferencesController, }); if (!networkSwitch) return; diff --git a/app/components/Views/SendFlow/SendTo/index.js b/app/components/Views/SendFlow/SendTo/index.js index 8fb8189ae8f..43898fa93d2 100644 --- a/app/components/Views/SendFlow/SendTo/index.js +++ b/app/components/Views/SendFlow/SendTo/index.js @@ -66,7 +66,6 @@ import { selectTicker, } from '../../../../selectors/networkController'; import { - selectFrequentRpcList, selectIdentities, selectSelectedAddress, } from '../../../../selectors/preferencesController'; @@ -139,10 +138,6 @@ class SendFlow extends PureComponent { * Indicates whether the current transaction is a deep link transaction */ isPaymentRequest: PropTypes.bool, - /** - * Frequent RPC list from PreferencesController - */ - frequentRpcList: PropTypes.array, /** * Boolean that indicates if the network supports buy */ @@ -253,11 +248,16 @@ class SendFlow extends PureComponent { handleNetworkSwitch = (chainId) => { try { - const { NetworkController, CurrencyRateController } = Engine.context; - const { showAlert, frequentRpcList } = this.props; - const network = handleNetworkSwitch(chainId, frequentRpcList, { + const { + NetworkController, + CurrencyRateController, + PreferencesController, + } = Engine.context; + const { showAlert } = this.props; + const network = handleNetworkSwitch(chainId, { networkController: NetworkController, currencyRateController: CurrencyRateController, + preferencesController: PreferencesController, }); if (!network) return; @@ -642,7 +642,6 @@ const mapStateToProps = (state) => ({ network: selectNetwork(state), providerType: selectProviderType(state), isPaymentRequest: state.transaction.paymentRequest, - frequentRpcList: selectFrequentRpcList(state), isNativeTokenBuySupported: isNetworkBuyNativeTokenSupported( selectChainId(state), getRampNetworks(state), diff --git a/app/core/DeeplinkManager.js b/app/core/DeeplinkManager.js index f420067f435..1504b90f54a 100644 --- a/app/core/DeeplinkManager.js +++ b/app/core/DeeplinkManager.js @@ -29,10 +29,9 @@ import { isNetworkBuySupported } from '../components/UI/Ramp/utils'; import { Minimizer } from './NativeModules'; class DeeplinkManager { - constructor({ navigation, frequentRpcList, dispatch }) { + constructor({ navigation, dispatch }) { this.navigation = navigation; this.pendingDeeplink = null; - this.frequentRpcList = frequentRpcList; this.dispatch = dispatch; } @@ -48,10 +47,12 @@ class DeeplinkManager { * @param switchToChainId - Corresponding chain id for new network */ _handleNetworkSwitch = (switchToChainId) => { - const { NetworkController, CurrencyRateController } = Engine.context; - const network = handleNetworkSwitch(switchToChainId, this.frequentRpcList, { + const { NetworkController, CurrencyRateController, PreferencesController } = + Engine.context; + const network = handleNetworkSwitch(switchToChainId, { networkController: NetworkController, currencyRateController: CurrencyRateController, + preferencesController: PreferencesController, }); if (!network) return; @@ -432,13 +433,12 @@ class DeeplinkManager { let instance = null; const SharedDeeplinkManager = { - init: ({ navigation, frequentRpcList, dispatch }) => { + init: ({ navigation, dispatch }) => { if (instance) { return; } instance = new DeeplinkManager({ navigation, - frequentRpcList, dispatch, }); }, diff --git a/app/util/networks/handleNetworkSwitch.ts b/app/util/networks/handleNetworkSwitch.ts index c7d8ef7d25b..36d381836c2 100644 --- a/app/util/networks/handleNetworkSwitch.ts +++ b/app/util/networks/handleNetworkSwitch.ts @@ -1,14 +1,19 @@ import { getNetworkTypeById } from './index'; +import type { NetworkController } from '@metamask/network-controller'; +import type { PreferencesController } from '@metamask/preferences-controller'; +import type { CurrencyRateController } from '@metamask/assets-controllers'; const handleNetworkSwitch = ( switchToChainId: string, - frequentRpcList: { - rpcUrl: string; - chainId: string; - ticker: string; - nickname: string; - }[], - { networkController, currencyRateController }: any, + { + networkController, + currencyRateController, + preferencesController, + }: { + networkController: NetworkController; + currencyRateController: CurrencyRateController; + preferencesController: PreferencesController; + }, ): string | undefined => { // If not specified, use the current network if (!switchToChainId) { @@ -23,8 +28,8 @@ const handleNetworkSwitch = ( return; } - const rpc = frequentRpcList.find( - ({ chainId }: { chainId: string }) => chainId === switchToChainId, + const rpc = preferencesController.state.frequentRpcList.find( + ({ chainId }) => chainId === switchToChainId, ); if (rpc) { diff --git a/app/util/networks/index.test.ts b/app/util/networks/index.test.ts index bb1fc11ccd5..4684a768d73 100644 --- a/app/util/networks/index.test.ts +++ b/app/util/networks/index.test.ts @@ -35,6 +35,9 @@ jest.mock('./../../core/Engine', () => ({ }, }, }, + PreferencesController: { + state: {}, + }, }, })); @@ -201,18 +204,17 @@ describe('NetworkUtils::handleNetworkSwitch', () => { }, ]; - const { NetworkController, CurrencyRateController } = Engine.context as any; + const { NetworkController, CurrencyRateController, PreferencesController } = + Engine.context as any; it('should change networks to the provided one', () => { const network = mockRPCFrequentList[0]; - const newNetwork = handleNetworkSwitch( - network.chainId, - mockRPCFrequentList, - { - networkController: NetworkController, - currencyRateController: CurrencyRateController, - }, - ); + PreferencesController.state.frequentRpcList = mockRPCFrequentList; + const newNetwork = handleNetworkSwitch(network.chainId, { + networkController: NetworkController, + currencyRateController: CurrencyRateController, + preferencesController: PreferencesController, + }); expect(newNetwork).toBe(network.nickname); }); });