From cca62fa0560afc845725c8e4883e4092430a3fea Mon Sep 17 00:00:00 2001 From: rvelaz Date: Fri, 13 Mar 2026 23:40:42 +0100 Subject: [PATCH 1/8] =?UTF-8?q?refactor(selectors):=20extract=20keyring=20?= =?UTF-8?q?utils=20to=20shared=20to=20remove=20shared=E2=86=92ui=20cycle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add shared/lib/selectors/keyring-utils.ts with pure keyring helpers - Add shared/lib/selectors/metamask-keyring.ts for getCurrentKeyringFromState - Implement isHardwareWallet and getHardwareWalletType in shared/lib/selectors/account.ts - Remove shared/lib/selectors index imports from ui/selectors/selectors - Regenerate circular-deps.jsonc (cycle count unchanged: 9) Made-with: Cursor --- development/circular-deps.jsonc | 2 +- shared/lib/selectors/account.ts | 43 +++++++++++++++++++++--- shared/lib/selectors/index.ts | 6 ---- shared/lib/selectors/keyring-utils.ts | 31 +++++++++++++++++ shared/lib/selectors/metamask-keyring.ts | 32 ++++++++++++++++++ 5 files changed, 103 insertions(+), 11 deletions(-) create mode 100644 shared/lib/selectors/keyring-utils.ts create mode 100644 shared/lib/selectors/metamask-keyring.ts diff --git a/development/circular-deps.jsonc b/development/circular-deps.jsonc index e08d3fdd50d1..99ad255faba2 100644 --- a/development/circular-deps.jsonc +++ b/development/circular-deps.jsonc @@ -8,8 +8,8 @@ [ [ - "shared/lib/selectors/account.ts", "shared/lib/selectors/index.ts", + "shared/lib/selectors/smart-transactions.ts", "ui/ducks/metamask/metamask.js", "ui/selectors/selectors.js", "ui/store/actions.ts" diff --git a/shared/lib/selectors/account.ts b/shared/lib/selectors/account.ts index 4ff0dec0b261..4701594a3656 100644 --- a/shared/lib/selectors/account.ts +++ b/shared/lib/selectors/account.ts @@ -1,5 +1,40 @@ -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { isHardwareWallet } from '../../../ui/selectors/selectors'; +import { getCurrentKeyringFromState } from './metamask-keyring'; +import { + isHardwareWalletFromKeyring, + getHardwareWalletTypeFromKeyring, +} from './keyring-utils'; -export { isHardwareWallet }; +type MetamaskAccountsState = { + metamask?: { + internalAccounts?: { + selectedAccount?: string; + accounts?: Record; + }; + }; +}; + +/** + * Returns true if the current wallet is a hardware wallet. + * Implemented in shared to avoid circular dependency on ui/selectors. + * + * @param state - Redux state + * @returns true if hardware wallet + */ +export function isHardwareWallet(state: MetamaskAccountsState): boolean { + const keyring = getCurrentKeyringFromState(state); + return isHardwareWalletFromKeyring(keyring); +} + +/** + * Returns the hardware wallet type string (e.g. "Ledger Hardware"), or undefined. + * Implemented in shared to avoid circular dependency on ui/selectors. + * + * @param state - Redux state + * @returns type string or undefined + */ +export function getHardwareWalletType( + state: MetamaskAccountsState, +): string | undefined { + const keyring = getCurrentKeyringFromState(state); + return getHardwareWalletTypeFromKeyring(keyring); +} diff --git a/shared/lib/selectors/index.ts b/shared/lib/selectors/index.ts index 6e983385b5cf..efd2a919dfd0 100644 --- a/shared/lib/selectors/index.ts +++ b/shared/lib/selectors/index.ts @@ -1,8 +1,2 @@ -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { getHardwareWalletType } from '../../../ui/selectors/selectors'; - export * from './smart-transactions'; export * from './account'; - -export { getHardwareWalletType }; diff --git a/shared/lib/selectors/keyring-utils.ts b/shared/lib/selectors/keyring-utils.ts new file mode 100644 index 000000000000..525188f64e79 --- /dev/null +++ b/shared/lib/selectors/keyring-utils.ts @@ -0,0 +1,31 @@ +/** + * Pure keyring helpers. No UI or state dependencies. + * Used by shared selectors to avoid circular dependency on ui/selectors. + */ + +/** + * Keyring-like object with optional type string. + */ +type KeyringLike = { type?: string } | null | undefined; + +/** + * Returns true if the keyring is a hardware wallet (type includes 'Hardware'). + * + * @param keyring - Keyring object from state + * @returns true if hardware wallet + */ +export function isHardwareWalletFromKeyring(keyring: KeyringLike): boolean { + return Boolean(keyring?.type?.includes('Hardware')); +} + +/** + * Returns the hardware wallet type string, or undefined if not a hardware wallet. + * + * @param keyring - Keyring object from state + * @returns type string e.g. "Ledger Hardware" or undefined + */ +export function getHardwareWalletTypeFromKeyring( + keyring: KeyringLike, +): string | undefined { + return isHardwareWalletFromKeyring(keyring) ? keyring?.type : undefined; +} diff --git a/shared/lib/selectors/metamask-keyring.ts b/shared/lib/selectors/metamask-keyring.ts new file mode 100644 index 000000000000..26775a114c28 --- /dev/null +++ b/shared/lib/selectors/metamask-keyring.ts @@ -0,0 +1,32 @@ +/** + * Minimal accessor for current keyring from MetaMask state. + * No UI imports - only reads state shape to break shared→ui cycle. + */ + +type InternalAccountsState = { + metamask?: { + internalAccounts?: { + selectedAccount?: string; + accounts?: Record; + }; + }; +}; + +/** + * Returns the keyring for the currently selected internal account. + * Uses minimal state shape to avoid depending on ui/selectors. + * + * @param state - Redux state with metamask.internalAccounts + * @returns keyring object or null + */ +export function getCurrentKeyringFromState( + state: InternalAccountsState, +): { type?: string } | null { + const selectedAccount = state.metamask?.internalAccounts?.selectedAccount; + const accounts = state.metamask?.internalAccounts?.accounts; + if (!selectedAccount || !accounts) { + return null; + } + const account = accounts[selectedAccount]; + return account?.metadata?.keyring ?? null; +} From 690dcbbaeda195999a84c1c5e75e367e54af6186 Mon Sep 17 00:00:00 2001 From: rvelaz Date: Fri, 13 Mar 2026 23:43:44 +0100 Subject: [PATCH 2/8] fix(deps): break template-renderer/translation cycle via context - Add TemplateRendererContext so MetaMaskTranslation does not import MetaMaskTemplateRenderer - MetaMaskTemplateRenderer provides itself via context; translation consumes it - Remove ui/components/app/** from .madgerc (no longer matches any cycle) - Cycle count: 9 -> 8 Made-with: Cursor --- .madgerc | 1 - development/circular-deps.jsonc | 7 ---- .../metamask-template-renderer.js | 38 +++++++++---------- .../template-renderer-context.js | 8 ++++ .../metamask-translation.js | 12 ++++-- 5 files changed, 35 insertions(+), 31 deletions(-) create mode 100644 ui/components/app/metamask-template-renderer/template-renderer-context.js diff --git a/.madgerc b/.madgerc index 7f83b44a330a..2062dd3faf6a 100644 --- a/.madgerc +++ b/.madgerc @@ -26,7 +26,6 @@ "ui/ducks/**", "ui/selectors/**", "ui/store/**", - "ui/components/app/**", "shared/lib/selectors/**" ] } diff --git a/development/circular-deps.jsonc b/development/circular-deps.jsonc index 99ad255faba2..2ec6c89f1908 100644 --- a/development/circular-deps.jsonc +++ b/development/circular-deps.jsonc @@ -14,13 +14,6 @@ "ui/selectors/selectors.js", "ui/store/actions.ts" ], - [ - "ui/components/app/metamask-template-renderer/index.js", - "ui/components/app/metamask-template-renderer/metamask-template-renderer.js", - "ui/components/app/metamask-template-renderer/safe-component-list.js", - "ui/components/app/metamask-translation/index.js", - "ui/components/app/metamask-translation/metamask-translation.js" - ], [ "ui/ducks/metamask/metamask.js", "ui/selectors/confirm-transaction.js", diff --git a/ui/components/app/metamask-template-renderer/metamask-template-renderer.js b/ui/components/app/metamask-template-renderer/metamask-template-renderer.js index 4120c9959e1d..f058ded5a312 100644 --- a/ui/components/app/metamask-template-renderer/metamask-template-renderer.js +++ b/ui/components/app/metamask-template-renderer/metamask-template-renderer.js @@ -1,6 +1,7 @@ import React, { memo } from 'react'; import { isEqual } from 'lodash'; import { safeComponentList } from './safe-component-list'; +import { TemplateRendererContext } from './template-renderer-context'; import { ValidChildren } from './section-shape'; function getElement(section) { @@ -42,25 +43,16 @@ function getPropComponents(components) { } const MetaMaskTemplateRenderer = ({ sections }) => { - if (!sections) { - // If sections is null eject early by returning null - return null; - } else if (typeof sections === 'string') { - // React can render strings directly, so return the string - return sections; - } else if ( - sections && - typeof sections === 'object' && - !Array.isArray(sections) - ) { - // If dealing with a single entry, then render a single object without key - return renderElement(sections); - } - - // The last case is dealing with an array of objects - return ( - <> - {sections.reduce((allChildren, child) => { + const content = + !sections ? null : typeof sections === 'string' ? ( + sections + ) : sections && + typeof sections === 'object' && + !Array.isArray(sections) ? ( + renderElement(sections) + ) : ( + <> + {sections.reduce((allChildren, child) => { if (child === undefined || child?.hide === true) { return allChildren; } @@ -97,7 +89,13 @@ const MetaMaskTemplateRenderer = ({ sections }) => { } return allChildren; }, [])} - + + ); + + return ( + + {content} + ); }; diff --git a/ui/components/app/metamask-template-renderer/template-renderer-context.js b/ui/components/app/metamask-template-renderer/template-renderer-context.js new file mode 100644 index 000000000000..79bae483d85d --- /dev/null +++ b/ui/components/app/metamask-template-renderer/template-renderer-context.js @@ -0,0 +1,8 @@ +import React from 'react'; + +/** + * Context to inject the template renderer component into MetaMaskTranslation. + * Breaks the circular dependency: MetaMaskTranslation no longer imports + * MetaMaskTemplateRenderer directly; it gets the renderer from this context. + */ +export const TemplateRendererContext = React.createContext(null); diff --git a/ui/components/app/metamask-translation/metamask-translation.js b/ui/components/app/metamask-translation/metamask-translation.js index 1694ce684fc8..6d5c0630af4d 100644 --- a/ui/components/app/metamask-translation/metamask-translation.js +++ b/ui/components/app/metamask-translation/metamask-translation.js @@ -1,7 +1,7 @@ -import React from 'react'; +import React, { useContext } from 'react'; import PropTypes from 'prop-types'; import { useI18nContext } from '../../../hooks/useI18nContext'; -import MetaMaskTemplateRenderer from '../metamask-template-renderer'; +import { TemplateRendererContext } from '../metamask-template-renderer/template-renderer-context'; import { SectionShape } from '../metamask-template-renderer/section-shape'; /** @@ -26,6 +26,7 @@ import { SectionShape } from '../metamask-template-renderer/section-shape'; */ export default function MetaMaskTranslation({ translationKey, variables }) { const t = useI18nContext(); + const TemplateRenderer = useContext(TemplateRendererContext); return t( translationKey, @@ -59,8 +60,13 @@ export default function MetaMaskTranslation({ translationKey, variables }) { 'MetaMaskTranslation does not allow for component trees of non trivial depth', ); } + if (!TemplateRenderer) { + throw new Error( + 'MetaMaskTranslation must be used inside MetaMaskTemplateRenderer (TemplateRendererContext.Provider)', + ); + } return ( - From dcf09b6cb7ed0389fbf6ea1cad07ec3020107d20 Mon Sep 17 00:00:00 2001 From: rvelaz Date: Fri, 13 Mar 2026 23:46:01 +0100 Subject: [PATCH 3/8] fix(deps): break metamask.js and actions.ts cycle via metamask-state-basic - Add ui/selectors/metamask-state-basic.js with getAlertEnabledness, getUnconnectedAccountAlertEnabledness, getCompletedOnboarding - actions.ts imports these from metamask-state-basic instead of ducks/metamask - metamask.js re-exports from metamask-state-basic and removes duplicate definitions - Cycle count: 8 -> 7 Made-with: Cursor --- development/circular-deps.jsonc | 3 +-- ui/ducks/metamask/metamask.js | 15 +++++++-------- ui/selectors/metamask-state-basic.js | 14 ++++++++++++++ ui/store/actions.ts | 2 +- 4 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 ui/selectors/metamask-state-basic.js diff --git a/development/circular-deps.jsonc b/development/circular-deps.jsonc index 2ec6c89f1908..583dd15cb906 100644 --- a/development/circular-deps.jsonc +++ b/development/circular-deps.jsonc @@ -48,6 +48,5 @@ "ui/selectors/selectors.js", "ui/store/actions.ts" ], - ["ui/ducks/metamask/metamask.js", "ui/selectors/selectors.js"], - ["ui/ducks/metamask/metamask.js", "ui/store/actions.ts"] + ["ui/ducks/metamask/metamask.js", "ui/selectors/selectors.js"] ] diff --git a/ui/ducks/metamask/metamask.js b/ui/ducks/metamask/metamask.js index 1e9c530848dd..deae51a77cda 100644 --- a/ui/ducks/metamask/metamask.js +++ b/ui/ducks/metamask/metamask.js @@ -31,6 +31,13 @@ import * as actionConstants from '../../store/actionConstants'; import { updateTransactionGasFees } from '../../store/actions'; import { setCustomGasLimit, setCustomGasPrice } from '../gas/gas.duck'; import { FirstTimeFlowType } from '../../../shared/constants/onboarding'; +import { + getAlertEnabledness, + getUnconnectedAccountAlertEnabledness, + getCompletedOnboarding, +} from '../../selectors/metamask-state-basic'; + +export { getAlertEnabledness, getUnconnectedAccountAlertEnabledness, getCompletedOnboarding }; const initialState = { isInitialized: false, @@ -248,11 +255,6 @@ export function updateGasFees({ // Selectors -export const getAlertEnabledness = (state) => state.metamask.alertEnabledness; - -export const getUnconnectedAccountAlertEnabledness = (state) => - getAlertEnabledness(state)[AlertTypes.unconnectedAccount]; - export const getWeb3ShimUsageAlertEnabledness = (state) => getAlertEnabledness(state)[AlertTypes.web3ShimUsage]; @@ -526,9 +528,6 @@ export function getIsNetworkBusyByChainId(state, chainId) { return gasFeeEstimates?.networkCongestion >= NetworkCongestionThresholds.busy; } -export function getCompletedOnboarding(state) { - return state.metamask.completedOnboarding; -} export function getIsInitialized(state) { return state.metamask.isInitialized; } diff --git a/ui/selectors/metamask-state-basic.js b/ui/selectors/metamask-state-basic.js new file mode 100644 index 000000000000..1ecdc71ae542 --- /dev/null +++ b/ui/selectors/metamask-state-basic.js @@ -0,0 +1,14 @@ +/** + * Minimal metamask state selectors with no dependency on store/actions or ducks/metamask. + * Used to break the metamask.js ↔ actions.ts circular dependency. + */ +import { AlertTypes } from '../../shared/constants/alerts'; + +export const getAlertEnabledness = (state) => state.metamask.alertEnabledness; + +export const getUnconnectedAccountAlertEnabledness = (state) => + getAlertEnabledness(state)[AlertTypes.unconnectedAccount]; + +export function getCompletedOnboarding(state) { + return state.metamask.completedOnboarding; +} diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 60f0e03c3aa8..196610d9f0d4 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -127,7 +127,7 @@ import { switchedToUnconnectedAccount } from '../ducks/alerts/unconnected-accoun import { getUnconnectedAccountAlertEnabledness, getCompletedOnboarding, -} from '../ducks/metamask/metamask'; +} from '../selectors/metamask-state-basic'; import { toChecksumHexAddress } from '../../shared/lib/hexstring-utils'; import { HardwareDeviceNames, From c494342069ffd1d0c725e61055c9ef9a98338fd0 Mon Sep 17 00:00:00 2001 From: rvelaz Date: Sat, 14 Mar 2026 00:50:11 +0100 Subject: [PATCH 4/8] fix(deps): decouple smart-transactions from ui/selectors - Add getPreferences and accountSupportsSmartTx to shared/lib/selectors/metamask-keyring.ts - Add selectNetworkConfigurationByChainId and selectDefaultRpcEndpointByChainId to shared/networks.ts - smart-transactions.ts imports from shared instead of ui/selectors/selectors - Add ui/pages/confirmations/** to .madgerc for remaining cycle - Remove shared/lib/selectors/** from .madgerc (no longer in any cycle) - Cycle count: 7 -> 2 Made-with: Cursor --- .madgerc | 2 +- development/circular-deps.jsonc | 37 ++-------------------- shared/lib/selectors/metamask-keyring.ts | 21 +++++++++++- shared/lib/selectors/networks.ts | 24 ++++++++++++++ shared/lib/selectors/smart-transactions.ts | 9 +++--- 5 files changed, 53 insertions(+), 40 deletions(-) diff --git a/.madgerc b/.madgerc index 2062dd3faf6a..927710a144c7 100644 --- a/.madgerc +++ b/.madgerc @@ -26,6 +26,6 @@ "ui/ducks/**", "ui/selectors/**", "ui/store/**", - "shared/lib/selectors/**" + "ui/pages/confirmations/**" ] } diff --git a/development/circular-deps.jsonc b/development/circular-deps.jsonc index 583dd15cb906..9dfa8891e832 100644 --- a/development/circular-deps.jsonc +++ b/development/circular-deps.jsonc @@ -7,46 +7,15 @@ // - For more information contact the Extension Platform team. [ - [ - "shared/lib/selectors/index.ts", - "shared/lib/selectors/smart-transactions.ts", - "ui/ducks/metamask/metamask.js", - "ui/selectors/selectors.js", - "ui/store/actions.ts" - ], - [ - "ui/ducks/metamask/metamask.js", - "ui/selectors/confirm-transaction.js", - "ui/selectors/custom-gas.js", - "ui/selectors/index.js", - "ui/selectors/selectors.js", - "ui/store/actions.ts" - ], - [ - "ui/ducks/metamask/metamask.js", - "ui/selectors/confirm-transaction.js", - "ui/selectors/custom-gas.js", - "ui/selectors/index.js", - "ui/store/actions.ts" - ], [ "ui/ducks/metamask/metamask.js", + "ui/pages/confirmations/confirmation/ResultTemplate.ts", + "ui/pages/confirmations/confirmation/templates/error.js", + "ui/pages/confirmations/confirmation/templates/index.js", "ui/selectors/confirm-transaction.js", "ui/selectors/index.js", "ui/selectors/selectors.js", "ui/store/actions.ts" ], - [ - "ui/ducks/metamask/metamask.js", - "ui/selectors/confirm-transaction.js", - "ui/selectors/index.js", - "ui/store/actions.ts" - ], - [ - "ui/ducks/metamask/metamask.js", - "ui/selectors/index.js", - "ui/selectors/selectors.js", - "ui/store/actions.ts" - ], ["ui/ducks/metamask/metamask.js", "ui/selectors/selectors.js"] ] diff --git a/shared/lib/selectors/metamask-keyring.ts b/shared/lib/selectors/metamask-keyring.ts index 26775a114c28..70ad14345a30 100644 --- a/shared/lib/selectors/metamask-keyring.ts +++ b/shared/lib/selectors/metamask-keyring.ts @@ -1,5 +1,7 @@ +import { KeyringTypes } from '@metamask/keyring-controller'; + /** - * Minimal accessor for current keyring from MetaMask state. + * Minimal accessors for MetaMask state. * No UI imports - only reads state shape to break shared→ui cycle. */ @@ -9,6 +11,7 @@ type InternalAccountsState = { selectedAccount?: string; accounts?: Record; }; + preferences?: Record; }; }; @@ -30,3 +33,19 @@ export function getCurrentKeyringFromState( const account = accounts[selectedAccount]; return account?.metadata?.keyring ?? null; } + +/** + * Returns metamask preferences. Used by smart-transactions to avoid ui import. + */ +export function getPreferences(state: InternalAccountsState): Record { + return state.metamask?.preferences ?? {}; +} + +/** + * Returns true if the current account supports smart transactions (not a snap account). + * Used by smart-transactions to avoid ui import. + */ +export function accountSupportsSmartTx(state: InternalAccountsState): boolean { + const keyring = getCurrentKeyringFromState(state); + return keyring?.type !== KeyringTypes.snap; +} diff --git a/shared/lib/selectors/networks.ts b/shared/lib/selectors/networks.ts index 045d4402848e..18a832214782 100644 --- a/shared/lib/selectors/networks.ts +++ b/shared/lib/selectors/networks.ts @@ -68,6 +68,30 @@ export const getNetworkConfigurationsByChainId = ( state: NetworkConfigurationsByChainIdState, ) => state.metamask.networkConfigurationsByChainId; +/** + * Parameterized selector: returns network configuration for a given chain ID. + */ +export const selectNetworkConfigurationByChainId = createSelector( + getNetworkConfigurationsByChainId, + (_state: NetworkConfigurationsByChainIdState, chainId: string) => chainId, + (networkConfigurationsByChainId, chainId) => + networkConfigurationsByChainId?.[chainId], +); + +/** + * Parameterized selector: returns the default RPC endpoint for a given chain ID. + */ +export const selectDefaultRpcEndpointByChainId = createSelector( + selectNetworkConfigurationByChainId, + (networkConfiguration) => { + if (!networkConfiguration) { + return undefined; + } + const { defaultRpcEndpointIndex, rpcEndpoints } = networkConfiguration; + return rpcEndpoints[defaultRpcEndpointIndex]; + }, +); + export const selectDefaultNetworkClientIdsByChainId = createSelector( getNetworkConfigurationsByChainId, (networkConfigurationsByChainId) => { diff --git a/shared/lib/selectors/smart-transactions.ts b/shared/lib/selectors/smart-transactions.ts index fa0ad52b9b28..d3c34a5d1937 100644 --- a/shared/lib/selectors/smart-transactions.ts +++ b/shared/lib/selectors/smart-transactions.ts @@ -12,17 +12,18 @@ import { import { accountSupportsSmartTx, getPreferences, +} from './metamask-keyring'; +import { + getCurrentChainId, selectDefaultRpcEndpointByChainId, - // TODO: Remove restricted import - // eslint-disable-next-line import/no-restricted-paths -} from '../../../ui/selectors/selectors'; // TODO: Migrate shared selectors to this file. + type NetworkState, +} from './networks'; import { getRemoteFeatureFlags, type RemoteFeatureFlagsState, // eslint-disable-next-line import/no-restricted-paths } from '../../../ui/selectors/remote-feature-flags'; import { isProduction } from '../environment'; -import { getCurrentChainId, type NetworkState } from './networks'; import { createDeepEqualSelector } from './util'; export type SmartTransactionsMetaMaskState = { From 28bf05cb821c6bdfddb8f0cd8d973fd61cbaa546 Mon Sep 17 00:00:00 2001 From: rvelaz Date: Sat, 14 Mar 2026 00:52:19 +0100 Subject: [PATCH 5/8] fix(deps): break selectors to templates cycle via approval-types - Extract TEMPLATED_CONFIRMATION_APPROVAL_TYPES to templates/approval-types.js - approval-types only imports from constants no store or selectors - selectors and useConfirmationNavigation import from approval-types - Remove ui/pages/confirmations/** from .madgerc - Cycle count: 2 (simplified 8-file confirmations cycle) Made-with: Cursor --- .madgerc | 3 +- development/circular-deps.jsonc | 4 --- .../confirmation/templates/approval-types.js | 30 +++++++++++++++++++ .../confirmation/templates/index.js | 3 +- .../hooks/useConfirmationNavigation.test.ts | 2 +- .../hooks/useConfirmationNavigation.ts | 2 +- ui/selectors/selectors.js | 2 +- 7 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 ui/pages/confirmations/confirmation/templates/approval-types.js diff --git a/.madgerc b/.madgerc index 927710a144c7..8de504dbbd65 100644 --- a/.madgerc +++ b/.madgerc @@ -25,7 +25,6 @@ "allowedCircularGlob": [ "ui/ducks/**", "ui/selectors/**", - "ui/store/**", - "ui/pages/confirmations/**" + "ui/store/**" ] } diff --git a/development/circular-deps.jsonc b/development/circular-deps.jsonc index 9dfa8891e832..0ecaee0b5d23 100644 --- a/development/circular-deps.jsonc +++ b/development/circular-deps.jsonc @@ -9,12 +9,8 @@ [ [ "ui/ducks/metamask/metamask.js", - "ui/pages/confirmations/confirmation/ResultTemplate.ts", - "ui/pages/confirmations/confirmation/templates/error.js", - "ui/pages/confirmations/confirmation/templates/index.js", "ui/selectors/confirm-transaction.js", "ui/selectors/index.js", - "ui/selectors/selectors.js", "ui/store/actions.ts" ], ["ui/ducks/metamask/metamask.js", "ui/selectors/selectors.js"] diff --git a/ui/pages/confirmations/confirmation/templates/approval-types.js b/ui/pages/confirmations/confirmation/templates/approval-types.js new file mode 100644 index 000000000000..939c5781b171 --- /dev/null +++ b/ui/pages/confirmations/confirmation/templates/approval-types.js @@ -0,0 +1,30 @@ +/** + * List of approval types that use templated confirmations. + * Extracted to break circular dependency: selectors → templates → actions → selectors. + * Only imports from constants - no store/actions or selectors. + */ +import { ApprovalType } from '@metamask/controller-utils'; +import { + HYPERLIQUID_APPROVAL_TYPE, + ASTERDEX_APPROVAL_TYPE, + GMX_APPROVAL_TYPE, + SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES, + SMART_TRANSACTION_CONFIRMATION_TYPES, +} from '../../../../../shared/constants/app'; + +export const TEMPLATED_CONFIRMATION_APPROVAL_TYPES = [ + ApprovalType.SwitchEthereumChain, + ApprovalType.ResultSuccess, + ApprovalType.ResultError, + SMART_TRANSACTION_CONFIRMATION_TYPES.showSmartTransactionStatusPage, + ApprovalType.SnapDialogAlert, + ApprovalType.SnapDialogConfirmation, + ApprovalType.SnapDialogPrompt, + ApprovalType.SnapDialogDefault, + SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.confirmAccountCreation, + SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.confirmAccountRemoval, + SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.showSnapAccountRedirect, + HYPERLIQUID_APPROVAL_TYPE, + ASTERDEX_APPROVAL_TYPE, + GMX_APPROVAL_TYPE, +]; diff --git a/ui/pages/confirmations/confirmation/templates/index.js b/ui/pages/confirmations/confirmation/templates/index.js index 556996eb65a5..94a02e0fc6eb 100644 --- a/ui/pages/confirmations/confirmation/templates/index.js +++ b/ui/pages/confirmations/confirmation/templates/index.js @@ -49,8 +49,7 @@ const APPROVAL_TEMPLATES = { [GMX_APPROVAL_TYPE]: defiReferralConsent, }; -export const TEMPLATED_CONFIRMATION_APPROVAL_TYPES = - Object.keys(APPROVAL_TEMPLATES); +export { TEMPLATED_CONFIRMATION_APPROVAL_TYPES } from './approval-types'; const ALLOWED_TEMPLATE_KEYS = [ 'cancelText', diff --git a/ui/pages/confirmations/hooks/useConfirmationNavigation.test.ts b/ui/pages/confirmations/hooks/useConfirmationNavigation.test.ts index cb9d96b85698..94551a7a3bc6 100644 --- a/ui/pages/confirmations/hooks/useConfirmationNavigation.test.ts +++ b/ui/pages/confirmations/hooks/useConfirmationNavigation.test.ts @@ -33,7 +33,7 @@ jest.mock('react-router-dom', () => { }; }); -jest.mock('../confirmation/templates', () => ({ +jest.mock('../confirmation/templates/approval-types', () => ({ TEMPLATED_CONFIRMATION_APPROVAL_TYPES: ['wallet_addEthereumChain'], })); diff --git a/ui/pages/confirmations/hooks/useConfirmationNavigation.ts b/ui/pages/confirmations/hooks/useConfirmationNavigation.ts index c8d65dabe53a..2bc13b7bddf9 100644 --- a/ui/pages/confirmations/hooks/useConfirmationNavigation.ts +++ b/ui/pages/confirmations/hooks/useConfirmationNavigation.ts @@ -6,7 +6,7 @@ import { isEqual } from 'lodash'; import { ApprovalRequest } from '@metamask/approval-controller'; import { Json } from '@metamask/utils'; -import { TEMPLATED_CONFIRMATION_APPROVAL_TYPES } from '../confirmation/templates'; +import { TEMPLATED_CONFIRMATION_APPROVAL_TYPES } from '../confirmation/templates/approval-types'; import { CONFIRM_ADD_SUGGESTED_NFT_ROUTE, CONFIRM_ADD_SUGGESTED_TOKEN_ROUTE, diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 495c6124712c..667c71100536 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -127,7 +127,7 @@ import { sortSelectedInternalAccounts, } from '../helpers/utils/util'; -import { TEMPLATED_CONFIRMATION_APPROVAL_TYPES } from '../pages/confirmations/confirmation/templates'; +import { TEMPLATED_CONFIRMATION_APPROVAL_TYPES } from '../pages/confirmations/confirmation/templates/approval-types'; import { STATIC_MAINNET_TOKEN_LIST } from '../../shared/constants/tokens'; import { DAY } from '../../shared/constants/time'; import { TERMS_OF_USE_LAST_UPDATED } from '../../shared/constants/terms'; From 1ed66a5cd35027fb4aab0cd0b0d1bcc6b7afe273 Mon Sep 17 00:00:00 2001 From: rvelaz Date: Sat, 14 Mar 2026 01:32:26 +0100 Subject: [PATCH 6/8] fix(deps): break circular dependencies in selectors and update circular-deps.jsonc - Removed circular dependencies by introducing new selectors in send-ether-selectors.js and metamask-selectors-standalone.js. - Updated imports in various files to utilize the new selectors, ensuring a cleaner architecture. - Cleared the circular-deps.jsonc file as there are no longer any circular dependencies detected. Made-with: Cursor --- development/circular-deps.jsonc | 10 +- scripts/count-circular-deps.sh | 24 ++ ui/ducks/metamask/metamask.js | 12 +- ui/selectors/confirm-transaction.js | 2 +- ui/selectors/custom-gas.js | 6 +- ui/selectors/metamask-gas-selectors.js | 55 +++++ ui/selectors/metamask-selectors-standalone.js | 50 +++++ ui/selectors/metamask-state-basic.js | 7 + ui/selectors/selectors.js | 41 +--- ui/selectors/send-ether-selectors.js | 208 ++++++++++++++++++ 10 files changed, 359 insertions(+), 56 deletions(-) create mode 100755 scripts/count-circular-deps.sh create mode 100644 ui/selectors/metamask-gas-selectors.js create mode 100644 ui/selectors/metamask-selectors-standalone.js create mode 100644 ui/selectors/send-ether-selectors.js diff --git a/development/circular-deps.jsonc b/development/circular-deps.jsonc index 0ecaee0b5d23..978ae552f535 100644 --- a/development/circular-deps.jsonc +++ b/development/circular-deps.jsonc @@ -6,12 +6,4 @@ // - To prevent new circular dependencies, ensure your changes don't add new cycles // - For more information contact the Extension Platform team. -[ - [ - "ui/ducks/metamask/metamask.js", - "ui/selectors/confirm-transaction.js", - "ui/selectors/index.js", - "ui/store/actions.ts" - ], - ["ui/ducks/metamask/metamask.js", "ui/selectors/selectors.js"] -] +[] diff --git a/scripts/count-circular-deps.sh b/scripts/count-circular-deps.sh new file mode 100755 index 000000000000..60a9df6c8306 --- /dev/null +++ b/scripts/count-circular-deps.sh @@ -0,0 +1,24 @@ +#!/bin/bash +# Count circular dependency cycles from development/circular-deps.jsonc +# Usage: ./scripts/count-circular-deps.sh (run from mm-extension root) +# Run after: yarn circular-deps:update +# Outputs: single integer (cycle count) + +set -e +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +cd "$REPO_ROOT" + +FILE="development/circular-deps.jsonc" +if [[ ! -f "$FILE" ]]; then + echo "Error: $FILE not found. Run yarn circular-deps:update first." >&2 + exit 1 +fi + +node -e " +const fs = require('fs'); +const content = fs.readFileSync('$FILE', 'utf8'); +const json = content.split('\n').filter(l => !l.trim().startsWith('//')).join('\n'); +const arr = JSON.parse(json); +console.log(Array.isArray(arr) ? arr.length : 0); +" diff --git a/ui/ducks/metamask/metamask.js b/ui/ducks/metamask/metamask.js index deae51a77cda..58a049e00649 100644 --- a/ui/ducks/metamask/metamask.js +++ b/ui/ducks/metamask/metamask.js @@ -21,7 +21,7 @@ import { accountsWithSendEtherInfoSelector, checkNetworkAndAccountSupports1559, getAddressBook, -} from '../../selectors/selectors'; +} from '../../selectors/send-ether-selectors'; import { getProviderConfig, getSelectedNetworkClientId, @@ -363,16 +363,6 @@ export function isNotEIP1559Network(state) { * @param state * @param networkClientId - The optional network client ID to check for EIP-1559 support. Defaults to the currently selected network. */ -export function isEIP1559Network(state, networkClientId) { - const selectedNetworkClientId = getSelectedNetworkClientId(state); - - return ( - state.metamask.networksMetadata?.[ - networkClientId ?? selectedNetworkClientId - ]?.EIPS[1559] === true - ); -} - function getGasFeeControllerEstimateType(state) { return state.metamask.gasEstimateType; } diff --git a/ui/selectors/confirm-transaction.js b/ui/selectors/confirm-transaction.js index 857fef2cb36c..9d1153069898 100644 --- a/ui/selectors/confirm-transaction.js +++ b/ui/selectors/confirm-transaction.js @@ -14,7 +14,7 @@ import { getGasEstimateType, getGasFeeEstimates, getNativeCurrency, -} from '../ducks/metamask/metamask'; +} from './metamask-gas-selectors'; import { GasEstimateTypes, CUSTOM_GAS_ESTIMATE, diff --git a/ui/selectors/custom-gas.js b/ui/selectors/custom-gas.js index 2f0a9a6c6299..63fb8a8308b4 100644 --- a/ui/selectors/custom-gas.js +++ b/ui/selectors/custom-gas.js @@ -9,12 +9,12 @@ import { GasEstimateTypes as GAS_FEE_CONTROLLER_ESTIMATE_TYPES } from '../../sha import { getGasEstimateType, getGasFeeEstimates, - isEIP1559Network, -} from '../ducks/metamask/metamask'; +} from './metamask-gas-selectors'; +import { isEIP1559Network } from './send-ether-selectors'; import { calcGasTotal } from '../../shared/lib/transactions-controller-utils'; import { Numeric } from '../../shared/lib/Numeric'; import { EtherDenomination } from '../../shared/constants/common'; -import { getIsMainnet } from './selectors'; +import { getIsMainnet } from './metamask-state-basic'; export function getCustomGasLimit(state) { return state.gas.customData.limit; diff --git a/ui/selectors/metamask-gas-selectors.js b/ui/selectors/metamask-gas-selectors.js new file mode 100644 index 000000000000..5c87e82526e1 --- /dev/null +++ b/ui/selectors/metamask-gas-selectors.js @@ -0,0 +1,55 @@ +/** + * Gas-related selectors that do not depend on metamask duck or the rest of + * selectors. Used to break the metamask → actions → selectors → confirm-transaction → metamask cycle. + * + * Imports only from: shared, reselect, transaction-controller. + */ + +import { createSelector } from 'reselect'; +import { mergeGasFeeEstimates } from '@metamask/transaction-controller'; +import { getProviderConfig } from '../../shared/lib/selectors/networks'; + +function getGasFeeControllerEstimateType(state) { + return state.metamask.gasEstimateType; +} + +function getGasFeeControllerEstimates(state) { + return state.metamask.gasFeeEstimates; +} + +function getTransactionGasFeeEstimates(state) { + const transactionMetadata = state.confirmTransaction?.txData; + return transactionMetadata?.gasFeeEstimates; +} + +const getTransactionGasFeeEstimateType = createSelector( + getTransactionGasFeeEstimates, + (transactionGasFeeEstimates) => transactionGasFeeEstimates?.type, +); + +export const getGasEstimateType = createSelector( + getGasFeeControllerEstimateType, + getTransactionGasFeeEstimateType, + (gasFeeControllerEstimateType, transactionGasFeeEstimateType) => { + return transactionGasFeeEstimateType ?? gasFeeControllerEstimateType; + }, +); + +export const getGasFeeEstimates = createSelector( + getGasFeeControllerEstimates, + getTransactionGasFeeEstimates, + (gasFeeControllerEstimates, transactionGasFeeEstimates) => { + if (transactionGasFeeEstimates) { + return mergeGasFeeEstimates({ + gasFeeControllerEstimates, + transactionGasFeeEstimates, + }); + } + + return gasFeeControllerEstimates; + }, +); + +export function getNativeCurrency(state) { + return getProviderConfig(state).ticker; +} diff --git a/ui/selectors/metamask-selectors-standalone.js b/ui/selectors/metamask-selectors-standalone.js new file mode 100644 index 000000000000..47be35dc228e --- /dev/null +++ b/ui/selectors/metamask-selectors-standalone.js @@ -0,0 +1,50 @@ +/** + * Metamask selectors that do not depend on metamask duck or the rest of selectors. + * Used to break the metamask → actions → index → selectors → metamask cycle. + * + * Imports only from: shared, metamask-state-basic, send-ether-selectors. + */ + +import { KeyringType } from '../../shared/constants/keyring'; +import { addHexPrefix, stripHexPrefix } from '../../shared/lib/hexstring-utils'; +import { isEqualCaseInsensitive } from '../../shared/lib/string-utils'; +import { getProviderConfig } from '../../shared/lib/selectors/networks'; +import { getCurrencyRateControllerCurrencyRates } from '../../shared/lib/selectors/assets-migration'; +import { getCompletedOnboarding } from './metamask-state-basic'; +import { isNotEIP1559Network } from './send-ether-selectors'; + +export { getCompletedOnboarding, isNotEIP1559Network }; + +export function getConversionRate(state) { + return ( + getCurrencyRateControllerCurrencyRates(state)[ + getProviderConfig(state).ticker + ]?.conversionRate ?? undefined + ); +} + +export function getIsUnlocked(state) { + return state.metamask.isUnlocked; +} + +export function getLedgerTransportType(state) { + return state.metamask.ledgerTransportType; +} + +function findKeyringForAddress(state, address) { + const keyring = state.metamask.keyrings.find((kr) => { + return kr.accounts.some((account) => { + return ( + isEqualCaseInsensitive(account, addHexPrefix(address)) || + isEqualCaseInsensitive(account, stripHexPrefix(address)) + ); + }); + }); + + return keyring; +} + +export function isAddressLedger(state, address) { + const keyring = findKeyringForAddress(state, address); + return keyring?.type === KeyringType.ledger; +} diff --git a/ui/selectors/metamask-state-basic.js b/ui/selectors/metamask-state-basic.js index 1ecdc71ae542..27cdae8073a8 100644 --- a/ui/selectors/metamask-state-basic.js +++ b/ui/selectors/metamask-state-basic.js @@ -3,6 +3,8 @@ * Used to break the metamask.js ↔ actions.ts circular dependency. */ import { AlertTypes } from '../../shared/constants/alerts'; +import { CHAIN_IDS } from '../../shared/constants/network'; +import { getCurrentChainId } from '../../shared/lib/selectors/networks'; export const getAlertEnabledness = (state) => state.metamask.alertEnabledness; @@ -12,3 +14,8 @@ export const getUnconnectedAccountAlertEnabledness = (state) => export function getCompletedOnboarding(state) { return state.metamask.completedOnboarding; } + +export function getIsMainnet(state) { + const chainId = getCurrentChainId(state); + return chainId === CHAIN_IDS.MAINNET; +} diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 667c71100536..81fcbe3df850 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -138,12 +138,11 @@ import { import { getConversionRate, isNotEIP1559Network, - isEIP1559Network, getLedgerTransportType, isAddressLedger, getIsUnlocked, getCompletedOnboarding, -} from '../ducks/metamask/metamask'; +} from './metamask-selectors-standalone'; import { getLedgerWebHidConnectedStatus, getLedgerTransportStatus, @@ -184,6 +183,8 @@ import { getCurrentNetworkTransactions, } from './transactions'; import { EMPTY_ARRAY, EMPTY_OBJECT } from './shared'; +import { accountsWithSendEtherInfoSelector } from './send-ether-selectors'; +import { getIsMainnet } from './metamask-state-basic'; /** * @typedef {import('../../ui/store/store').MetaMaskReduxState} MetaMaskReduxState @@ -403,10 +404,9 @@ export function getCurrentKeyring(state) { * @param state * @param [networkClientId] - The optional network client ID to check network and account for EIP-1559 support */ -export function checkNetworkAndAccountSupports1559(state, networkClientId) { - const networkSupports1559 = isEIP1559Network(state, networkClientId); - return networkSupports1559; -} +export { + checkNetworkAndAccountSupports1559, +} from './send-ether-selectors'; /** * The function returns true if network and account details are fetched and @@ -1168,13 +1168,7 @@ export const getTokensMarketData = (state) => { export { getTokenRatesControllerMarketData as getMarketData }; -export function getAddressBook(state) { - const chainId = getCurrentChainId(state); - if (!state.metamask.addressBook[chainId]) { - return []; - } - return Object.values(state.metamask.addressBook[chainId]); -} +export { getAddressBook } from './send-ether-selectors'; export function getCompleteAddressBook(state) { const addresses = state.metamask.addressBook; @@ -1233,21 +1227,7 @@ export function getAccountName(accounts, accountAddress) { return account && account.metadata.name !== '' ? account.metadata.name : ''; } -export function accountsWithSendEtherInfoSelector(state) { - const accounts = getMetaMaskAccounts(state); - const internalAccounts = getInternalAccounts(state); - - const accountsWithSendEtherInfo = Object.values(internalAccounts).map( - (internalAccount) => { - return { - ...internalAccount, - ...accounts[internalAccount.address], - }; - }, - ); - - return accountsWithSendEtherInfo; -} +export { accountsWithSendEtherInfoSelector } from './send-ether-selectors'; export const getAccountsWithLabels = createSelector( getMetaMaskAccountsOrdered, @@ -1432,10 +1412,7 @@ export function getSuggestedNfts(state) { ); } -export function getIsMainnet(state) { - const chainId = getCurrentChainId(state); - return chainId === CHAIN_IDS.MAINNET; -} +export { getIsMainnet } from './metamask-state-basic'; export function getIsLineaMainnet(state) { const chainId = getCurrentChainId(state); diff --git a/ui/selectors/send-ether-selectors.js b/ui/selectors/send-ether-selectors.js new file mode 100644 index 000000000000..3828baac97e3 --- /dev/null +++ b/ui/selectors/send-ether-selectors.js @@ -0,0 +1,208 @@ +/** + * Send-ether-related selectors that do not depend on metamask duck or the rest + * of selectors. Used to break the metamask ↔ selectors circular dependency. + * + * Imports only from: shared, ui/selectors/accounts, ui/selectors/shared. + */ + +import { createSelector } from 'reselect'; +import { isEvmAccountType } from '@metamask/keyring-api'; +import { + getCurrentChainId, + getSelectedNetworkClientId, +} from '../../shared/lib/selectors/networks'; +import { + getAccountTrackerControllerAccountsByChainId, + getMultiChainBalancesControllerBalances, +} from '../../shared/lib/selectors/assets-migration'; +import { getEnabledNetworks } from '../../shared/lib/selectors/multichain'; +import { createParameterizedShallowEqualSelector } from '../../shared/lib/selectors/selector-creators'; +import { + MULTICHAIN_PROVIDER_CONFIGS, + MULTICHAIN_NETWORK_TO_ASSET_TYPES, +} from '../../shared/constants/multichain/networks'; +import { getInternalAccounts } from './accounts'; +import { EMPTY_OBJECT } from './shared'; + +// ----------------------------------------------------------------------------- +// getAddressBook +// ----------------------------------------------------------------------------- + +export function getAddressBook(state) { + const chainId = getCurrentChainId(state); + if (!state.metamask.addressBook[chainId]) { + return []; + } + return Object.values(state.metamask.addressBook[chainId]); +} + +// ----------------------------------------------------------------------------- +// isEIP1559Network & checkNetworkAndAccountSupports1559 +// ----------------------------------------------------------------------------- + +export function isEIP1559Network(state, networkClientId) { + const selectedNetworkClientId = getSelectedNetworkClientId(state); + + return ( + state.metamask.networksMetadata?.[ + networkClientId ?? selectedNetworkClientId + ]?.EIPS?.[1559] === true + ); +} + +export function checkNetworkAndAccountSupports1559(state, networkClientId) { + return isEIP1559Network(state, networkClientId); +} + +export function isNotEIP1559Network(state) { + const selectedNetworkClientId = getSelectedNetworkClientId(state); + return ( + state.metamask.networksMetadata?.[selectedNetworkClientId]?.EIPS?.[1559] === + false + ); +} + +// ----------------------------------------------------------------------------- +// getMetaMaskAccountBalances, getMetaMaskCachedBalances, getMetaMaskAccounts +// (internal helpers for accountsWithSendEtherInfoSelector) +// ----------------------------------------------------------------------------- + +const getMetaMaskAccountBalances = createSelector( + getAccountTrackerControllerAccountsByChainId, + getCurrentChainId, + (accountsByChainId, currentChainId) => { + const balancesForCurrentChain = accountsByChainId?.[currentChainId] ?? {}; + if (Object.keys(balancesForCurrentChain).length === 0) { + return EMPTY_OBJECT; + } + return Object.entries(balancesForCurrentChain).reduce( + (acc, [address, value]) => { + acc[address.toLowerCase()] = value; + return acc; + }, + {}, + ); + }, +); + +const getMetaMaskCachedBalances = createSelector( + getAccountTrackerControllerAccountsByChainId, + getEnabledNetworks, + getCurrentChainId, + (_, networkChainId) => networkChainId, + (accountsByChainId, enabledNetworks, currentChainId, networkChainId) => { + const eip155 = enabledNetworks?.eip155 ?? {}; + const enabledIds = Object.keys(eip155).filter((id) => Boolean(eip155[id])); + if (enabledIds.length === 1) { + const chainId = enabledIds[0]; + if (Object.keys(accountsByChainId?.[chainId] ?? {}).length === 0) { + return EMPTY_OBJECT; + } + return Object.entries(accountsByChainId[chainId]).reduce( + (accumulator, [key, value]) => { + accumulator[key.toLowerCase()] = value.balance; + return accumulator; + }, + {}, + ); + } + + const chainId = networkChainId ?? currentChainId; + if (Object.keys(accountsByChainId?.[chainId] ?? {}).length === 0) { + return EMPTY_OBJECT; + } + return Object.entries(accountsByChainId[chainId]).reduce( + (accumulator, [key, value]) => { + accumulator[key.toLowerCase()] = value.balance; + return accumulator; + }, + {}, + ); + }, +); + +const createChainIdSelector = createParameterizedShallowEqualSelector(10); + +const getMetaMaskAccounts = createChainIdSelector( + getInternalAccounts, + getMetaMaskAccountBalances, + getMetaMaskCachedBalances, + getMultiChainBalancesControllerBalances, + getCurrentChainId, + (_, chainId) => chainId, + ( + internalAccounts, + balances, + cachedBalances, + multichainBalances, + currentChainId, + chainId, + ) => { + return internalAccounts.reduce((accounts, internalAccount) => { + let account = internalAccount; + + if (chainId === undefined || currentChainId === chainId) { + if (isEvmAccountType(internalAccount.type)) { + if (balances?.[internalAccount.address]) { + account = { + ...account, + ...balances[internalAccount.address], + }; + } + } else { + const multichainNetwork = Object.values( + MULTICHAIN_PROVIDER_CONFIGS, + ).find((network) => + internalAccount.scopes.some((scope) => scope === network.chainId), + ); + account = { + ...account, + balance: + multichainBalances?.[internalAccount.id]?.[ + MULTICHAIN_NETWORK_TO_ASSET_TYPES[multichainNetwork.chainId] + ]?.amount ?? '0', + }; + } + + if (account.balance === null || account.balance === undefined) { + account = { + ...account, + balance: + (cachedBalances && cachedBalances[internalAccount.address]) ?? + '0x0', + }; + } + } else { + account = { + ...account, + balance: + (cachedBalances && cachedBalances[internalAccount.address]) ?? + '0x0', + }; + } + + accounts[internalAccount.address] = account; + return accounts; + }, {}); + }, +); + +// ----------------------------------------------------------------------------- +// accountsWithSendEtherInfoSelector +// ----------------------------------------------------------------------------- + +export function accountsWithSendEtherInfoSelector(state) { + const accounts = getMetaMaskAccounts(state); + const internalAccounts = getInternalAccounts(state); + + const accountsWithSendEtherInfo = Object.values(internalAccounts).map( + (internalAccount) => { + return { + ...internalAccount, + ...accounts[internalAccount.address], + }; + }, + ); + + return accountsWithSendEtherInfo; +} From c339bc5187c547d9ef4cf3ed4444306fa9339b15 Mon Sep 17 00:00:00 2001 From: rvelaz Date: Sat, 14 Mar 2026 01:36:05 +0100 Subject: [PATCH 7/8] refactor(madgerc): clear allowedCircularGlob to remove obsolete entries - Removed all entries from the allowedCircularGlob array in .madgerc, reflecting the recent architectural changes that have eliminated circular dependencies in the codebase. Made-with: Cursor --- .madgerc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.madgerc b/.madgerc index 8de504dbbd65..9fe4e9579f42 100644 --- a/.madgerc +++ b/.madgerc @@ -22,9 +22,5 @@ "skipAsyncImports": true } }, - "allowedCircularGlob": [ - "ui/ducks/**", - "ui/selectors/**", - "ui/store/**" - ] + "allowedCircularGlob": [] } From c5bd63329a86effc3e492dc4bbcb3ba0d25c409a Mon Sep 17 00:00:00 2001 From: rvelaz Date: Sat, 14 Mar 2026 01:48:33 +0100 Subject: [PATCH 8/8] refactor(selectors): convert extracted selectors to TypeScript - Convert template-renderer-context, approval-types, metamask-gas-selectors, metamask-selectors-standalone, metamask-state-basic, send-ether-selectors to .ts - Memoize accountsWithSendEtherInfoSelector with createSelector --- ...ontext.js => template-renderer-context.ts} | 7 +- .../{approval-types.js => approval-types.ts} | 2 +- ...selectors.js => metamask-gas-selectors.ts} | 19 ++- ...ne.js => metamask-selectors-standalone.ts} | 27 ++- ...state-basic.js => metamask-state-basic.ts} | 14 +- ...r-selectors.js => send-ether-selectors.ts} | 158 ++++++++++-------- 6 files changed, 134 insertions(+), 93 deletions(-) rename ui/components/app/metamask-template-renderer/{template-renderer-context.js => template-renderer-context.ts} (58%) rename ui/pages/confirmations/confirmation/templates/{approval-types.js => approval-types.ts} (93%) rename ui/selectors/{metamask-gas-selectors.js => metamask-gas-selectors.ts} (69%) rename ui/selectors/{metamask-selectors-standalone.js => metamask-selectors-standalone.ts} (63%) rename ui/selectors/{metamask-state-basic.js => metamask-state-basic.ts} (50%) rename ui/selectors/{send-ether-selectors.js => send-ether-selectors.ts} (54%) diff --git a/ui/components/app/metamask-template-renderer/template-renderer-context.js b/ui/components/app/metamask-template-renderer/template-renderer-context.ts similarity index 58% rename from ui/components/app/metamask-template-renderer/template-renderer-context.js rename to ui/components/app/metamask-template-renderer/template-renderer-context.ts index 79bae483d85d..bf1fa1c65be8 100644 --- a/ui/components/app/metamask-template-renderer/template-renderer-context.js +++ b/ui/components/app/metamask-template-renderer/template-renderer-context.ts @@ -5,4 +5,9 @@ import React from 'react'; * Breaks the circular dependency: MetaMaskTranslation no longer imports * MetaMaskTemplateRenderer directly; it gets the renderer from this context. */ -export const TemplateRendererContext = React.createContext(null); +export type TemplateRendererComponent = React.ComponentType<{ + sections: unknown; +}>; + +export const TemplateRendererContext = + React.createContext(null); diff --git a/ui/pages/confirmations/confirmation/templates/approval-types.js b/ui/pages/confirmations/confirmation/templates/approval-types.ts similarity index 93% rename from ui/pages/confirmations/confirmation/templates/approval-types.js rename to ui/pages/confirmations/confirmation/templates/approval-types.ts index 939c5781b171..ef93c1cba9f4 100644 --- a/ui/pages/confirmations/confirmation/templates/approval-types.js +++ b/ui/pages/confirmations/confirmation/templates/approval-types.ts @@ -12,7 +12,7 @@ import { SMART_TRANSACTION_CONFIRMATION_TYPES, } from '../../../../../shared/constants/app'; -export const TEMPLATED_CONFIRMATION_APPROVAL_TYPES = [ +export const TEMPLATED_CONFIRMATION_APPROVAL_TYPES: readonly string[] = [ ApprovalType.SwitchEthereumChain, ApprovalType.ResultSuccess, ApprovalType.ResultError, diff --git a/ui/selectors/metamask-gas-selectors.js b/ui/selectors/metamask-gas-selectors.ts similarity index 69% rename from ui/selectors/metamask-gas-selectors.js rename to ui/selectors/metamask-gas-selectors.ts index 5c87e82526e1..913b38356a77 100644 --- a/ui/selectors/metamask-gas-selectors.js +++ b/ui/selectors/metamask-gas-selectors.ts @@ -8,29 +8,36 @@ import { createSelector } from 'reselect'; import { mergeGasFeeEstimates } from '@metamask/transaction-controller'; import { getProviderConfig } from '../../shared/lib/selectors/networks'; +import type { MetaMaskReduxState } from '../store/store'; -function getGasFeeControllerEstimateType(state) { +function getGasFeeControllerEstimateType( + state: MetaMaskReduxState, +): string | undefined { return state.metamask.gasEstimateType; } -function getGasFeeControllerEstimates(state) { +function getGasFeeControllerEstimates(state: MetaMaskReduxState): unknown { return state.metamask.gasFeeEstimates; } -function getTransactionGasFeeEstimates(state) { +function getTransactionGasFeeEstimates(state: MetaMaskReduxState): unknown { const transactionMetadata = state.confirmTransaction?.txData; return transactionMetadata?.gasFeeEstimates; } const getTransactionGasFeeEstimateType = createSelector( getTransactionGasFeeEstimates, - (transactionGasFeeEstimates) => transactionGasFeeEstimates?.type, + (transactionGasFeeEstimates: unknown) => + (transactionGasFeeEstimates as { type?: string })?.type, ); export const getGasEstimateType = createSelector( getGasFeeControllerEstimateType, getTransactionGasFeeEstimateType, - (gasFeeControllerEstimateType, transactionGasFeeEstimateType) => { + ( + gasFeeControllerEstimateType: string | undefined, + transactionGasFeeEstimateType: string | undefined, + ) => { return transactionGasFeeEstimateType ?? gasFeeControllerEstimateType; }, ); @@ -50,6 +57,6 @@ export const getGasFeeEstimates = createSelector( }, ); -export function getNativeCurrency(state) { +export function getNativeCurrency(state: MetaMaskReduxState): string { return getProviderConfig(state).ticker; } diff --git a/ui/selectors/metamask-selectors-standalone.js b/ui/selectors/metamask-selectors-standalone.ts similarity index 63% rename from ui/selectors/metamask-selectors-standalone.js rename to ui/selectors/metamask-selectors-standalone.ts index 47be35dc228e..f001014cc97c 100644 --- a/ui/selectors/metamask-selectors-standalone.js +++ b/ui/selectors/metamask-selectors-standalone.ts @@ -5,17 +5,20 @@ * Imports only from: shared, metamask-state-basic, send-ether-selectors. */ +import { addHexPrefix, stripHexPrefix } from 'ethereumjs-util'; import { KeyringType } from '../../shared/constants/keyring'; -import { addHexPrefix, stripHexPrefix } from '../../shared/lib/hexstring-utils'; import { isEqualCaseInsensitive } from '../../shared/lib/string-utils'; import { getProviderConfig } from '../../shared/lib/selectors/networks'; import { getCurrencyRateControllerCurrencyRates } from '../../shared/lib/selectors/assets-migration'; +import type { MetaMaskReduxState } from '../store/store'; import { getCompletedOnboarding } from './metamask-state-basic'; import { isNotEIP1559Network } from './send-ether-selectors'; export { getCompletedOnboarding, isNotEIP1559Network }; -export function getConversionRate(state) { +export function getConversionRate( + state: MetaMaskReduxState, +): number | undefined { return ( getCurrencyRateControllerCurrencyRates(state)[ getProviderConfig(state).ticker @@ -23,17 +26,22 @@ export function getConversionRate(state) { ); } -export function getIsUnlocked(state) { +export function getIsUnlocked(state: MetaMaskReduxState): boolean { return state.metamask.isUnlocked; } -export function getLedgerTransportType(state) { +export function getLedgerTransportType( + state: MetaMaskReduxState, +): string | undefined { return state.metamask.ledgerTransportType; } -function findKeyringForAddress(state, address) { - const keyring = state.metamask.keyrings.find((kr) => { - return kr.accounts.some((account) => { +function findKeyringForAddress( + state: MetaMaskReduxState, + address: string, +): { type?: string; accounts: string[] } | undefined { + const keyring = state.metamask.keyrings.find((kr: { accounts: string[] }) => { + return kr.accounts.some((account: string) => { return ( isEqualCaseInsensitive(account, addHexPrefix(address)) || isEqualCaseInsensitive(account, stripHexPrefix(address)) @@ -44,7 +52,10 @@ function findKeyringForAddress(state, address) { return keyring; } -export function isAddressLedger(state, address) { +export function isAddressLedger( + state: MetaMaskReduxState, + address: string, +): boolean { const keyring = findKeyringForAddress(state, address); return keyring?.type === KeyringType.ledger; } diff --git a/ui/selectors/metamask-state-basic.js b/ui/selectors/metamask-state-basic.ts similarity index 50% rename from ui/selectors/metamask-state-basic.js rename to ui/selectors/metamask-state-basic.ts index 27cdae8073a8..f0c0b5320d48 100644 --- a/ui/selectors/metamask-state-basic.js +++ b/ui/selectors/metamask-state-basic.ts @@ -5,17 +5,21 @@ import { AlertTypes } from '../../shared/constants/alerts'; import { CHAIN_IDS } from '../../shared/constants/network'; import { getCurrentChainId } from '../../shared/lib/selectors/networks'; +import type { MetaMaskReduxState } from '../store/store'; -export const getAlertEnabledness = (state) => state.metamask.alertEnabledness; +export const getAlertEnabledness = ( + state: MetaMaskReduxState, +): Record => state.metamask.alertEnabledness; -export const getUnconnectedAccountAlertEnabledness = (state) => - getAlertEnabledness(state)[AlertTypes.unconnectedAccount]; +export const getUnconnectedAccountAlertEnabledness = ( + state: MetaMaskReduxState, +): boolean => getAlertEnabledness(state)[AlertTypes.unconnectedAccount]; -export function getCompletedOnboarding(state) { +export function getCompletedOnboarding(state: MetaMaskReduxState): boolean { return state.metamask.completedOnboarding; } -export function getIsMainnet(state) { +export function getIsMainnet(state: MetaMaskReduxState): boolean { const chainId = getCurrentChainId(state); return chainId === CHAIN_IDS.MAINNET; } diff --git a/ui/selectors/send-ether-selectors.js b/ui/selectors/send-ether-selectors.ts similarity index 54% rename from ui/selectors/send-ether-selectors.js rename to ui/selectors/send-ether-selectors.ts index 3828baac97e3..f6d0f721dfb9 100644 --- a/ui/selectors/send-ether-selectors.js +++ b/ui/selectors/send-ether-selectors.ts @@ -7,6 +7,7 @@ import { createSelector } from 'reselect'; import { isEvmAccountType } from '@metamask/keyring-api'; +import type { InternalAccount } from '@metamask/keyring-internal-api'; import { getCurrentChainId, getSelectedNetworkClientId, @@ -17,10 +18,9 @@ import { } from '../../shared/lib/selectors/assets-migration'; import { getEnabledNetworks } from '../../shared/lib/selectors/multichain'; import { createParameterizedShallowEqualSelector } from '../../shared/lib/selectors/selector-creators'; -import { - MULTICHAIN_PROVIDER_CONFIGS, - MULTICHAIN_NETWORK_TO_ASSET_TYPES, -} from '../../shared/constants/multichain/networks'; +import { MULTICHAIN_PROVIDER_CONFIGS } from '../../shared/constants/multichain/networks'; +import { MULTICHAIN_NETWORK_TO_ASSET_TYPES } from '../../shared/constants/multichain/assets'; +import type { MetaMaskReduxState } from '../store/store'; import { getInternalAccounts } from './accounts'; import { EMPTY_OBJECT } from './shared'; @@ -28,7 +28,7 @@ import { EMPTY_OBJECT } from './shared'; // getAddressBook // ----------------------------------------------------------------------------- -export function getAddressBook(state) { +export function getAddressBook(state: MetaMaskReduxState): unknown[] { const chainId = getCurrentChainId(state); if (!state.metamask.addressBook[chainId]) { return []; @@ -40,7 +40,10 @@ export function getAddressBook(state) { // isEIP1559Network & checkNetworkAndAccountSupports1559 // ----------------------------------------------------------------------------- -export function isEIP1559Network(state, networkClientId) { +export function isEIP1559Network( + state: MetaMaskReduxState, + networkClientId?: string, +): boolean { const selectedNetworkClientId = getSelectedNetworkClientId(state); return ( @@ -50,11 +53,14 @@ export function isEIP1559Network(state, networkClientId) { ); } -export function checkNetworkAndAccountSupports1559(state, networkClientId) { +export function checkNetworkAndAccountSupports1559( + state: MetaMaskReduxState, + networkClientId?: string, +): boolean { return isEIP1559Network(state, networkClientId); } -export function isNotEIP1559Network(state) { +export function isNotEIP1559Network(state: MetaMaskReduxState): boolean { const selectedNetworkClientId = getSelectedNetworkClientId(state); return ( state.metamask.networksMetadata?.[selectedNetworkClientId]?.EIPS?.[1559] === @@ -76,7 +82,7 @@ const getMetaMaskAccountBalances = createSelector( return EMPTY_OBJECT; } return Object.entries(balancesForCurrentChain).reduce( - (acc, [address, value]) => { + (acc: Record, [address, value]) => { acc[address.toLowerCase()] = value; return acc; }, @@ -89,7 +95,7 @@ const getMetaMaskCachedBalances = createSelector( getAccountTrackerControllerAccountsByChainId, getEnabledNetworks, getCurrentChainId, - (_, networkChainId) => networkChainId, + (_state: MetaMaskReduxState, networkChainId?: string) => networkChainId, (accountsByChainId, enabledNetworks, currentChainId, networkChainId) => { const eip155 = enabledNetworks?.eip155 ?? {}; const enabledIds = Object.keys(eip155).filter((id) => Boolean(eip155[id])); @@ -99,8 +105,8 @@ const getMetaMaskCachedBalances = createSelector( return EMPTY_OBJECT; } return Object.entries(accountsByChainId[chainId]).reduce( - (accumulator, [key, value]) => { - accumulator[key.toLowerCase()] = value.balance; + (accumulator: Record, [key, value]) => { + accumulator[key.toLowerCase()] = value.balance ?? '0'; return accumulator; }, {}, @@ -112,8 +118,8 @@ const getMetaMaskCachedBalances = createSelector( return EMPTY_OBJECT; } return Object.entries(accountsByChainId[chainId]).reduce( - (accumulator, [key, value]) => { - accumulator[key.toLowerCase()] = value.balance; + (accumulator: Record, [key, value]) => { + accumulator[key.toLowerCase()] = value.balance ?? '0'; return accumulator; }, {}, @@ -129,80 +135,88 @@ const getMetaMaskAccounts = createChainIdSelector( getMetaMaskCachedBalances, getMultiChainBalancesControllerBalances, getCurrentChainId, - (_, chainId) => chainId, + (_state: MetaMaskReduxState, chainId?: string) => chainId, ( - internalAccounts, - balances, - cachedBalances, - multichainBalances, - currentChainId, - chainId, + internalAccounts: InternalAccount[], + balances: Record, + cachedBalances: Record, + multichainBalances: + | Record> + | undefined, + currentChainId: string, + chainId: string | undefined, ) => { - return internalAccounts.reduce((accounts, internalAccount) => { - let account = internalAccount; + return internalAccounts.reduce( + ( + accounts: Record, + internalAccount, + ) => { + let account: InternalAccount & { balance?: string } = internalAccount; + + if (chainId === undefined || currentChainId === chainId) { + if (isEvmAccountType(internalAccount.type)) { + if (balances?.[internalAccount.address]) { + account = { + ...account, + ...(balances[internalAccount.address] as object), + }; + } + } else { + const multichainNetwork = Object.values( + MULTICHAIN_PROVIDER_CONFIGS, + ).find((network) => + internalAccount.scopes?.some( + (scope) => scope === network.chainId, + ), + ); + account = { + ...account, + balance: + multichainBalances?.[internalAccount.id]?.[ + multichainNetwork + ? MULTICHAIN_NETWORK_TO_ASSET_TYPES[ + multichainNetwork.chainId as keyof typeof MULTICHAIN_NETWORK_TO_ASSET_TYPES + ]?.[0] ?? '' + : '' + ]?.amount ?? '0', + }; + } - if (chainId === undefined || currentChainId === chainId) { - if (isEvmAccountType(internalAccount.type)) { - if (balances?.[internalAccount.address]) { + if (account.balance === null || account.balance === undefined) { account = { ...account, - ...balances[internalAccount.address], + balance: cachedBalances?.[internalAccount.address] ?? '0x0', }; } } else { - const multichainNetwork = Object.values( - MULTICHAIN_PROVIDER_CONFIGS, - ).find((network) => - internalAccount.scopes.some((scope) => scope === network.chainId), - ); account = { ...account, - balance: - multichainBalances?.[internalAccount.id]?.[ - MULTICHAIN_NETWORK_TO_ASSET_TYPES[multichainNetwork.chainId] - ]?.amount ?? '0', + balance: cachedBalances?.[internalAccount.address] ?? '0x0', }; } - if (account.balance === null || account.balance === undefined) { - account = { - ...account, - balance: - (cachedBalances && cachedBalances[internalAccount.address]) ?? - '0x0', - }; - } - } else { - account = { - ...account, - balance: - (cachedBalances && cachedBalances[internalAccount.address]) ?? - '0x0', - }; - } - - accounts[internalAccount.address] = account; - return accounts; - }, {}); + accounts[internalAccount.address] = account; + return accounts; + }, + {}, + ); }, ); // ----------------------------------------------------------------------------- -// accountsWithSendEtherInfoSelector +// accountsWithSendEtherInfoSelector (memoized) // ----------------------------------------------------------------------------- -export function accountsWithSendEtherInfoSelector(state) { - const accounts = getMetaMaskAccounts(state); - const internalAccounts = getInternalAccounts(state); - - const accountsWithSendEtherInfo = Object.values(internalAccounts).map( - (internalAccount) => { - return { - ...internalAccount, - ...accounts[internalAccount.address], - }; - }, - ); - - return accountsWithSendEtherInfo; -} +export const accountsWithSendEtherInfoSelector = createSelector( + getInternalAccounts, + (state: MetaMaskReduxState) => getMetaMaskAccounts(state), + ( + internalAccounts: InternalAccount[], + accounts: Record, + ) => { + return internalAccounts.map((internalAccount) => ({ + ...internalAccount, + ...accounts[internalAccount.address], + })); + }, +);