Skip to content

Commit cec1249

Browse files
chore: Notification services eslint cleanup (#7482)
## Explanation * **What is the current state of things and why does it need to change?** The `notification-services-controller` package had several ESLint suppressions in `eslint-suppressions.json` for rules such as `@typescript-eslint/explicit-function-return-type`, `id-length`, `id-denylist`, and `no-param-reassign`. These suppressions indicated deviations from our established linting standards, affecting code readability, maintainability, and type safety. * **What is the solution your changes offer and how does it work?** This PR resolves all identified ESLint violations in `process-notifications.ts`, `api-notifications.ts`, and `feature-announcements.test.ts`. * **`@typescript-eslint/explicit-function-return-type`**: Explicit return types were added to functions for improved type safety and clarity. * **`id-length` and `id-denylist`**: Short or disallowed variable names were refactored to be more descriptive, enhancing readability. * **`no-param-reassign`**: Code was refactored to avoid reassigning function parameters, promoting immutability and preventing unexpected side effects. After fixing the code, the corresponding entries were removed from `eslint-suppressions.json`. * **Are there any changes whose purpose might not obvious to those unfamiliar with the domain?** * For `id-length`, many single-character variables (e.g., `n`, `a`, `r`) were expanded to full, descriptive names (e.g., `notification`, `addr`, `response`). * For `no-param-reassign`, new variables were introduced to hold modified data instead of directly altering the original function parameters. * For `explicit-function-return-type`, specific types like `never` for exhaustive checks and `Promise<void>` for async functions were added where appropriate. ## References * Fixes [ASSETS-2100](https://consensyssoftware.atlassian.net/browse/ASSETS-2100) ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them --- <a href="https://cursor.com/background-agent?bcId=bc-d95d2f85-cc08-41dd-bf5d-f59cb48619d3"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-cursor-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-cursor-light.svg"><img alt="Open in Cursor" src="https://cursor.com/open-in-cursor.svg"></picture></a>&nbsp;<a href="https://cursor.com/agents?id=bc-d95d2f85-cc08-41dd-bf5d-f59cb48619d3"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-web-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-web-light.svg"><img alt="Open in Web" src="https://cursor.com/open-in-web.svg"></picture></a> [ASSETS-2100]: https://consensyssoftware.atlassian.net/browse/ASSETS-2100?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Cleans up notification-services-controller by adding explicit return types, renaming short variables, avoiding parameter reassignment, and removing corresponding ESLint suppressions. > > - **Notification processing (`processors/process-notifications.ts`)**: > - Add explicit return types to type guards and helpers (including `never` for exhaustive check). > - Rename variables for clarity (`n` → `notification`, etc.) and tighten generics (`isNotUndefined`). > - Refactor `processAndFilterNotifications` to typed map/filter without implicit any. > - **API services (`services/api-notifications.ts`)**: > - Add explicit return types to endpoint builders and async functions. > - Avoid parameter reassignment by creating normalized copies of address arrays; rename variables (`response`, `apiResponse`). > - Minor comment/identifier cleanups (e.g., "Marks notifications as read", catch `error`). > - **Tests (`services/feature-announcements.test.ts`)**: > - Add explicit Promise return types to helpers; refine typings. > - **ESLint**: > - Remove suppressions for `process-notifications.ts`, `api-notifications.ts`, and `feature-announcements.test.ts` in `eslint-suppressions.json`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d5f632d. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 736915b commit cec1249

File tree

4 files changed

+59
-75
lines changed

4 files changed

+59
-75
lines changed

eslint-suppressions.json

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1602,33 +1602,6 @@
16021602
"count": 1
16031603
}
16041604
},
1605-
"packages/notification-services-controller/src/NotificationServicesController/processors/process-notifications.ts": {
1606-
"@typescript-eslint/explicit-function-return-type": {
1607-
"count": 2
1608-
},
1609-
"id-length": {
1610-
"count": 6
1611-
}
1612-
},
1613-
"packages/notification-services-controller/src/NotificationServicesController/services/api-notifications.ts": {
1614-
"@typescript-eslint/explicit-function-return-type": {
1615-
"count": 8
1616-
},
1617-
"id-denylist": {
1618-
"count": 1
1619-
},
1620-
"id-length": {
1621-
"count": 4
1622-
},
1623-
"no-param-reassign": {
1624-
"count": 2
1625-
}
1626-
},
1627-
"packages/notification-services-controller/src/NotificationServicesController/services/feature-announcements.test.ts": {
1628-
"@typescript-eslint/explicit-function-return-type": {
1629-
"count": 2
1630-
}
1631-
},
16321605
"packages/notification-services-controller/src/NotificationServicesController/services/feature-announcements.ts": {
16331606
"@typescript-eslint/explicit-function-return-type": {
16341607
"count": 2

packages/notification-services-controller/src/NotificationServicesController/processors/process-notifications.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,19 @@ import type { NormalisedAPINotification } from '../types/notification-api/notifi
1717
import type { RawSnapNotification } from '../types/snaps';
1818

1919
const isOnChainNotification = (
20-
n: RawNotificationUnion,
21-
): n is NormalisedAPINotification =>
22-
NOTIFICATION_API_TRIGGER_TYPES_SET.has(n.type);
20+
notification: RawNotificationUnion,
21+
): notification is NormalisedAPINotification =>
22+
NOTIFICATION_API_TRIGGER_TYPES_SET.has(notification.type);
2323

2424
const isFeatureAnnouncement = (
25-
n: RawNotificationUnion,
26-
): n is FeatureAnnouncementRawNotification =>
27-
n.type === TRIGGER_TYPES.FEATURES_ANNOUNCEMENT;
25+
notification: RawNotificationUnion,
26+
): notification is FeatureAnnouncementRawNotification =>
27+
notification.type === TRIGGER_TYPES.FEATURES_ANNOUNCEMENT;
2828

