MetaMask / MetaMask/metamask-mobile
Memoize useBalanceChanges balanceChanges return array
- Dominant language
- TypeScript
- Stars
- 3k
- Forks
- 1.7k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 669
Description
> **Performance audit finding** · Severity: **High** · Effort: Easy · Fix risk: Simple · Test safety net: Covered (app/components/UI/SimulationDetails/useBalanceChanges.test.ts)
> Owner: `@MetaMask/confirmations (suggested)`
> File: `app/components/UI/SimulationDetails/useBalanceChanges.ts:307`
### What is this about?
`useBalanceChanges` constructs a fresh `balanceChanges` array (and fresh `{ pending, value }` wrapper) on every render and returns it without `useMemo`. Any consumer that depends on the returned `value` array reference — including `useSimulationMetrics`, which derives a stringified-properties effect from it — sees a new reference each render.
**Why it matters**
SimulationDetails renders on every confirmation and re-renders as fiat rates, decimals, and currency selectors update. Because `value` is a new array each render, downstream memoization (e.g. `useMemo`/`React.memo` over balance changes, and the `JSON.stringify(params)` effect in `useSimulationMetrics`) cannot short-circuit, causing repeated recomputation and metric dispatches.
### Scenario
N/A — see Technical Details.
### Design
N/A — internal performance change; no UI/design impact.
### Technical Details
**Evidence**
`app/components/UI/SimulationDetails/useBalanceChanges.ts:307`
```ts
const balanceChanges: BalanceChange[] = [
...(nativeChange ? [nativeChange] : []),
...tokenChanges,
];
return { pending: false, value: balanceChanges };
```
`nativeChange` and `tokenChanges` are themselves recomputed inline each render (`getNativeBalanceChange`, `getTokenBalanceChanges` at lines 292 and 299) outside any memo.
**Fix**
Compute `balanceChanges` (and the returned object) inside a `useMemo` keyed on the resolved async values and inputs: `nativeBalanceChange`, `tokenBalanceChanges`, `erc20Decimals.value`, `erc20FiatRates.value`, `erc20UsdRates.value`, `nativeFiatRate`, `nativeUsdRate`, `chainId`. Return a stable `{ pending, value }` object. Note the early-return `{ pending: true, value: [] }` should also use a shared frozen empty-array constant so the pending state is reference-stable too.
### 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/UI/SimulationDetails/useBalanceChanges.test.ts`.
- Add a test that renders the hook twice with identical inputs and asserts `result.value` is `===` across renders.
### References
- File: `app/components/UI/SimulationDetails/useBalanceChanges.ts:307`
- Source: MetaMask Mobile performance audit — finding `unstablehook-usebalancechanges-fresh-array`
- Owner (CODEOWNERS / best-effort): @MetaMask/confirmations (suggested)
- Status: **UNVALIDATED**
Contributor guide
Research direction
Start with app/components/UI/SimulationDetails/useBalanceChanges.ts at the referenced lines, then read app/components/UI/SimulationDetails/useBalanceChanges.test.ts. Run the named Jest test before changing anything and add the specified identical-input rerender assertion. Done means the hook returns the same value reference across unchanged renders while the existing tests continue to pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react-native, typescript
- Domain
- frontend, mobile, performance
- Issue type
- Refactor
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100