MetaMask / MetaMask/metamask-mobile
Use stable EMPTY_OBJECT for selectSwapsTransactions fallback
- 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/confirmations`
> File: `app/selectors/transactionController.ts:294`
### What is this about?
`selectSwapsTransactions` is a plain `createSelector` whose result function returns `transactionControllerState.swapsTransactions ?? {}`. The input is the whole `TransactionController` slice. When `swapsTransactions` is undefined, the `?? {}` creates a NEW empty object literal on every recompute, and because the input is the full controller slice, any transaction-state change recomputes and yields a fresh `{}` reference.
**Why it matters**
Consumers reading swaps transactions re-render whenever ANY transaction-controller field changes (very frequent during pending/confirming transactions), even when there are no swaps. The `?? {}` defeats memoization for all empty-swaps users.
### Scenario
N/A — see Technical Details.
### Design
N/A — internal performance change; no UI/design impact.
### Technical Details
**Evidence**
`app/selectors/transactionController.ts:294`
```ts
export const selectSwapsTransactions = createSelector(
selectTransactionControllerState,
(transactionControllerState) =>
//@ts-expect-error - This is populated at the app level...
transactionControllerState.swapsTransactions ?? {},
);
```
**Fix**
Define a module-level frozen `EMPTY_OBJECT` and return it for the fallback, and/or use `createDeepEqualSelector`. Minimal mechanical fix:
```ts
const EMPTY_SWAPS = Object.freeze({});
export const selectSwapsTransactions = createSelector(
selectTransactionControllerState,
(transactionControllerState) =>
transactionControllerState.swapsTransactions ?? EMPTY_SWAPS,
);
```
### Threat Modeling Framework
N/A — performance-only change; behavior is preserved, no new data flow / trust boundary / attack surface.
### Acceptance Criteria
- `app/selectors/transactionController.test.ts:88` asserts `toStrictEqual({})` for the empty path but not referential stability. Add `expect(selectSwapsTransactions(s1)).toBe(selectSwapsTransactions(s2))` for two states with no swaps. Profiler: confirm swaps consumers stop re-rendering on unrelated transaction updates.
### References
- File: `app/selectors/transactionController.ts:294`
- Source: MetaMask Mobile performance audit — finding `selector-swaps-transactions-empty-object`
- Owner (CODEOWNERS / best-effort): @MetaMask/confirmations
- Status: **UNVALIDATED**
Contributor guide
Research direction
Start in app/selectors/transactionController.ts at selectSwapsTransactions, then read app/selectors/transactionController.test.ts around line 88. Run the selector tests and compare two states without swaps. Done means the empty result remains referentially stable across unrelated transaction-state changes while existing selector behavior stays intact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- performance
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100