Skip to content

Commit cc732fb

Browse files
n3psn3ps
authored
chore: unnecessary if's (#37433)
## **Description** No need for a nested `if` without `else` [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37433?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Consolidates nested conditionals and adds minimal guard checks across background, controllers, services, streams, tests, UI, and selectors without altering behavior. > > - **Background** > - Simplify phishing test bypass check in `app/scripts/background.js` by merging nested conditions. > - **Controllers** > - Streamline `addPollingToken` in `app/scripts/controllers/app-state-controller.ts` (combine checks and call updater directly). > - **Services** > - Tighten success URL match with optional chaining and simplify listener logic in `app/scripts/services/subscription/subscription-service.ts`. > - **Streams** > - Collapse legacy notification method remap conditional in `app/scripts/streams/provider-stream.ts`. > - **Tests** > - Flatten error-handling call generation logic in `test/e2e/api-specs/MultichainAuthorizationConfirmationErrors.ts`. > - **UI** > - Add `onNetworkChange` guard and merge conditions on multiselect submit in `ui/components/.../asset-picker.tsx`. > - **Selectors** > - Simplify connected-account selection logic in `ui/selectors/selectors.js` (ensure selected address not already connected). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fdc88d2. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent a37a298 commit cc732fb

File tree

7 files changed

+78
-84
lines changed

7 files changed

+78
-84
lines changed

‎app/scripts/background.js‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -339,11 +339,12 @@ function maybeDetectPhishing(theController) {
339339
}
340340

341341
const { hostname, href, searchParams } = new URL(details.url);
342-
if (inTest) {
343-
if (searchParams.has('IN_TEST_BYPASS_EARLY_PHISHING_DETECTION')) {
344-
// this is a test page that needs to bypass early phishing detection
345-
return {};
346-
}
342+
if (
343+
inTest &&
344+
searchParams.has('IN_TEST_BYPASS_EARLY_PHISHING_DETECTION')
345+
) {
346+
// this is a test page that needs to bypass early phishing detection
347+
return {};
347348
}
348349

349350
theController.phishingController.maybeUpdateState();

‎app/scripts/controllers/app-state-controller.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,11 +1033,10 @@ export class AppStateController extends BaseController<
10331033
): void {
10341034
if (
10351035
pollingTokenType.toString() !==
1036-
POLLING_TOKEN_ENVIRONMENT_TYPES[ENVIRONMENT_TYPE_BACKGROUND]
1036+
POLLING_TOKEN_ENVIRONMENT_TYPES[ENVIRONMENT_TYPE_BACKGROUND] &&
1037+
this.#isValidPollingTokenType(pollingTokenType)
10371038
) {
1038-
if (this.#isValidPollingTokenType(pollingTokenType)) {
1039-
this.#updatePollingTokens(pollingToken, pollingTokenType);
1040-
}
1039+
this.#updatePollingTokens(pollingToken, pollingTokenType);
10411040
}
10421041
}
10431042

