Skip to content

Commit 03b341a

Browse files
authored
fix: remove ens calls when accounts list is opened (MetaMask#23920)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** remove ens calls from account list to improve perf <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: remove ens calls from account list to improve perf ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/0e03ec65-2c9e-4c24-8c72-825d5c809ca7 ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/353267ee-e869-41c8-a01c-1ffbe5b75883 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Introduce a `fetchENS` flag to `useAccounts` (default true) and pass `fetchENS: false` from `AccountSelector` to avoid ENS lookups when the accounts list opens; add tests for both behaviors. > > - **Hooks (`useAccounts`)**: > - Add optional `fetchENS` param (default `true`) to `UseAccountsParams` and hook signature. > - Conditionally call `fetchENSNames` based on `fetchENS`. > - Export `evmAccounts` alongside `accounts` and `ensByAccountAddress` (unchanged behavior). > - **UI (`AccountSelector`)**: > - Pass `{ isLoading: reloadAccounts, `fetchENS: false` }` to `useAccounts` to skip ENS lookups when opening the account list. > - **Tests (`useAccounts.test.ts`)**: > - Add cases verifying ENS fetching when `fetchENS` is true (default and explicit) and not fetching when `false`. > - Minor test description tweak (grammar). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3befab0. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent ead4b0c commit 03b341a

4 files changed

Lines changed: 79 additions & 3 deletions

File tree

app/components/Views/AccountSelector/AccountSelector.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ const AccountSelector = ({ route }: AccountSelectorProps) => {
143143
const accountsParams = useMemo(
144144
() => ({
145145
isLoading: reloadAccounts,
146+
fetchENS: false,
146147
}),
147148
[reloadAccounts],
148149
);

app/components/hooks/useAccounts/useAccounts.test.ts

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,73 @@ describe('useAccounts', () => {
125125
expect(result.current.ensByAccountAddress).toStrictEqual(expectedENSNames);
126126
});
127127

128-
it('return scopes for evm accounts', async () => {
128+
it('returns scopes for evm accounts', async () => {
129129
const { result, waitForNextUpdate } = renderHook(() => useAccounts());
130130
await act(async () => {
131131
await waitForNextUpdate();
132132
});
133133
expect(result.current.accounts[0].scopes).toStrictEqual([EthScope.Eoa]);
134134
});
135+
136+
describe('fetchENS parameter', () => {
137+
it('fetches ENS names when fetchENS is true (default)', async () => {
138+
const expectedENSNames = {
139+
[MOCK_ACCOUNT_1.address]: MOCK_ENS_CACHED_NAME,
140+
};
141+
142+
const { result, waitForNextUpdate } = renderHook(() => useAccounts());
143+
await act(async () => {
144+
await waitForNextUpdate();
145+
});
146+
147+
expect(result.current.ensByAccountAddress).toStrictEqual(
148+
expectedENSNames,
149+
);
150+
});
151+
152+
it('fetches ENS names when fetchENS is explicitly true', async () => {
153+
const expectedENSNames = {
154+
[MOCK_ACCOUNT_1.address]: MOCK_ENS_CACHED_NAME,
155+
};
156+
157+
const { result, waitForNextUpdate } = renderHook(() =>
158+
useAccounts({ fetchENS: true }),
159+
);
160+
await act(async () => {
161+
await waitForNextUpdate();
162+
});
163+
164+
expect(result.current.ensByAccountAddress).toStrictEqual(
165+
expectedENSNames,
166+
);
167+
});
168+
169+
it('does not fetch ENS names when fetchENS is false', async () => {
170+
const { result } = renderHook(() => useAccounts({ fetchENS: false }));
171+
172+
// Give some time for any potential async operations
173+
await act(async () => {
174+
await new Promise((resolve) => setTimeout(resolve, 100));
175+
});
176+
177+
expect(result.current.ensByAccountAddress).toStrictEqual({});
178+
});
179+
180+
it('returns accounts but not ENS names when fetchENS is false', async () => {
181+
const expectedInternalAccounts: Account[] = [
182+
MOCK_ACCOUNT_1,
183+
MOCK_ACCOUNT_2,
184+
];
185+
186+
const { result } = renderHook(() => useAccounts({ fetchENS: false }));
187+
188+
// Give some time for accounts to be populated
189+
await act(async () => {
190+
await new Promise((resolve) => setTimeout(resolve, 100));
191+
});
192+
193+
expect(result.current.accounts).toStrictEqual(expectedInternalAccounts);
194+
expect(result.current.ensByAccountAddress).toStrictEqual({});
195+
});
196+
});
135197
});

app/components/hooks/useAccounts/useAccounts.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
*/
3232
const useAccounts = ({
3333
isLoading = false,
34+
fetchENS = true,
3435
}: UseAccountsParams = {}): UseAccounts => {
3536
const isMountedRef = useRef(false);
3637
const [accounts, setAccounts] = useState<Account[]>([]);
@@ -141,8 +142,15 @@ const useAccounts = ({
141142
setEVMAccounts(
142143
flattenedAccounts.filter((account) => !isNonEvmAddress(account.address)),
143144
);
144-
fetchENSNames({ flattenedAccounts, startingIndex: selectedIndex });
145-
}, [internalAccounts, fetchENSNames, selectedInternalAccount?.address]);
145+
if (fetchENS) {
146+
fetchENSNames({ flattenedAccounts, startingIndex: selectedIndex });
147+
}
148+
}, [
149+
internalAccounts,
150+
fetchENS,
151+
fetchENSNames,
152+
selectedInternalAccount?.address,
153+
]);
146154

147155
useEffect(() => {
148156
if (!isMountedRef.current) {

app/components/hooks/useAccounts/useAccounts.types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ export interface UseAccountsParams {
8989
* @default false
9090
*/
9191
isLoading?: boolean;
92+
/**
93+
* Optional boolean that indicates if ENS names should be fetched for accounts.
94+
* @default true
95+
*/
96+
fetchENS?: boolean;
9297
}
9398

9499
/**

0 commit comments

Comments
 (0)