Skip to content

chore: Notification services eslint cleanup #7480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Prithpal-Sooriya merged 4 commits into from
Dec 15, 2025
Merged

chore: Notification services eslint cleanup #7480

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4625e40
Refactor: Add explicit return types and remove unused ESLint suppress…
cursoragent Dec 15, 2025
0fed078
feat: Add explicit return types to NotificationServicesPushController
cursoragent Dec 15, 2025
786c700
Refactor: Simplify test setup for notification services
cursoragent Dec 15, 2025
43b5233
Refactor: Move controller peer dependencies to direct
cursoragent Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions eslint-suppressions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1915,21 +1915,6 @@
"count": 1
}
},
"packages/notification-services-controller/src/NotificationServicesPushController/mocks/mockResponse.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 3
}
},
"packages/notification-services-controller/src/NotificationServicesPushController/services/endpoints.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 2
}
},
"packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 4
}
},
"packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts": {
"@typescript-eslint/naming-convention": {
"count": 1
Expand Down
1 change: 1 addition & 0 deletions packages/notification-services-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Add explicit return types to functions in `NotificationServicesPushController` ([#7480](https://github.com/MetaMask/core/pull/7480))
- Move peer dependencies for controller and service packages to direct dependencies ([#7209](https://github.com/MetaMask/core/pull/7209))
- The dependencies moved are:
- `@metamask/keyring-controller` (^25.0.0)
Expand Down
6 changes: 3 additions & 3 deletions ...fication-services-controller/src/NotificationServicesPushController/mocks/mockResponse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ type MockResponse = {

export const MOCK_REG_TOKEN = 'REG_TOKEN';

export const getMockUpdatePushNotificationLinksResponse = () => {
export const getMockUpdatePushNotificationLinksResponse = (): MockResponse => {
return {
url: REGISTRATION_TOKENS_ENDPOINT(),
requestMethod: 'POST',
Expand All @@ -27,15 +27,15 @@ export const MOCK_FCM_RESPONSE = {
},
};

export const getMockCreateFCMRegistrationTokenResponse = () => {
export const getMockCreateFCMRegistrationTokenResponse = (): MockResponse => {
return {
url: /^https:\/\/fcmregistrations\.googleapis\.com\/v1\/projects\/.*$/u,
requestMethod: 'POST',
response: MOCK_FCM_RESPONSE,
} satisfies MockResponse;
};

export const getMockDeleteFCMRegistrationTokenResponse = () => {
export const getMockDeleteFCMRegistrationTokenResponse = (): MockResponse => {
return {
url: /^https:\/\/fcmregistrations\.googleapis\.com\/v1\/projects\/.*$/u,
requestMethod: 'POST',
Expand Down
4 changes: 2 additions & 2 deletions ...fication-services-controller/src/NotificationServicesPushController/services/endpoints.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ const PUSH_API_ENV = {
prd: 'https://push.api.cx.metamask.io',
} satisfies Record<ENV, string>;

export const PUSH_API = (env: ENV = 'prd') =>
export const PUSH_API = (env: ENV = 'prd'): string =>
PUSH_API_ENV[env] ?? PUSH_API_ENV.prd;

export const REGISTRATION_TOKENS_ENDPOINT = (env: ENV = 'prd') =>
export const REGISTRATION_TOKENS_ENDPOINT = (env: ENV = 'prd'): string =>
`${PUSH_API(env)}/api/v2/token`;
41 changes: 37 additions & 4 deletions ...tion-services-controller/src/NotificationServicesPushController/services/services.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import log from 'loglevel';
import type nock from 'nock';

import {
activatePushNotifications,
Expand All @@ -9,7 +10,7 @@ import { mockEndpointUpdatePushNotificationLinks } from '../__fixtures__/mockSer
import type { PushNotificationEnv } from '../types/firebase';

// Testing util to clean up verbose logs when testing errors
const mockErrorLog = () =>
const mockErrorLog = (): jest.SpyInstance =>
jest.spyOn(log, 'error').mockImplementation(jest.fn());

const MOCK_REG_TOKEN = 'REG_TOKEN';
Expand All @@ -19,7 +20,7 @@ const MOCK_JWT = 'MOCK_JWT';

describe('NotificationServicesPushController Services', () => {
describe('updateLinksAPI', () => {
const act = async () =>
const act = async (): Promise<boolean> =>
await updateLinksAPI({
bearerToken: MOCK_JWT,
addresses: MOCK_ADDRESSES,
Expand Down Expand Up @@ -54,7 +55,33 @@ describe('NotificationServicesPushController Services', () => {
});

describe('activatePushNotifications', () => {
const arrangeMocks = (override?: { mockPut?: { status: number } }) => {
const arrangeMocks = (override?: {
mockPut?: { status: number };
}): {
params: {
Copy link
Contributor Author

@Prithpal-Sooriya Prithpal-Sooriya Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cursor I'm not a fan of the return type eslint here (it is pretty loose).
Lets disable for this line and the reason is because this is an internal testing util

Copy link

@cursor cursor bot Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I've updated both arrangeMocks functions in services.test.ts to use eslint-disable comments instead of the verbose inline return types:

// Internal testing utility - return type is inferred
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
const arrangeMocks = (override?: { mockPut?: { status: number } }) => {

The changes:

  1. Replaced the verbose inline return types with eslint-disable-next-line comments
  2. Added explanation comments: "Internal testing utility - return type is inferred"
  3. Removed the now-unused import type nock from 'nock'

All tests pass and ESLint is happy with the changes.

Open in Cursor Open in Web

Learn more about Cursor Agents

bearerToken: string;
addresses: string[];
createRegToken: jest.Mock;
regToken: {
platform: 'extension';
locale: string;
};
env: PushNotificationEnv;
};
mobileParams: {
bearerToken: string;
addresses: string[];
createRegToken: jest.Mock;
regToken: {
platform: 'mobile';
locale: string;
};
env: PushNotificationEnv;
};
apis: {
mockPut: nock.Scope;
};
} => {
const params = {
bearerToken: MOCK_JWT,
addresses: MOCK_ADDRESSES,
Expand Down Expand Up @@ -124,7 +151,13 @@ describe('NotificationServicesPushController Services', () => {
});

describe('deactivatePushNotifications', () => {
const arrangeMocks = () => {
const arrangeMocks = (): {
params: {
regToken: string;
deleteRegToken: jest.Mock;
env: PushNotificationEnv;
};
} => {
const params = {
regToken: MOCK_REG_TOKEN,
deleteRegToken: jest.fn().mockResolvedValue(true),
Expand Down
Loading