MetaMask / MetaMask/metamask-mobile
Avoid JSON.stringify in usePolling effect dependency
- 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: Covered (app/components/hooks/usePolling.test.ts)
> Owner: `@MetaMask/mobile-platform (suggested)`
> File: `app/components/hooks/usePolling.ts:42`
### What is this about?
`usePolling`'s start/stop effect uses `[usePollingOptions.input && JSON.stringify(usePollingOptions.input)]` as its dependency array. `JSON.stringify` runs over the entire `input` array on every render to compute the dep, and the body again calls `JSON.stringify(input)` per element (and per existing token in the stop loop), giving repeated O(n) serialization on each render.
**Why it matters**
`usePolling` is a shared hook used by many polling controllers (balances, prices, gas, etc.). Stringifying the full input array on every render of every consumer — plus per-element stringify inside the effect's nested loops — is recurring JS-thread cost that scales with the number of polled inputs. The stringify-as-dep is also a band-aid for the absence of a stable input key.
### Scenario
N/A — see Technical Details.
### Design
N/A — internal performance change; no UI/design impact.
### Technical Details
**Evidence**
`app/components/hooks/usePolling.ts:42`
```ts
[usePollingOptions.input && JSON.stringify(usePollingOptions.input)],
```
Inside the effect, repeated stringify:
`app/components/hooks/usePolling.ts:21` and `:31`
```ts
const key = JSON.stringify(input);
...
const exists = usePollingOptions.input.some((i) => inputKey === JSON.stringify(i));
```
**Fix**
Compute a single memoized key with `useMemo(() => JSON.stringify(usePollingOptions.input), [usePollingOptions.input])` and use that as the dependency. Inside the effect, build a per-element key map once (e.g. `input.map(i => [JSON.stringify(i), i])`) and reuse it for both the start and stop loops instead of re-stringifying inside `.some(...)` (which is O(n^2)). Keep behavior identical; only the serialization frequency changes.
### 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/hooks/usePolling.test.ts` to confirm start/stop semantics are preserved.
- Add a test that re-renders with an unchanged `input` and asserts `startPolling`/`stopPollingByPollingToken` are not re-invoked.
### References
- File: `app/components/hooks/usePolling.ts:42`
- Source: MetaMask Mobile performance audit — finding `hookdeps-usepolling-jsonstringify`
- Owner (CODEOWNERS / best-effort): @MetaMask/mobile-platform (suggested)
- Status: **UNVALIDATED**
Contributor guide
Research direction
Start with app/components/hooks/usePolling.ts, especially lines 21, 31, and 42, to trace how polling keys are created and compared. Then run yarn jest app/components/hooks/usePolling.test.ts and add the requested unchanged-input rerender coverage. Done means serialization is reused while start/stop semantics remain covered by the test suite.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- performance, testing-qa
- Issue type
- Refactor
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100