‎app/scripts/services/subscription/subscription-service.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,16 +137,17 @@ export class SubscriptionService {
137137
changeInfo: { url: string },
138138
) => {
139139
// We only care about updates to our specific checkout tab
140-
if (tabId === openedTab.id && changeInfo.url) {
141-
if (changeInfo.url.startsWith(params.successUrl)) {
142-
// Payment was successful!
143-
succeeded = true;
144-
145-
// Clean up: close the tab
146-
this.#platform.closeTab(tabId);
147-
}
148-
// TODO: handle cancel url ?
140+
if (
141+
tabId === openedTab.id &&
142+
changeInfo.url?.startsWith(params.successUrl)
143+
) {
144+
// Payment was successful!
145+
succeeded = true;
146+
147+
// Clean up: close the tab
148+
this.#platform.closeTab(tabId);
149149
}
150+
// TODO: handle cancel url ?
150151
};
151152
this.#platform.addTabUpdatedListener(onTabUpdatedListener);
152153

‎app/scripts/streams/provider-stream.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -314,12 +314,13 @@ function getNotificationTransformStream() {
314314
highWaterMark: 16,
315315
objectMode: true,
316316
transform: (chunk, _, cb) => {
317-
if (chunk?.name === METAMASK_EIP_1193_PROVIDER) {
318-
if (chunk.data?.method === 'metamask_accountsChanged') {
319-
chunk.data.method = 'wallet_accountsChanged';
320-
chunk.data.result = chunk.data.params;
321-
delete chunk.data.params;
322-
}
317+
if (
318+
chunk?.name === METAMASK_EIP_1193_PROVIDER &&
319+
chunk.data?.method === 'metamask_accountsChanged'
320+
) {
321+
chunk.data.method = 'wallet_accountsChanged';
322+
chunk.data.result = chunk.data.params;
323+
delete chunk.data.params;
323324
}
324325
cb(null, chunk);
325326
},

‎test/e2e/api-specs/MultichainAuthorizationConfirmationErrors.ts‎

Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -77,55 +77,53 @@ export class MultichainAuthorizationConfirmationErrors implements Rule {
7777
getCalls(__: unknown, method: MethodObject) {
7878
const calls: Call[] = [];
7979
const isMethodAllowed = this.only ? this.only.includes(method.name) : true;
80-
if (isMethodAllowed) {
81-
if (method.errors) {
82-
method.errors.forEach((err) => {
83-
const unsupportedErrorCodes = [5000, 5100, 5101, 5102, 5300, 5301];
84-
const error = err as ErrorObject;
85-
if (unsupportedErrorCodes.includes(error.code)) {
86-
return;
87-
}
88-
let params: Record<string, unknown> = {};
89-
switch (error.code) {
90-
case 5100:
91-
params = {
92-
requiredScopes: {
93-
'eip155:10124': {
94-
methods: ['eth_signTypedData_v4'],
95-
notifications: [],
96-
},
80+
if (isMethodAllowed && method.errors) {
81+
method.errors.forEach((err) => {
82+
const unsupportedErrorCodes = [5000, 5100, 5101, 5102, 5300, 5301];
83+
const error = err as ErrorObject;
84+
if (unsupportedErrorCodes.includes(error.code)) {
85+
return;
86+
}
87+
let params: Record<string, unknown> = {};
88+
switch (error.code) {
89+
case 5100:
90+
params = {
91+
requiredScopes: {
92+
'eip155:10124': {
93+
methods: ['eth_signTypedData_v4'],
94+
notifications: [],
9795
},
98-
};
99-
break;
100-
case 5302:
101-
params = {
102-
requiredScopes: {
103-
'eip155:1': {
104-
methods: ['eth_signTypedData_v4'],
105-
notifications: [],
106-
},
96+
},
97+
};
98+
break;
99+
case 5302:
100+
params = {
101+
requiredScopes: {
102+
'eip155:1': {
103+
methods: ['eth_signTypedData_v4'],
104+
notifications: [],
107105
},
108-
sessionProperties: {},
109-
};
110-
break;
111-
default:
112-
break;
113-
}
106+
},
107+
sessionProperties: {},
108+
};
109+
break;
110+
default:
111+
break;
112+
}
114113

115-
// params should make error happen (or lifecycle hooks will make it happen)
116-
calls.push({
117-
title: `${this.getTitle()} - with error ${error.code} ${
118-
error.message
119-
} `,
120-
methodName: method.name,
121-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
122-
params: params as any,
123-
url: '',
124-
resultSchema: (method.result as ContentDescriptorObject).schema,
125-
expectedResult: error,
126-
});
114+
// params should make error happen (or lifecycle hooks will make it happen)
115+
calls.push({
116+
title: `${this.getTitle()} - with error ${error.code} ${
117+
error.message
118+
} `,
119+
methodName: method.name,
120+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
121+
params: params as any,
122+
url: '',
123+
resultSchema: (method.result as ContentDescriptorObject).schema,
124+
expectedResult: error,
127125
});
128-
}
126+
});
129127
}
130128
return calls;
131129
}

‎ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,13 +196,12 @@ export function AssetPicker({
196196
// If there is only 1 selected network switch to that network to populate tokens
197197
if (
198198
chainIds.length === 1 &&
199-
chainIds[0] !== currentNetworkProviderConfig?.chainId
199+
chainIds[0] !== currentNetworkProviderConfig?.chainId &&
200+
networkProps?.onNetworkChange
200201
) {
201-
if (networkProps?.onNetworkChange) {
202-
networkProps.onNetworkChange(
203-
allNetworks[chainIds[0] as keyof typeof allNetworks],
204-
);
205-
}
202+
networkProps.onNetworkChange(
203+
allNetworks[chainIds[0] as keyof typeof allNetworks],
204+
);
206205
}
207206
}}
208207
selectedChainIds={selectedChainIds}

‎ui/selectors/selectors.js‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3927,16 +3927,11 @@ export function getAccountToConnectToActiveTab(state) {
39273927
const numberOfAccounts = Object.keys(accounts).length;
39283928

39293929
if (
3930-
connectedAccounts.length &&
3931-
connectedAccounts.length !== numberOfAccounts
3930+
connectedAccounts.length > 0 &&
3931+
connectedAccounts.length !== numberOfAccounts &&
3932+
!connectedAccounts.includes(selectedInternalAccount.address)
39323933
) {
3933-
if (
3934-
connectedAccounts.findIndex(
3935-
(address) => address === selectedInternalAccount.address,
3936-
) === -1
3937-
) {
3938-
return getInternalAccount(state, selectedInternalAccount.id);
3939-
}
3934+
return getInternalAccount(state, selectedInternalAccount.id);
39403935
}
39413936

39423937
return undefined;

0 commit comments

Comments
 (0)