MetaMask / MetaMask/metamask-mobile

Add co-located test for live transaction selector in TransactionDetailsSheet

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

Description

> **Performance audit finding** · Severity: **Low** · Effort: Easy · Fix risk: Simple · Test safety net: Uncovered (no co-located TransactionDetailsSheet.test.tsx)
> Owner: `@MetaMask/mobile-core-ux`
> File: `app/components/UI/TransactionElement/TransactionDetailsSheet/TransactionDetailsSheet.tsx:61`

### What is this about?

`TransactionDetailsSheet` subscribes to live transaction state with an inline `useSelector` that calls the parametrized selector `selectTransactionMetadataById(state, tx.id)`, then derives `currentTx` via `useMemo`. The TransactionController updates the metadata for an in-flight transaction frequently (status, gas, confirmations), so this component re-renders on each update. The `useMemo` for `currentTx` clones into a new object every time `liveTransaction` changes, which is correct, but there is no co-located test asserting render behavior, so a regression that broadens the selector (e.g. selecting the whole transactions array) would not be caught.

**Why it matters**

This sheet is shown for pending/recent transactions and reads live-updating Redux state. If a future change replaces the per-id parametrized selector with a coarser selector or drops the `useMemo`, the sheet would re-render on every transaction-controller tick (which fires often during confirmations), with no test to flag the regression.

### Scenario

N/A — see Technical Details.

### Design

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

### Technical Details

**Evidence**

`app/components/UI/TransactionElement/TransactionDetailsSheet/TransactionDetailsSheet.tsx:61`
```ts
const liveTransaction = useSelector((state: RootState) =>
selectTransactionMetadataById(state, tx.id),
);
const currentTx = useMemo(
() =>
liveTransaction
? { ...liveTransaction, txParams: { ...liveTransaction.txParams } }
: tx,
[liveTransaction, tx],
);
```

**Fix**

The current implementation is already reasonable (parametrized selector + memoized projection). The actionable item is to add a co-located `TransactionDetailsSheet.test.tsx` that mounts the sheet, dispatches an unrelated transaction-state change, and asserts the component does not re-render / `currentTx` reference is stable for the same `tx.id`. This locks in the per-id subscription so future selector changes cannot silently regress it.

### Threat Modeling Framework

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

### Acceptance Criteria

- - Add `app/components/UI/TransactionElement/TransactionDetailsSheet/TransactionDetailsSheet.test.tsx` with a render-count assertion across an unrelated TransactionController update.
- Run `yarn jest app/components/UI/TransactionElement/TransactionDetailsSheet`.

### References

- File: `app/components/UI/TransactionElement/TransactionDetailsSheet/TransactionDetailsSheet.tsx:61`
- Source: MetaMask Mobile performance audit — finding `redux-transactiondetailssheet-livetransaction`
- Owner (CODEOWNERS / best-effort): @MetaMask/mobile-core-ux
- Status: **UNVALIDATED**

Contributor guide

Open the contributing guide

Research direction

Start with app/components/UI/TransactionElement/TransactionDetailsSheet/TransactionDetailsSheet.tsx at the live selector, then add the co-located TransactionDetailsSheet.test.tsx. Mount the sheet, dispatch an unrelated TransactionController transaction-state update, and assert the component does not re-render for the same tx.id. Run yarn jest app/components/UI/TransactionElement/TransactionDetailsSheet.

Written by the indexing model from the issue text.

Assessment

Tech stack
react-native, typescript
Domain
mobile, testing
Issue type
Feature
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.