2929
const isSnapNotification = (
30-
n: RawNotificationUnion,
31-
): n is RawSnapNotification => n.type === TRIGGER_TYPES.SNAP;
30+
notification: RawNotificationUnion,
31+
): notification is RawSnapNotification =>
32+
notification.type === TRIGGER_TYPES.SNAP;
3233

3334
/**
3435
* Process feature announcement and wallet notifications into a shared/normalised notification shape.
@@ -42,15 +43,18 @@ export function processNotification(
4243
notification: RawNotificationUnion,
4344
readNotifications: string[] = [],
4445
): INotification {
45-
const exhaustedAllCases = (_: never) => {
46+
const exhaustedAllCases = (_uncheckedCase: never): never => {
4647
const type: string = notification?.type;
4748
throw new Error(`No processor found for notification kind ${type}`);
4849
};
4950

5051
if (isFeatureAnnouncement(notification)) {
51-
const n = processFeatureAnnouncement(notification);
52-
n.isRead = isFeatureAnnouncementRead(n, readNotifications);
53-
return n;
52+
const processedNotification = processFeatureAnnouncement(notification);
53+
processedNotification.isRead = isFeatureAnnouncementRead(
54+
processedNotification,
55+
readNotifications,
56+
);
57+
return processedNotification;
5458
}
5559

5660
if (isSnapNotification(notification)) {
@@ -86,8 +90,11 @@ export function safeProcessNotification(
8690
}
8791
}
8892

89-
const isNotUndefined = <Item>(t?: Item): t is Item => Boolean(t);
93+
const isNotUndefined = <Item>(item?: Item): item is Item => Boolean(item);
9094
export const processAndFilterNotifications = (
91-
ns: RawNotificationUnion[],
95+
notifications: RawNotificationUnion[],
9296
readIds: string[],
93-
) => ns.map((n) => safeProcessNotification(n, readIds)).filter(isNotUndefined);
97+
): INotification[] =>
98+
notifications
99+
.map((notification) => safeProcessNotification(notification, readIds))
100+
.filter(isNotUndefined);

packages/notification-services-controller/src/NotificationServicesController/services/api-notifications.ts

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const TRIGGER_API_ENV = {
2424
prd: 'https://trigger.api.cx.metamask.io',
2525
} satisfies Record<ENV, string>;
2626

27-
export const TRIGGER_API = (env: ENV = 'prd') =>
27+
export const TRIGGER_API = (env: ENV = 'prd'): string =>
2828
TRIGGER_API_ENV[env] ?? TRIGGER_API_ENV.prd;
2929

3030
const NOTIFICATION_API_ENV = {
@@ -33,24 +33,26 @@ const NOTIFICATION_API_ENV = {
3333
prd: 'https://notification.api.cx.metamask.io',
3434
};
3535

36-
export const NOTIFICATION_API = (env: ENV = 'prd') =>
36+
export const NOTIFICATION_API = (env: ENV = 'prd'): string =>
3737
NOTIFICATION_API_ENV[env] ?? NOTIFICATION_API_ENV.prd;
3838

3939
// Gets notification settings for each account provided
40-
export const TRIGGER_API_NOTIFICATIONS_QUERY_ENDPOINT = (env: ENV = 'prd') =>
41-
`${TRIGGER_API(env)}/api/v2/notifications/query`;
40+
export const TRIGGER_API_NOTIFICATIONS_QUERY_ENDPOINT = (
41+
env: ENV = 'prd',
42+
): string => `${TRIGGER_API(env)}/api/v2/notifications/query`;
4243

4344
// Used to create/update account notifications for each account provided
44-
export const TRIGGER_API_NOTIFICATIONS_ENDPOINT = (env: ENV = 'prd') =>
45+
export const TRIGGER_API_NOTIFICATIONS_ENDPOINT = (env: ENV = 'prd'): string =>
4546
`${TRIGGER_API(env)}/api/v2/notifications`;
4647

4748
// Lists notifications for each account provided
48-
export const NOTIFICATION_API_LIST_ENDPOINT = (env: ENV = 'prd') =>
49+
export const NOTIFICATION_API_LIST_ENDPOINT = (env: ENV = 'prd'): string =>
4950
`${NOTIFICATION_API(env)}/api/v3/notifications`;
5051

51-
// Makrs notifications as read
52-
export const NOTIFICATION_API_MARK_ALL_AS_READ_ENDPOINT = (env: ENV = 'prd') =>
53-
`${NOTIFICATION_API(env)}/api/v3/notifications/mark-as-read`;
52+
// Marks notifications as read
53+
export const NOTIFICATION_API_MARK_ALL_AS_READ_ENDPOINT = (
54+
env: ENV = 'prd',
55+
): string => `${NOTIFICATION_API(env)}/api/v3/notifications/mark-as-read`;
5456

5557
/**
5658
* fetches notification config (accounts enabled vs disabled)
@@ -66,31 +68,31 @@ export async function getNotificationsApiConfigCached(
6668
bearerToken: string,
6769
addresses: string[],
6870
env: ENV = 'prd',
69-
) {
71+
): Promise<{ address: string; enabled: boolean }[]> {
7072
if (addresses.length === 0) {
7173
return [];
7274
}
7375

74-
addresses = addresses.map((a) => a.toLowerCase());
76+
const normalizedAddresses = addresses.map((addr) => addr.toLowerCase());
7577

76-
const cached = notificationsConfigCache.get(addresses);
78+
const cached = notificationsConfigCache.get(normalizedAddresses);
7779
if (cached) {
7880
return cached;
7981
}
8082

8183
type RequestBody = { address: string }[];
8284
type Response = { address: string; enabled: boolean }[];
83-
const body: RequestBody = addresses.map((address) => ({ address }));
84-
const data = await makeApiCall(
85+
const body: RequestBody = normalizedAddresses.map((address) => ({ address }));
86+
const apiResponse = await makeApiCall(
8587
bearerToken,
8688
TRIGGER_API_NOTIFICATIONS_QUERY_ENDPOINT(env),
8789
'POST',
8890
body,
8991
)
90-
.then<Response | null>((r) => (r.ok ? r.json() : null))
92+
.then<Response | null>((response) => (response.ok ? response.json() : null))
9193
.catch(() => null);
9294

93-
const result = data ?? [];
95+
const result = apiResponse ?? [];
9496

9597
if (result.length > 0) {
9698
notificationsConfigCache.set(result);
@@ -111,25 +113,25 @@ export async function updateOnChainNotifications(
111113
bearerToken: string,
112114
addresses: { address: string; enabled: boolean }[],
113115
env: ENV = 'prd',
114-
) {
116+
): Promise<void> {
115117
if (addresses.length === 0) {
116118
return;
117119
}
118120

119-
addresses = addresses.map((a) => {
120-
a.address = a.address.toLowerCase();
121-
return a;
122-
});
121+
const normalizedAddresses = addresses.map((item) => ({
122+
...item,
123+
address: item.address.toLowerCase(),
124+
}));
123125

124126
type RequestBody = { address: string; enabled: boolean }[];
125-
const body: RequestBody = addresses;
127+
const body: RequestBody = normalizedAddresses;
126128
await makeApiCall(
127129
bearerToken,
128130
TRIGGER_API_NOTIFICATIONS_ENDPOINT(env),
129131
'POST',
130132
body,
131133
)
132-
.then(() => notificationsConfigCache.set(addresses))
134+
.then(() => notificationsConfigCache.set(normalizedAddresses))
133135
.catch(() => null);
134136
}
135137

@@ -160,7 +162,7 @@ export async function getAPINotifications(
160162
Schema.paths['/api/v3/notifications']['post']['responses']['200']['content']['application/json'];
161163

162164
const body: RequestBody = {
163-
addresses: addresses.map((a) => a.toLowerCase()),
165+
addresses: addresses.map((addr) => addr.toLowerCase()),
164166
locale,
165167
platform,
166168
};
@@ -170,23 +172,25 @@ export async function getAPINotifications(
170172
'POST',
171173
body,
172174
)
173-
.then<APIResponse | null>((r) => (r.ok ? r.json() : null))
175+
.then<APIResponse | null>((response) =>
176+
response.ok ? response.json() : null,
177+
)
174178
.catch(() => null);
175179

176180
// Transform and sort notifications
177181
const transformedNotifications = notifications
178-
?.map((n): UnprocessedRawNotification | undefined => {
179-
if (!n.notification_type) {
182+
?.map((notification): UnprocessedRawNotification | undefined => {
183+
if (!notification.notification_type) {
180184
return undefined;
181185
}
182186

183187
try {
184-
return toRawAPINotification(n);
188+
return toRawAPINotification(notification);
185189
} catch {
186190
return undefined;
187191
}
188192
})
189-
.filter((n): n is NormalisedAPINotification => Boolean(n));
193+
.filter((item): item is NormalisedAPINotification => Boolean(item));
190194

191195
return transformedNotifications ?? [];
192196
}
@@ -223,7 +227,7 @@ export async function markNotificationsAsRead(
223227
'POST',
224228
body,
225229
);
226-
} catch (err) {
227-
log.error('Error marking notifications as read:', err);
230+
} catch (error) {
231+
log.error('Error marking notifications as read:', error);
228232
}
229233
}

packages/notification-services-controller/src/NotificationServicesController/services/feature-announcements.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('Feature Announcement Notifications', () => {
3333

3434
const assertEnvEmpty = async (
3535
override: Partial<typeof featureAnnouncementsEnv>,
36-
) => {
36+
): Promise<void> => {
3737
const result = await getFeatureAnnouncementNotifications({
3838
...featureAnnouncementsEnv,
3939
...override,
@@ -125,7 +125,7 @@ describe('Feature Announcement Notifications', () => {
125125
minimumVersion: string | undefined,
126126
maximumVersion: string | undefined,
127127
platformVersion: string | undefined,
128-
) => {
128+
): Promise<INotification[]> => {
129129
const apiResponse = createMockFeatureAnnouncementAPIResult();
130130
if (apiResponse.items?.[0]) {
131131
apiResponse.items[0].fields.extensionMinimumVersionNumber = undefined;

0 commit comments

Comments
 (0)