MetaMask / MetaMask/metamask-mobile

Stabilize useTokenLogo default Set params and memoize returned handlers

Open Beginner friendly
#31,382 1 comment 0 reactions 0 assignees View on GitHub
area-performance Sev3 size-S ta-ai-fixable ta-triaged team-assets team-mobile-platform
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: Covered (app/components/hooks/useTokenLogo/useTokenLogo.test.ts)
> Owner: `@MetaMask/metamask-assets (suggested)`
> File: `app/components/hooks/useTokenLogo/useTokenLogo.ts:38`

### What is this about?

`useTokenLogo` has two interacting issues:

1. **Default params create fresh `Set`s every render**, busting downstream memoization:
```ts
export const useTokenLogo = ({
symbol,
size = 44,
assetsRequiringLightBg = new Set(), // line 38 — new Set each render when omitted
assetsRequiringDarkBg = new Set(), // line 39 — new Set each render when omitted
}: UseTokenLogoConfig): UseTokenLogoReturn => {
```
These two `Set`s are dependencies of the `needsLightBg/needsDarkBg` `useMemo` (line 58), so when a caller omits them the memo recomputes on every render — and that result feeds the `containerStyle` `useMemo` (line 80), so the container style object is rebuilt every render too.

2. **The three event handlers are recreated every render** and the return is a fresh object literal:
```ts
const handleLoadStart = () => { setIsLoading(true); setHasError(false); }; // line 111
const handleLoadEnd = () => { setIsLoading(false); }; // line 116
const handleError = () => { setIsLoading(false); setHasError(true); }; // line 120

return { isLoading, hasError, containerStyle, ..., handleLoadStart, handleLoadEnd, handleError }; // line 125
```
All the styles are carefully `useMemo`'d, but the handlers (passed straight to ``) get a new identity each render, so any memoized child `Image` re-renders anyway.

**Why it matters**

`useTokenLogo` backs `TrendingTokenLogo` (`app/components/UI/Trending/components/TrendingTokenLogo/TrendingTokenLogo.tsx`), which renders one logo per row in trending-token lists — a hot, virtualized list path. The fresh-`Set` default defeats the style memoization the hook author intentionally added, and the unstable handlers defeat `Image` prop stability, so each row recomputes styles and re-binds image callbacks on every render of the list.

### Scenario

N/A — see Technical Details.

### Design

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

### Technical Details

**Evidence**

`app/components/hooks/useTokenLogo/useTokenLogo.ts:38`
```ts
assetsRequiringLightBg = new Set(),
assetsRequiringDarkBg = new Set(),
```
`app/components/hooks/useTokenLogo/useTokenLogo.ts:52`
```ts
const { needsLightBg, needsDarkBg } = useMemo(() => { ... },
[symbol, assetsRequiringLightBg, assetsRequiringDarkBg]); // fresh Sets bust this
```
`app/components/hooks/useTokenLogo/useTokenLogo.ts:111` handlers are inline, returned at line 125.

**Fix**

1. Hoist the empty-`Set` defaults to module-level constants so the default identity is stable:
```ts
const EMPTY_SET = new Set();
... assetsRequiringLightBg = EMPTY_SET, assetsRequiringDarkBg = EMPTY_SET ...
```
2. Wrap `handleLoadStart`/`handleLoadEnd`/`handleError` in `useCallback` with empty deps (they only call stable setters).

### Threat Modeling Framework

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

### Acceptance Criteria

- - Run `yarn jest app/components/hooks/useTokenLogo/useTokenLogo.test.ts`.
- Add a test that renders the hook twice (omitting the `Set` params) and asserts `containerStyle`, `handleLoadStart`, etc. are `===` across renders.

### References

- File: `app/components/hooks/useTokenLogo/useTokenLogo.ts:38`
- Source: MetaMask Mobile performance audit — finding `unstablehook-usetokenlogo-default-set-params-bust-memo`
- Owner (CODEOWNERS / best-effort): @MetaMask/metamask-assets (suggested)
- Status: **UNVALIDATED**

Contributor guide

Open the contributing guide

Research direction

Start with app/components/hooks/useTokenLogo/useTokenLogo.ts, focusing on the defaults, memoized styles, and returned handlers. Run yarn jest app/components/hooks/useTokenLogo/useTokenLogo.test.ts and add the described repeated-render coverage; done means omitted Set parameters and returned style and handler values retain identity without changing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
react-native, typescript
Domain
frontend
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.