MetaMask / MetaMask/metamask-mobile
Drop pointless isEqual on getActiveTabUrl (returns a string)
- 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: Covered (app/components/Views/AccountPermissions/AccountPermissions.test.tsx)
> Owner: `@MetaMask/wallet-integrations (suggested)`
> File: `app/components/Views/AccountPermissions/AccountPermissions.tsx:121`
### What is this about?
`AccountPermissions` does `useSelector(getActiveTabUrl, isEqual)`. `getActiveTabUrl` returns a plain string (the active tab URL). Using `lodash.isEqual` as the equality function for a primitive string is strictly wasteful — the default referential/`===` comparison already handles strings perfectly. `getActiveTabUrl` is also a non-memoized plain function (not `createSelector`) defined in `app/util/transactions/index.js`, so it recomputes on each call, but its string result compares cheaply by value.
**Why it matters**
`isEqual` is heavier than `===` and provides zero benefit for a string. It runs on every Redux dispatch while this screen is mounted. Removing it is a safe micro-optimization and removes a misleading band-aid that implies the value is a complex object.
### Scenario
N/A — see Technical Details.
### Design
N/A — internal performance change; no UI/design impact.
### Technical Details
**Evidence**
`app/components/Views/AccountPermissions/AccountPermissions.tsx:121`
```ts
const origin: string = useSelector(getActiveTabUrl, isEqual);
```
Selector returns a string:
`app/util/transactions/index.js:958`
```js
export const getActiveTabUrl = ({ browser = {} }) =>
browser.tabs &&
browser.activeTab &&
browser.tabs.find(({ id }) => id === browser.activeTab)?.url;
```
**Fix**
Remove the `isEqual` second argument: `const origin = useSelector(getActiveTabUrl);`. Optionally convert `getActiveTabUrl` to a `createSelector` over `state.browser.tabs`/`activeTab` to memoize the lookup, but the string result already compares correctly with default equality.
### 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/Views/AccountPermissions/AccountPermissions.test.tsx`.
- Confirm the favicon/origin-derived UI behaves identically when switching the active tab.
### References
- File: `app/components/Views/AccountPermissions/AccountPermissions.tsx:121`
- Source: MetaMask Mobile performance audit — finding `redux-accountpermissions-isequal-on-string`
- Owner (CODEOWNERS / best-effort): @MetaMask/wallet-integrations (suggested)
- Status: **UNVALIDATED**
Contributor guide
Research direction
Start in app/components/Views/AccountPermissions/AccountPermissions.tsx at line 121, then inspect getActiveTabUrl in app/util/transactions/index.js. Remove the unnecessary equality argument while preserving the origin-derived UI behavior. Run yarn jest app/components/Views/AccountPermissions/AccountPermissions.test.tsx and confirm the test passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, react-native, typescript
- Domain
- frontend, performance, testing-qa
- Issue type
- Refactor
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100