MetaMask / MetaMask/metamask-mobile
Add cancellation (`AbortController` / cancelled flag) to async effects that `setState` after resolve
- Dominant language
- TypeScript
- Stars
- 3k
- Forks
- 1.7k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 669
Description
> **Performance audit finding** · Severity: **Medium (batch)** · Effort: Medium · Fix risk: Simple · Test safety net: Partial
> Owner: per-file CODEOWNERS
> Scope: 16 sweep hits app-wide
### What is this about?
Async effects that `fetch(...)`/`.then(...)` and set state on resolve, with no cancelled flag or `AbortController`, keep running after unmount (wasted work, "setState on unmounted component") and race when the driving input changes quickly (stale response overwrites fresh data). Sweep on `main` (`b3e7eb88`) found 16 candidate sites. Verified instances:
- [ ] `app/components/Views/ConnectQRHardware/index.tsx:129` — `KeyringController.getAccounts().then(...)`
- [ ] `app/components/Views/LedgerSelectAccount/index.tsx:135` and `:210`
- [ ] `app/components/Views/RevealPrivateCredential/RevealPrivateCredential.tsx:139`
- [ ] remaining 12 hits: re-run the detection recipe and triage (some may be guarded upstream)
Detection recipe:
`grep -rn -A6 "useEffect(" app --include="*.ts" --include="*.tsx" | grep -E "fetch\(|\.then\(" | grep -v "signal\|cancelled\|abort" | grep -v ".test."`
### Technical Details
Extract shared hooks ([`useIsMounted`](https://github.com/MetaMask/MetaMask-planning/issues/6544), [`useAbortableEffect`](https://github.com/MetaMask/MetaMask-planning/issues/6542)) so cleanup is default.
Fix per site: cancelled-flag cleanup (cheapest, works for any promise) or `AbortController` (also cancels the request; RN fetch supports `signal`).
### Acceptance Criteria
- All 16 hits triaged: fixed or waived with a stated reason (e.g. app-lifetime component).
- No "setState on unmounted component" warnings when rapidly entering/leaving the affected screens.
### References
- Source: `mms-performance` sweep recipes added in MetaMask/skills#49 (`mm-useeffect-antipatterns`)
- Status: **UNVALIDATED** per instance (static sweep)
Contributor guide
Research direction
Start by rerunning the provided grep recipe on app/, then inspect the listed effects in app/components/Views/ConnectQRHardware/index.tsx, LedgerSelectAccount/index.tsx, and RevealPrivateCredential/RevealPrivateCredential.tsx. Triage all 16 hits, applying the stated cleanup approach or recording a waiver, then test rapid entry and exit of the affected screens for unmounted-state warnings.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react-native, typescript
- Domain
- frontend, mobile
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100