MetaMask / MetaMask/metamask-mobile

Drop pointless isEqual on getActiveTabUrl (returns a string)

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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.