MetaMask / MetaMask/metamask-mobile

Avoid JSON.stringify in usePolling effect dependency

Open Beginner friendly
#31,297 1 comment 0 reactions 0 assignees View on GitHub
area-performance Sev3 size-S ta-ai-fixable ta-triaged team-mobile-platform
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.