MetaMask / MetaMask/metamask-mobile

Memoize selectPopularNetworkConfigurationsByCaipChainId filter result

Open Beginner friendly
#31,352 2 comments 0 reactions 0 assignees View on GitHub
area-performance Sev3 size-XS ta-triaged team-mobile-ux team-networks
Dominant language
TypeScript
Stars
3k
Forks
1.7k
Avg merge
1d 14h
Merged PRs (30d)
669

Description

> **Performance audit finding** · Severity: **Medium** · Effort: Easy · Fix risk: Simple · Test safety net: Partial
> Owner: `@MetaMask/mobile-core-ux (suggested)`
> File: `app/selectors/networkController.ts:287`

### What is this about?

`selectPopularNetworkConfigurationsByCaipChainId` is a plain `createSelector` whose result function returns `Object.values(...).filter(...)`. This builds a new array every recompute. Its input `selectNetworkConfigurationsByCaipChainId` is itself a plain `createSelector` whose result function calls `getNetworkConfigurationsByCaipChainId` (which builds a fresh object via spreads), so the input reference also changes whenever either network slice changes — chaining the instability.

**Why it matters**

Network configuration lists drive network pickers and multi-network token filtering. A new array reference on each recompute re-renders consumers even when popular networks are unchanged. Imported in ~5 modules.

### Scenario

N/A — see Technical Details.

### Design

N/A — internal performance change; no UI/design impact.

### Technical Details

**Evidence**

`app/selectors/networkController.ts:287`
```ts
export const selectPopularNetworkConfigurationsByCaipChainId = createSelector(
selectNetworkConfigurationsByCaipChainId,
(networkConfigurationsByChainId) =>
Object.values(networkConfigurationsByChainId).filter(
(networkConfiguration) =>
POPULAR_NETWORK_CHAIN_IDS.has(networkConfiguration.chainId as Hex) &&
!NON_EVM_TESTNET_IDS.includes(networkConfiguration.caipChainId),
),
);
```

**Fix**

Switch to `createDeepEqualSelector` so the filtered array stays referentially stable when contents are unchanged. (Consider also wrapping the upstream `selectNetworkConfigurationsByCaipChainId` in deep-equal, since its `getNetworkConfigurationsByCaipChainId` builds a new object each call.)
```ts
export const selectPopularNetworkConfigurationsByCaipChainId =
createDeepEqualSelector(
selectNetworkConfigurationsByCaipChainId,
(networkConfigurationsByChainId) =>
Object.values(networkConfigurationsByChainId).filter(/* ... */),
);
```

### Threat Modeling Framework

N/A — performance-only change; behavior is preserved, no new data flow / trust boundary / attack surface.

### Acceptance Criteria

- `app/selectors/networkController.test.ts:406` asserts values but not referential stability. Add a test asserting `toBe` across two equal-content states. Profiler: confirm network-picker consumers stop re-rendering on unrelated network-state changes.

### References

- File: `app/selectors/networkController.ts:287`
- Source: MetaMask Mobile performance audit — finding `selector-popular-network-configs-by-caip`
- Owner (CODEOWNERS / best-effort): @MetaMask/mobile-core-ux (suggested)
- Status: **UNVALIDATED**

Contributor guide

Open the contributing guide

Research direction

Start in app/selectors/networkController.ts:287 and read the existing selector plus the createDeepEqualSelector utility. Then inspect app/selectors/networkController.test.ts:406 and run its tests; done means the selector returns the same array reference for equal-content states while preserving the existing values.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
mobile, performance
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.