MetaMask / MetaMask/metamask-mobile
Remove isEqual band-aid on selectApprovalFlows in useApprovalFlow
- Dominant language
- TypeScript
- Stars
- 3k
- Forks
- 1.7k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 669
Description
> **Performance audit finding** · Severity: **Low** · Effort: Hard · Fix risk: Risky · Test safety net: Partial (useApprovalFlow used via confirmation hook tests; no co-located useApprovalFlow.test.ts)
> Owner: `@MetaMask/confirmations`
> File: `app/components/Views/confirmations/hooks/useApprovalFlow.ts:6`
### What is this about?
`useApprovalFlow` calls `useSelector(selectApprovalFlows, isEqual)`. `selectApprovalFlows` is an identity reselect selector returning the raw `approvalFlows` array slice, so `isEqual` runs a deep comparison of the array on every Redux dispatch to suppress re-renders caused by reference churn from unrelated ApprovalController updates.
**Why it matters**
The hook only ever uses the last element (`approvalFlows?.slice(-1)[0]`). Paying for a deep `isEqual` over the whole flows array on every dispatch — when only the tail element matters — is avoidable overhead, and the comparator hides the missing stable projection.
### Scenario
N/A — see Technical Details.
### Design
N/A — internal performance change; no UI/design impact.
### Technical Details
**Evidence**
`app/components/Views/confirmations/hooks/useApprovalFlow.ts:6`
```ts
const approvalFlows = useSelector(selectApprovalFlows, isEqual);
const approvalFlow = approvalFlows?.length ? approvalFlows?.slice(-1)[0] : undefined;
```
Selector (identity passthrough):
`app/selectors/approvalController.ts:14`
```ts
export const selectApprovalFlows = createSelector(
selectApprovalControllerState,
(approvalControllerState) => approvalControllerState?.approvalFlows,
);
```
**Fix**
Add a memoized `selectCurrentApprovalFlow` selector (`createSelector(selectApprovalFlows, (flows) => (flows?.length ? flows[flows.length - 1] : undefined))`) and consume that with default equality. reselect keeps the result stable while `approvalFlows` is unchanged, removing the per-dispatch deep compare and the `isEqual` band-aid.
### Threat Modeling Framework
N/A — performance-only change; behavior is preserved, no new data flow / trust boundary / attack surface.
### Acceptance Criteria
- - Run the confirmations hook test suite that exercises `useApprovalFlow`.
- Add a selector test asserting `selectCurrentApprovalFlow` returns the last flow and a stable reference for unchanged input.
### References
- File: `app/components/Views/confirmations/hooks/useApprovalFlow.ts:6`
- Source: MetaMask Mobile performance audit — finding `redux-useapprovalflow-isequal-bandaid`
- Owner (CODEOWNERS / best-effort): @MetaMask/confirmations
- Status: **UNVALIDATED**
Contributor guide
Research direction
Start with app/components/Views/confirmations/hooks/useApprovalFlow.ts and app/selectors/approvalController.ts to trace the current selector and last-flow projection. Run the confirmations hook test suite, then verify selector coverage for the last flow and stable unchanged input. Done means the deep comparison is no longer used and the listed selector and hook tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend, mobile
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100