Skip to content

Commit 398221b

Browse files
darkwingdarkwing
authored
fix: Prevent rerenders of the AccountListItem (#30873)
## **Description** The AccountListMenu has been been flagged as slow in the past, so I've memoized some selectors and child elements to make the menu render much, much less. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30873?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Check out `main` branch 2. Start the extension with the `ENABLE_WHY_DID_YOU_RENDER` flag on 3. Open the Account List Menu 4. See *loads* of rerendering of the `AccountListItem` due to 4 props changing (onClick, closeMenu, etc) 5. Checkout this branch PR 6. Start the extension with the `ENABLE_WHY_DID_YOU_RENDER` flag on 7. Open the Account List Menu 8. See 0 re-renders of the `AccountListItem` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
1 parent b32986d commit 398221b

File tree

8 files changed

+151
-119
lines changed

8 files changed

+151
-119
lines changed

‎ui/components/multichain/account-list-item/account-list-item.js

+9-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useContext, useEffect, useRef, useState } from 'react';
1+
import React, { useContext, useEffect, useMemo, useRef, useState } from 'react';
22
import PropTypes from 'prop-types';
33
import classnames from 'classnames';
44
///: BEGIN:ONLY_INCLUDE_IF(build-main)
@@ -163,10 +163,12 @@ const AccountListItem = ({
163163
formattedTokensWithBalancesPerChain,
164164
);
165165
// cross chain agg balance
166-
const mappedOrderedTokenList = accountTotalFiatBalances.orderedTokenList.map(
167-
(item) => ({
168-
avatarValue: item.iconUrl,
169-
}),
166+
const mappedOrderedTokenList = useMemo(
167+
() =>
168+
accountTotalFiatBalances.orderedTokenList.map((item) => ({
169+
avatarValue: item.iconUrl,
170+
})),
171+
[accountTotalFiatBalances.orderedTokenList],
170172
);
171173
let balanceToTranslate;
172174
if (isEvmNetwork) {
@@ -259,7 +261,7 @@ const AccountListItem = ({
259261
// Without this check, the account will be selected after
260262
// the account options menu closes
261263
if (!accountOptionsMenuOpen) {
262-
onClick?.();
264+
onClick?.(account);
263265
}
264266
}}
265267
>
@@ -338,7 +340,7 @@ const AccountListItem = ({
338340
as="button"
339341
onClick={(e) => {
340342
e.stopPropagation();
341-
onClick?.();
343+
onClick?.(account);
342344
}}
343345
variant={TextVariant.bodyMdMedium}
344346
className="multichain-account-list-item__account-name__button"

‎ui/components/multichain/account-list-menu/account-list-menu.tsx

+91-77
Original file line numberDiff line numberDiff line change
@@ -333,19 +333,23 @@ export const AccountListMenu = ({
333333
);
334334
///: END:ONLY_INCLUDE_IF
335335

336-
let searchResults: MergedInternalAccount[] = filteredUpdatedAccountList;
337-
if (searchQuery) {
338-
const fuse = new Fuse(filteredAccounts, {
339-
threshold: 0.2,
340-
location: 0,
341-
distance: 100,
342-
maxPatternLength: 32,
343-
minMatchCharLength: 1,
344-
keys: ['metadata.name', 'address'],
345-
});
346-
fuse.setCollection(filteredAccounts);
347-
searchResults = fuse.search(searchQuery);
348-
}
336+
const searchResults: MergedInternalAccount[] = useMemo(() => {
337+
let _searchResults: MergedInternalAccount[] = filteredUpdatedAccountList;
338+
if (searchQuery) {
339+
const fuse = new Fuse(filteredAccounts, {
340+
threshold: 0.2,
341+
location: 0,
342+
distance: 100,
343+
maxPatternLength: 32,
344+
minMatchCharLength: 1,
345+
keys: ['metadata.name', 'address'],
346+
});
347+
fuse.setCollection(filteredAccounts);
348+
_searchResults = fuse.search(searchQuery);
349+
}
350+
351+
return _searchResults;
352+
}, [filteredAccounts, filteredUpdatedAccountList, searchQuery]);
349353

350354
const title = useMemo(
351355
() => getActionTitle(t as (text: string) => string, actionMode),
@@ -367,32 +371,80 @@ export const AccountListMenu = ({
367371
}
368372

369373
const onAccountListItemItemClicked = useCallback(
370-
(account) => {
371-
return () => {
372-
onClose();
373-
trackEvent({
374-
category: MetaMetricsEventCategory.Navigation,
375-
event: MetaMetricsEventName.NavAccountSwitched,
376-
properties: {
377-
location: 'Main Menu',
378-
},
379-
});
380-
endTrace({
381-
name: ACCOUNT_OVERVIEW_TAB_KEY_TO_TRACE_NAME_MAP[
382-
defaultHomeActiveTabName
383-
],
384-
});
385-
trace({
386-
name: ACCOUNT_OVERVIEW_TAB_KEY_TO_TRACE_NAME_MAP[
387-
defaultHomeActiveTabName
388-
],
389-
});
390-
dispatch(setSelectedAccount(account.address));
391-
};
374+
(account: MergedInternalAccount) => {
375+
onClose();
376+
trackEvent({
377+
category: MetaMetricsEventCategory.Navigation,
378+
event: MetaMetricsEventName.NavAccountSwitched,
379+
properties: {
380+
location: 'Main Menu',
381+
},
382+
});
383+
endTrace({
384+
name: ACCOUNT_OVERVIEW_TAB_KEY_TO_TRACE_NAME_MAP[
385+
defaultHomeActiveTabName
386+
],
387+
});
388+
trace({
389+
name: ACCOUNT_OVERVIEW_TAB_KEY_TO_TRACE_NAME_MAP[
390+
defaultHomeActiveTabName
391+
],
392+
});
393+
dispatch(setSelectedAccount(account.address));
392394
},
393-
[dispatch, onClose, trackEvent],
395+
[dispatch, onClose, trackEvent, defaultHomeActiveTabName],
394396
);
395397

398+
const accountListItems = useMemo(() => {
399+
return searchResults.map((account) => {
400+
const connectedSite = connectedSites[account.address]?.find(
401+
({ origin }) => origin === currentTabOrigin,
402+
);
403+
404+
const hideAccountListItem = searchQuery.length === 0 && account.hidden;
405+
406+
/* NOTE: Hidden account will be displayed only in the search list */
407+
408+
return (
409+
<Box
410+
className={
411+
account.hidden
412+
? 'multichain-account-menu-popover__list--menu-item-hidden'
413+
: 'multichain-account-menu-popover__list--menu-item'
414+
}
415+
display={hideAccountListItem ? Display.None : Display.Block}
416+
key={account.address}
417+
>
418+
<AccountListItem
419+
onClick={onAccountListItemItemClicked}
420+
account={account}
421+
key={account.address}
422+
selected={selectedAccount.address === account.address}
423+
closeMenu={onClose}
424+
connectedAvatar={connectedSite?.iconUrl}
425+
menuType={AccountListItemMenuTypes.Account}
426+
isPinned={Boolean(account.pinned)}
427+
isHidden={Boolean(account.hidden)}
428+
currentTabOrigin={currentTabOrigin}
429+
isActive={Boolean(account.active)}
430+
privacyMode={privacyMode}
431+
{...accountListItemProps}
432+
/>
433+
</Box>
434+
);
435+
});
436+
}, [
437+
searchResults,
438+
connectedSites,
439+
currentTabOrigin,
440+
privacyMode,
441+
accountListItemProps,
442+
selectedAccount,
443+
onClose,
444+
onAccountListItemItemClicked,
445+
searchQuery,
446+
]);
447+
396448
return (
397449
<Modal isOpen onClose={onClose}>
398450
<ModalOverlay />
@@ -755,9 +807,8 @@ export const AccountListMenu = ({
755807
size: Size.SM,
756808
}}
757809
inputProps={{ autoFocus: true }}
758-
// TODO: These props are required in the TextFieldSearch component. These should be optional
759-
endAccessory
760-
className
810+
endAccessory={null}
811+
className=""
761812
/>
762813
</Box>
763814
) : null}
@@ -773,44 +824,7 @@ export const AccountListMenu = ({
773824
{t('noAccountsFound')}
774825
</Text>
775826
) : null}
776-
{searchResults.map((account) => {
777-
const connectedSite = connectedSites[account.address]?.find(
778-
({ origin }) => origin === currentTabOrigin,
779-
);
780-
781-
const hideAccountListItem =
782-
searchQuery.length === 0 && account.hidden;
783-
784-
/* NOTE: Hidden account will be displayed only in the search list */
785-
786-
return (
787-
<Box
788-
className={
789-
account.hidden
790-
? 'multichain-account-menu-popover__list--menu-item-hidden'
791-
: 'multichain-account-menu-popover__list--menu-item'
792-
}
793-
display={hideAccountListItem ? Display.None : Display.Block}
794-
key={account.address}
795-
>
796-
<AccountListItem
797-
onClick={onAccountListItemItemClicked(account)}
798-
account={account}
799-
key={account.address}
800-
selected={selectedAccount.address === account.address}
801-
closeMenu={onClose}
802-
connectedAvatar={connectedSite?.iconUrl}
803-
menuType={AccountListItemMenuTypes.Account}
804-
isPinned={Boolean(account.pinned)}
805-
isHidden={Boolean(account.hidden)}
806-
currentTabOrigin={currentTabOrigin}
807-
isActive={Boolean(account.active)}
808-
privacyMode={privacyMode}
809-
{...accountListItemProps}
810-
/>
811-
</Box>
812-
);
813-
})}
827+
{accountListItems}
814828
</Box>
815829
{/* Hidden Accounts, this component shows hidden accounts in account list Item*/}
816830
{hiddenAddresses.length > 0 ? (

‎ui/components/multichain/pages/send/components/account-picker.tsx

+3-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import { AccountListMenu } from '../../..';
1616
import { SEND_STAGES, getSendStage } from '../../../../../ducks/send';
1717
import { SendPageRow } from './send-page-row';
1818

19+
const AccountListItemProps = { showOptions: false };
20+
1921
export const SendPageAccountPicker = () => {
2022
const t = useContext(I18nContext);
2123
const internalAccount = useSelector(getSelectedInternalAccount);
@@ -24,7 +26,6 @@ export const SendPageAccountPicker = () => {
2426

2527
const sendStage = useSelector(getSendStage);
2628
const disabled = SEND_STAGES.EDIT === sendStage;
27-
const accountListItemProps = { showOptions: false };
2829
const onAccountListMenuClose = useCallback(() => {
2930
setShowAccountPicker(false);
3031
}, []);
@@ -64,7 +65,7 @@ export const SendPageAccountPicker = () => {
6465
/>
6566
{showAccountPicker ? (
6667
<AccountListMenu
67-
accountListItemProps={accountListItemProps}
68+
accountListItemProps={AccountListItemProps}
6869
showAccountCreation={false}
6970
onClose={onAccountListMenuClose}
7071
allowedAccountTypes={[EthAccountType.Eoa, EthAccountType.Erc4337]}

‎ui/components/multichain/pages/send/components/your-accounts.tsx

+32-26
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useContext, useMemo } from 'react';
1+
import React, { useCallback, useContext, useMemo } from 'react';
22
import { useDispatch, useSelector } from 'react-redux';
33
import { EthAccountType, KeyringAccountType } from '@metamask/keyring-api';
44
import { InternalAccount } from '@metamask/keyring-internal-api';
@@ -17,6 +17,7 @@ import {
1717
MetaMetricsEventCategory,
1818
MetaMetricsEventName,
1919
} from '../../../../../../shared/constants/metametrics';
20+
import { MergedInternalAccount } from '../../../../../selectors/selectors.types';
2021
import { SendPageRow } from './send-page-row';
2122

2223
type SendPageYourAccountsProps = {
@@ -40,6 +41,35 @@ export const SendPageYourAccounts = ({
4041
}, [accounts]);
4142
const selectedAccount = useSelector(getSelectedInternalAccount);
4243

44+
const onClick = useCallback(
45+
(account: MergedInternalAccount) => {
46+
dispatch(
47+
addHistoryEntry(
48+
`sendFlow - User clicked recipient from my accounts. address: ${account.address}, nickname ${account.metadata.name}`,
49+
),
50+
);
51+
trackEvent(
52+
{
53+
event: MetaMetricsEventName.sendRecipientSelected,
54+
category: MetaMetricsEventCategory.Send,
55+
properties: {
56+
location: 'my accounts',
57+
inputType: 'click',
58+
},
59+
},
60+
{ excludeMetaMetricsId: false },
61+
);
62+
dispatch(
63+
updateRecipient({
64+
address: account.address,
65+
nickname: account.metadata.name,
66+
}),
67+
);
68+
dispatch(updateRecipientUserInput(account.address));
69+
},
70+
[dispatch, trackEvent],
71+
);
72+
4373
return (
4474
<SendPageRow>
4575
{/* TODO: Replace `any` with type */}
@@ -51,31 +81,7 @@ export const SendPageYourAccounts = ({
5181
key={account.address}
5282
isPinned={Boolean(account.pinned)}
5383
shouldScrollToWhenSelected={false}
54-
onClick={() => {
55-
dispatch(
56-
addHistoryEntry(
57-
`sendFlow - User clicked recipient from my accounts. address: ${account.address}, nickname ${account.name}`,
58-
),
59-
);
60-
trackEvent(
61-
{
62-
event: MetaMetricsEventName.sendRecipientSelected,
63-
category: MetaMetricsEventCategory.Send,
64-
properties: {
65-
location: 'my accounts',
66-
inputType: 'click',
67-
},
68-
},
69-
{ excludeMetaMetricsId: false },
70-
);
71-
dispatch(
72-
updateRecipient({
73-
address: account.address,
74-
nickname: account.name,
75-
}),
76-
);
77-
dispatch(updateRecipientUserInput(account.address));
78-
}}
84+
onClick={onClick}
7985
/>
8086
))}
8187
</SendPageRow>

‎ui/pages/routes/routes.component.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ export default class Routes extends Component {
483483
{showOnboardingHeader(location) && <OnboardingAppHeader />}
484484
{isAccountMenuOpen ? (
485485
<AccountListMenu
486-
onClose={() => toggleAccountMenu()}
486+
onClose={toggleAccountMenu}
487487
privacyMode={privacyMode}
488488
/>
489489
) : null}

‎ui/selectors/accounts.test.ts

+6
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ describe('Accounts Selectors', () => {
3434
Object.values(mockState.metamask.internalAccounts.accounts),
3535
);
3636
});
37+
38+
it('returns the same object', () => {
39+
const result1 = getInternalAccounts(mockState as AccountsState);
40+
const result2 = getInternalAccounts(mockState as AccountsState);
41+
expect(result1 === result2).toBe(true);
42+
});
3743
});
3844

3945
describe('#getSelectedInternalAccount', () => {

‎ui/selectors/accounts.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
} from '@metamask/keyring-api';
66
import { InternalAccount } from '@metamask/keyring-internal-api';
77
import { AccountsControllerState } from '@metamask/accounts-controller';
8+
import { createSelector } from 'reselect';
89
import {
910
isBtcMainnetAddress,
1011
isBtcTestnetAddress,
@@ -26,9 +27,11 @@ export function isSolanaAccount(account: InternalAccount) {
2627
return Boolean(account && account.type === DataAccount);
2728
}
2829

29-
export function getInternalAccounts(state: AccountsState) {
30-
return Object.values(state.metamask.internalAccounts.accounts);
31-
}
30+
export const getInternalAccounts = createSelector(
31+
(state: AccountsState) =>
32+
Object.values(state.metamask.internalAccounts.accounts),
33+
(accounts) => accounts,
34+
);
3235

3336
export function getSelectedInternalAccount(state: AccountsState) {
3437
const accountId = state.metamask.internalAccounts.selectedAccount;

0 commit comments

Comments
 (0)