From 1e69a9031e598d06e5f33abb6d4af8fe75b711b3 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 30 Jul 2024 00:08:18 +0100 Subject: [PATCH 1/3] Optimize token list selectors --- .../app/confirm/info/row/address.tsx | 104 ++++++------ ui/components/app/name/name.tsx | 160 ++++++++++-------- ui/hooks/useDisplayName.ts | 4 +- .../decoding/address/address.component.js | 4 +- ui/selectors/selectors.js | 76 +++------ 5 files changed, 170 insertions(+), 178 deletions(-) diff --git a/ui/components/app/confirm/info/row/address.tsx b/ui/components/app/confirm/info/row/address.tsx index 693dc41ea899..8165205ce9a9 100644 --- a/ui/components/app/confirm/info/row/address.tsx +++ b/ui/components/app/confirm/info/row/address.tsx @@ -1,5 +1,5 @@ import { NameType } from '@metamask/name-controller'; -import React, { useState } from 'react'; +import React, { memo, useState } from 'react'; import { useSelector } from 'react-redux'; import { AlignItems, @@ -24,55 +24,57 @@ export type ConfirmInfoRowAddressProps = { isSnapUsingThis?: boolean; }; -export const ConfirmInfoRowAddress = ({ - address, - isSnapUsingThis, -}: ConfirmInfoRowAddressProps) => { - const isPetNamesEnabled = useSelector(getPetnamesEnabled); - const { displayName, hexAddress } = useFallbackDisplayName(address); - const [isNicknamePopoverShown, setIsNicknamePopoverShown] = useState(false); - const handleDisplayNameClick = () => setIsNicknamePopoverShown(true); - const onCloseHandler = () => setIsNicknamePopoverShown(false); +export const ConfirmInfoRowAddress = memo( + ({ address, isSnapUsingThis }: ConfirmInfoRowAddressProps) => { + const isPetNamesEnabled = useSelector(getPetnamesEnabled); + const { displayName, hexAddress } = useFallbackDisplayName(address); + const [isNicknamePopoverShown, setIsNicknamePopoverShown] = useState(false); + const handleDisplayNameClick = () => setIsNicknamePopoverShown(true); + const onCloseHandler = () => setIsNicknamePopoverShown(false); - return ( - - { - // PetNames on this component are disabled for snaps until the `` - // component can support variations. See this comment for context: // - // https://github.com/MetaMask/metamask-extension/pull/23487#discussion_r1525055546 - isPetNamesEnabled && !isSnapUsingThis ? ( - - ) : ( - <> - - - + { + // PetNames on this component are disabled for snaps until the `` + // component can support variations. See this comment for context: // + // https://github.com/MetaMask/metamask-extension/pull/23487#discussion_r1525055546 + isPetNamesEnabled && !isSnapUsingThis ? ( + + ) : ( + <> + - {displayName} - - - {isNicknamePopoverShown ? ( - - ) : null} - - ) - } - - ); -}; + + + {displayName} + + + {isNicknamePopoverShown ? ( + + ) : null} + + ) + } + + ); + }, +); diff --git a/ui/components/app/name/name.tsx b/ui/components/app/name/name.tsx index 3559181ae8ff..6b5433123e1e 100644 --- a/ui/components/app/name/name.tsx +++ b/ui/components/app/name/name.tsx @@ -1,4 +1,10 @@ -import React, { useCallback, useContext, useEffect, useState } from 'react'; +import React, { + memo, + useCallback, + useContext, + useEffect, + useState, +} from 'react'; import { NameType } from '@metamask/name-controller'; import classnames from 'classnames'; import { toChecksumAddress } from 'ethereumjs-util'; @@ -44,81 +50,85 @@ function formatValue(value: string, type: NameType): string { } } -export default function Name({ - value, - type, - disableEdit, - internal, - preferContractSymbol = false, -}: NameProps) { - const [modalOpen, setModalOpen] = useState(false); - const trackEvent = useContext(MetaMetricsContext); - - const { name, hasPetname } = useDisplayName( +export const Name = memo( + ({ value, type, - preferContractSymbol, - ); - - useEffect(() => { - if (internal) { - return; - } - - trackEvent({ - event: MetaMetricsEventName.PetnameDisplayed, - category: MetaMetricsEventCategory.Petnames, - properties: { - petname_category: type, - has_petname: Boolean(name?.length), - }, - }); - }, []); - - const handleClick = useCallback(() => { - setModalOpen(true); - }, [setModalOpen]); - - const handleModalClose = useCallback(() => { - setModalOpen(false); - }, [setModalOpen]); - - const formattedValue = formatValue(value, type); - const hasDisplayName = Boolean(name); - - return ( -
- {!disableEdit && modalOpen && ( - - )} -
- {hasDisplayName ? ( - - ) : ( - - )} - {hasDisplayName ? ( - - {name} - - ) : ( - - {formattedValue} - + disableEdit, + internal, + preferContractSymbol = false, + }: NameProps) => { + const [modalOpen, setModalOpen] = useState(false); + const trackEvent = useContext(MetaMetricsContext); + + const { name, hasPetname } = useDisplayName( + value, + type, + preferContractSymbol, + ); + + useEffect(() => { + if (internal) { + return; + } + + trackEvent({ + event: MetaMetricsEventName.PetnameDisplayed, + category: MetaMetricsEventCategory.Petnames, + properties: { + petname_category: type, + has_petname: Boolean(name?.length), + }, + }); + }, []); + + const handleClick = useCallback(() => { + setModalOpen(true); + }, [setModalOpen]); + + const handleModalClose = useCallback(() => { + setModalOpen(false); + }, [setModalOpen]); + + const formattedValue = formatValue(value, type); + const hasDisplayName = Boolean(name); + + return ( +
+ {!disableEdit && modalOpen && ( + )} +
+ {hasDisplayName ? ( + + ) : ( + + )} + {hasDisplayName ? ( + + {name} + + ) : ( + + {formattedValue} + + )} +
-
- ); -} + ); + }, +); + +export default Name; diff --git a/ui/hooks/useDisplayName.ts b/ui/hooks/useDisplayName.ts index 96e1f48c84e9..eee32dd4f72c 100644 --- a/ui/hooks/useDisplayName.ts +++ b/ui/hooks/useDisplayName.ts @@ -1,6 +1,6 @@ import { NameType } from '@metamask/name-controller'; import { useSelector } from 'react-redux'; -import { getMemoizedMetadataContracts } from '../selectors'; +import { getMemoizedMetadataContracts, getRemoteTokens } from '../selectors'; import { getNftContractsByAddressOnCurrentChain } from '../selectors/nft'; import { useNames } from './useName'; import { useFirstPartyContractNames } from './useFirstPartyContractName'; @@ -32,7 +32,7 @@ export function useDisplayNames( const contractInfo = useSelector((state) => // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - (getMemoizedMetadataContracts as any)(state, values, true), + (getRemoteTokens as any)(state, values), ); const watchedNftNames = useSelector(getNftContractsByAddressOnCurrentChain); diff --git a/ui/pages/confirmations/components/transaction-decoding/components/decoding/address/address.component.js b/ui/pages/confirmations/components/transaction-decoding/components/decoding/address/address.component.js index 8f97629ca07d..57f50e468757 100644 --- a/ui/pages/confirmations/components/transaction-decoding/components/decoding/address/address.component.js +++ b/ui/pages/confirmations/components/transaction-decoding/components/decoding/address/address.component.js @@ -6,8 +6,8 @@ import { shortenAddress } from '../../../../../../../helpers/utils/util'; import Identicon from '../../../../../../../components/ui/identicon'; import { useI18nContext } from '../../../../../../../hooks/useI18nContext'; import { - getMemoizedMetadataContractName, getMemoizedAddressBook, + getMetadataContractName, } from '../../../../../../../selectors'; import NicknamePopovers from '../../../../../../../components/app/modals/nickname-popovers'; import { COPY_OPTIONS } from '../../../../../../../../shared/constants/copy'; @@ -29,7 +29,7 @@ const Address = ({ ); const recipientNickname = addressBookEntryObject?.name; const recipientMetadataName = useSelector((state) => - getMemoizedMetadataContractName(state, checksummedRecipientAddress), + getMetadataContractName(state, checksummedRecipientAddress), ); const recipientToRender = addressOnly diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index c9cc9c47d95f..49dffe906bef 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -582,14 +582,6 @@ export function getAccountName(accounts, accountAddress) { return account && account.metadata.name !== '' ? account.metadata.name : ''; } -export function getMetadataContractName(state, address) { - const tokenList = getTokenList(state); - const entry = Object.values(tokenList).find((token) => - isEqualCaseInsensitive(token.address, address), - ); - return entry && entry.name !== '' ? entry.name : ''; -} - export function accountsWithSendEtherInfoSelector(state) { const accounts = getMetaMaskAccounts(state); const internalAccounts = getInternalAccounts(state); @@ -1363,38 +1355,45 @@ export const getMemoizedAddressBook = createDeepEqualSelector( (addressBook) => addressBook, ); +export const getRemoteTokenList = createDeepEqualSelector( + (state) => state.metamask.tokenList, + (remoteTokenList) => remoteTokenList, +); + +/** + * To retrieve the token list for use throughout the UI. Will return the remotely fetched list + * from the tokens controller if token detection is enabled, or the static list if not. + * + * @param {*} state + * @returns {object} + */ +export const getTokenList = createSelector( + getRemoteTokenList, + getIsTokenDetectionInactiveOnMainnet, + (remoteTokenList, isTokenDetectionInactiveOnMainnet) => + isTokenDetectionInactiveOnMainnet ? STATIC_MAINNET_TOKEN_LIST : remoteTokenList +); + /** * Returns a memoized selector that gets contract info. * * @param state - The Redux store state. * @param addresses - An array of contract addresses. - * @param forceRemoteTokenList - Whether to force the use of the remote token list. * @returns {Array} A map of contract info, keyed by address. */ -export const getMemoizedMetadataContracts = createDeepEqualSelector( - (state, _addresses, forceRemoteTokenList) => - getTokenList(state, forceRemoteTokenList), - (_tokenList, addresses) => addresses, - (tokenList, addresses) => { - return addresses.map((address) => - Object.values(tokenList).find((identity) => - isEqualCaseInsensitive(identity.address, address), - ), - ); - }, +export const getRemoteTokens = createSelector( + getRemoteTokenList, + (_state, addresses) => addresses, + (remoteTokenList, addresses) => addresses.map((address) => remoteTokenList[address.toLowerCase()]) ); -export const getMemoizedMetadataContract = createDeepEqualSelector( - getTokenList, - (_tokenList, address) => address, - (tokenList, address) => { - return Object.values(tokenList).find((identity) => - isEqualCaseInsensitive(identity.address, address), - ); - }, +export const getMemoizedMetadataContract = createSelector( + (state, _address) => getTokenList(state), + (_state, address) => address, + (tokenList, address) => tokenList[address.toLowerCase()] ); -export const getMemoizedMetadataContractName = createDeepEqualSelector( +export const getMetadataContractName = createSelector( getMemoizedMetadataContract, (entry) => entry?.name ?? '', ); @@ -2006,25 +2005,6 @@ export function getTheme(state) { return state.metamask.theme; } -/** - * To retrieve the token list for use throughout the UI. Will return the remotely fetched list - * from the tokens controller if token detection is enabled, or the static list if not. - * - * @param {*} state - * @param {boolean} forceRemote - Whether to force the use of the remote token list regardless of the user preference. Defaults to false. - * @returns {object} - */ -export function getTokenList(state, forceRemote = false) { - const isTokenDetectionInactiveOnMainnet = - getIsTokenDetectionInactiveOnMainnet(state); - - if (isTokenDetectionInactiveOnMainnet && !forceRemote) { - return STATIC_MAINNET_TOKEN_LIST; - } - - return state.metamask.tokenList; -} - export function doesAddressRequireLedgerHidConnection(state, address) { const addressIsLedger = isAddressLedger(state, address); const transportTypePreferenceIsWebHID = From 7bc4184c72bacc82f633f2acbf536205c733385b Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 30 Jul 2024 00:26:36 +0100 Subject: [PATCH 2/3] Fix unit tests --- ui/hooks/useDisplayName.test.ts | 16 +++++++--------- ui/selectors/selectors.js | 4 ++-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/ui/hooks/useDisplayName.test.ts b/ui/hooks/useDisplayName.test.ts index 771cb9773144..1e2e4ea19900 100644 --- a/ui/hooks/useDisplayName.test.ts +++ b/ui/hooks/useDisplayName.test.ts @@ -1,6 +1,6 @@ import { NameEntry, NameType } from '@metamask/name-controller'; import { NftContract } from '@metamask/assets-controllers'; -import { getMemoizedMetadataContracts } from '../selectors'; +import { getRemoteTokens } from '../selectors'; import { getNftContractsByAddressOnCurrentChain } from '../selectors/nft'; import { useDisplayName } from './useDisplayName'; import { useNames } from './useName'; @@ -21,7 +21,7 @@ jest.mock('./useFirstPartyContractName', () => ({ })); jest.mock('../selectors', () => ({ - getMemoizedMetadataContracts: jest.fn(), + getRemoteTokens: jest.fn(), getCurrentChainId: jest.fn(), })); @@ -55,9 +55,7 @@ const WATCHED_NFT_FOUND_RETURN_VALUE = { describe('useDisplayName', () => { const useNamesMock = jest.mocked(useNames); - const getMemoizedMetadataContractsMock = jest.mocked( - getMemoizedMetadataContracts, - ); + const getRemoteTokensMock = jest.mocked(getRemoteTokens); const useFirstPartyContractNamesMock = jest.mocked( useFirstPartyContractNames, ); @@ -72,7 +70,7 @@ describe('useDisplayName', () => { useFirstPartyContractNamesMock.mockReturnValue([ NO_FIRST_PARTY_CONTRACT_NAME_FOUND_RETURN_VALUE, ]); - getMemoizedMetadataContractsMock.mockReturnValue([ + getRemoteTokensMock.mockReturnValue([ { name: NO_CONTRACT_NAME_FOUND_RETURN_VALUE, }, @@ -94,7 +92,7 @@ describe('useDisplayName', () => { useFirstPartyContractNamesMock.mockReturnValue([ FIRST_PARTY_CONTRACT_NAME_MOCK, ]); - getMemoizedMetadataContractsMock.mockReturnValue([ + getRemoteTokensMock.mockReturnValue([ { name: CONTRACT_NAME_MOCK, }, @@ -114,7 +112,7 @@ describe('useDisplayName', () => { useFirstPartyContractNamesMock.mockReturnValue([ FIRST_PARTY_CONTRACT_NAME_MOCK, ]); - getMemoizedMetadataContractsMock.mockReturnValue({ + getRemoteTokensMock.mockReturnValue({ name: CONTRACT_NAME_MOCK, }); getNftContractsByAddressOnCurrentChainMock.mockReturnValue( @@ -128,7 +126,7 @@ describe('useDisplayName', () => { }); it('prioritizes a contract name over a watched NFT name', () => { - getMemoizedMetadataContractsMock.mockReturnValue([ + getRemoteTokensMock.mockReturnValue([ { name: CONTRACT_NAME_MOCK, }, diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 49dffe906bef..be88dd1dc4e2 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1384,13 +1384,13 @@ export const getTokenList = createSelector( export const getRemoteTokens = createSelector( getRemoteTokenList, (_state, addresses) => addresses, - (remoteTokenList, addresses) => addresses.map((address) => remoteTokenList[address.toLowerCase()]) + (remoteTokenList, addresses) => addresses.map((address) => remoteTokenList[address?.toLowerCase()]) ); export const getMemoizedMetadataContract = createSelector( (state, _address) => getTokenList(state), (_state, address) => address, - (tokenList, address) => tokenList[address.toLowerCase()] + (tokenList, address) => tokenList[address?.toLowerCase()] ); export const getMetadataContractName = createSelector( From ec238607d1acf5326e7cc002edd72cd88888011b Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 30 Jul 2024 01:26:23 +0100 Subject: [PATCH 3/3] Fix linting --- ui/components/app/name/name.tsx | 2 +- ui/hooks/useDisplayName.ts | 2 +- ui/selectors/selectors.js | 15 ++++++++++----- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/ui/components/app/name/name.tsx b/ui/components/app/name/name.tsx index 6b5433123e1e..a5accdc5306e 100644 --- a/ui/components/app/name/name.tsx +++ b/ui/components/app/name/name.tsx @@ -50,7 +50,7 @@ function formatValue(value: string, type: NameType): string { } } -export const Name = memo( +const Name = memo( ({ value, type, diff --git a/ui/hooks/useDisplayName.ts b/ui/hooks/useDisplayName.ts index eee32dd4f72c..13dc005dac00 100644 --- a/ui/hooks/useDisplayName.ts +++ b/ui/hooks/useDisplayName.ts @@ -1,6 +1,6 @@ import { NameType } from '@metamask/name-controller'; import { useSelector } from 'react-redux'; -import { getMemoizedMetadataContracts, getRemoteTokens } from '../selectors'; +import { getRemoteTokens } from '../selectors'; import { getNftContractsByAddressOnCurrentChain } from '../selectors/nft'; import { useNames } from './useName'; import { useFirstPartyContractNames } from './useFirstPartyContractName'; diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index be88dd1dc4e2..28ca05d585d8 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1364,14 +1364,15 @@ export const getRemoteTokenList = createDeepEqualSelector( * To retrieve the token list for use throughout the UI. Will return the remotely fetched list * from the tokens controller if token detection is enabled, or the static list if not. * - * @param {*} state - * @returns {object} + * @type {() => object} */ export const getTokenList = createSelector( getRemoteTokenList, getIsTokenDetectionInactiveOnMainnet, (remoteTokenList, isTokenDetectionInactiveOnMainnet) => - isTokenDetectionInactiveOnMainnet ? STATIC_MAINNET_TOKEN_LIST : remoteTokenList + isTokenDetectionInactiveOnMainnet + ? STATIC_MAINNET_TOKEN_LIST + : remoteTokenList, ); /** @@ -1384,15 +1385,19 @@ export const getTokenList = createSelector( export const getRemoteTokens = createSelector( getRemoteTokenList, (_state, addresses) => addresses, - (remoteTokenList, addresses) => addresses.map((address) => remoteTokenList[address?.toLowerCase()]) + (remoteTokenList, addresses) => + addresses.map((address) => remoteTokenList[address?.toLowerCase()]), ); export const getMemoizedMetadataContract = createSelector( (state, _address) => getTokenList(state), (_state, address) => address, - (tokenList, address) => tokenList[address?.toLowerCase()] + (tokenList, address) => tokenList[address?.toLowerCase()], ); +/** + * @type (state: any, address: string) => string + */ export const getMetadataContractName = createSelector( getMemoizedMetadataContract, (entry) => entry?.name ?? '',