Skip to content

chore: Notification controller eslint cleanup #7483

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 5 commits into from
Dec 15, 2025
Merged

chore: Notification controller eslint cleanup #7483

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9465aba
Refactor: Improve type safety and testability in push notifications
cursoragent Dec 15, 2025
e0e4636
Remove accidental core dump file and update changelog
cursoragent Dec 15, 2025
0d5fd3a
Remove changelog entry for ESLint cleanup
cursoragent Dec 15, 2025
b21afad
Merge branch 'main' into cursor/notification-controller-eslint-cleanu…
Prithpal-Sooriya Dec 15, 2025
ba5e91a
Merge branch 'main' of github.com:MetaMask/core into cursor/notificat…
Prithpal-Sooriya 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
Binary file added core
View file
Open in desktop
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 remove this file

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 completed both tasks:

  1. Removed the core file - This was an accidentally committed core dump file (36MB ELF binary from light-locker)

  2. Updated the changelog - Added an entry under [Unreleased] > Changed for PR chore: Notification controller eslint cleanup #7483:

    - Improve code quality by adding explicit return types and fixing ESLint violations in push notification utilities ([#7483](https://github.com/MetaMask/core/pull/7483))

The changes have been committed and pushed to the branch.

Open in Cursor Open in Web

Learn more about Cursor Agents

Binary file not shown.
21 changes: 0 additions & 21 deletions eslint-suppressions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1957,27 +1957,6 @@
"count": 9
}
},
"packages/notification-services-controller/src/NotificationServicesPushController/web/push-utils.test.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 10
},
"no-restricted-globals": {
"count": 3
}
},
"packages/notification-services-controller/src/NotificationServicesPushController/web/push-utils.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 7
},
"require-atomic-updates": {
"count": 1
}
},
"packages/notification-services-controller/src/shared/is-onchain-notification.ts": {
"id-length": {
"count": 1
}
},
"packages/notification-services-controller/src/shared/to-raw-notification.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 1
Expand Down
86 changes: 62 additions & 24 deletions ...ication-services-controller/src/NotificationServicesPushController/web/push-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ const firebaseApp: FirebaseAppModule.FirebaseApp = {
options: mockEnv,
};

const arrangeFirebaseAppMocks = () => {
function arrangeFirebaseAppMocks(): {
mockGetApp: jest.SpyInstance;
mockInitializeApp: jest.SpyInstance;
} {
const mockGetApp = jest
.spyOn(FirebaseAppModule, 'getApp')
.mockReturnValue(firebaseApp);
Expand All @@ -43,9 +46,14 @@ const arrangeFirebaseAppMocks = () => {
.mockReturnValue(firebaseApp);

return { mockGetApp, mockInitializeApp };
};

const arrangeFirebaseMessagingSWMocks = () => {
}

function arrangeFirebaseMessagingSWMocks(): {
mockIsSupported: jest.SpyInstance;
mockGetMessaging: jest.SpyInstance;
mockOnBackgroundMessage: jest.SpyInstance;
mockOnBackgroundMessageUnsub: jest.Mock;
} {
const mockIsSupported = jest
.spyOn(FirebaseMessagingSWModule, 'isSupported')
.mockResolvedValue(true);
Expand All @@ -65,9 +73,12 @@ const arrangeFirebaseMessagingSWMocks = () => {
mockOnBackgroundMessage,
mockOnBackgroundMessageUnsub,
};
};
}

const arrangeFirebaseMessagingMocks = () => {
function arrangeFirebaseMessagingMocks(): {
mockGetToken: jest.SpyInstance;
mockDeleteToken: jest.SpyInstance;
} {
const mockGetToken = jest
.spyOn(FirebaseMessagingModule, 'getToken')
.mockResolvedValue('test-token');
Expand All @@ -77,12 +88,14 @@ const arrangeFirebaseMessagingMocks = () => {
.mockResolvedValue(true);

return { mockGetToken, mockDeleteToken };
};
}

describe('createRegToken() tests', () => {
const TEST_TOKEN = 'test-token';

const arrange = () => {
function arrange(): ReturnType<typeof arrangeFirebaseAppMocks> &
ReturnType<typeof arrangeFirebaseMessagingSWMocks> &
ReturnType<typeof arrangeFirebaseMessagingMocks> {
const firebaseMocks = {
...arrangeFirebaseAppMocks(),
...arrangeFirebaseMessagingSWMocks(),
Expand All @@ -94,7 +107,7 @@ describe('createRegToken() tests', () => {
return {
...firebaseMocks,
};
};
}

afterEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -147,13 +160,15 @@ describe('createRegToken() tests', () => {
});

describe('deleteRegToken() tests', () => {
const arrange = () => {
function arrange(): ReturnType<typeof arrangeFirebaseAppMocks> &
ReturnType<typeof arrangeFirebaseMessagingSWMocks> &
ReturnType<typeof arrangeFirebaseMessagingMocks> {
return {
...arrangeFirebaseAppMocks(),
...arrangeFirebaseMessagingSWMocks(),
...arrangeFirebaseMessagingMocks(),
};
};
}

afterEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -193,7 +208,13 @@ describe('deleteRegToken() tests', () => {
});

describe('createSubscribeToPushNotifications() tests', () => {
const arrangeMessengerMocks = () => {
function arrangeMessengerMocks(): {
messenger: ReturnType<
typeof buildPushPlatformNotificationsControllerMessenger
>;
onNewNotificationsListener: jest.Mock;
pushNotificationClickedListener: jest.Mock;
} {
const messenger = buildPushPlatformNotificationsControllerMessenger();

const onNewNotificationsListener = jest.fn();
Expand All @@ -213,19 +234,31 @@ describe('createSubscribeToPushNotifications() tests', () => {
onNewNotificationsListener,
pushNotificationClickedListener,
};
};

const arrangeClickListenerMocks = () => {
}

function arrangeClickListenerMocks(): {
mockAddEventListener: jest.SpyInstance;
mockRemoveEventListener: jest.SpyInstance;
} {
// Testing service worker functionality requires using the 'self' global
// eslint-disable-next-line no-restricted-globals
const mockAddEventListener = jest.spyOn(self, 'addEventListener');
// eslint-disable-next-line no-restricted-globals
const mockRemoveEventListener = jest.spyOn(self, 'removeEventListener');

return {
mockAddEventListener,
mockRemoveEventListener,
};
};

const arrange = () => {
}

function arrange(): ReturnType<typeof arrangeFirebaseAppMocks> &
ReturnType<typeof arrangeFirebaseMessagingSWMocks> &
ReturnType<typeof arrangeMessengerMocks> &
ReturnType<typeof arrangeClickListenerMocks> & {
mockOnReceivedHandler: jest.Mock;
mockOnClickHandler: jest.Mock;
} {
const firebaseMocks = {
...arrangeFirebaseAppMocks(),
...arrangeFirebaseMessagingSWMocks(),
Expand All @@ -238,17 +271,19 @@ describe('createSubscribeToPushNotifications() tests', () => {
mockOnReceivedHandler: jest.fn(),
mockOnClickHandler: jest.fn(),
};
};
}

const actCreateSubscription = async (mocks: ReturnType<typeof arrange>) => {
async function actCreateSubscription(
mocks: ReturnType<typeof arrange>,
): Promise<() => void> {
const unsubscribe = await createSubscribeToPushNotifications({
messenger: mocks.messenger,
onReceivedHandler: mocks.mockOnReceivedHandler,
onClickHandler: mocks.mockOnClickHandler,
})(mockEnv);

return unsubscribe;
};
}

afterEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -288,7 +323,9 @@ describe('createSubscribeToPushNotifications() tests', () => {
expect(mocks.mockRemoveEventListener).toHaveBeenCalled();
});

const arrangeActNotificationReceived = async (testData: unknown) => {
async function arrangeActNotificationReceived(
testData: unknown,
): Promise<ReturnType<typeof arrange>> {
const mocks = arrange();
await actCreateSubscription(mocks);

Expand All @@ -303,7 +340,7 @@ describe('createSubscribeToPushNotifications() tests', () => {
firebaseCallback(payload);

return mocks;
};
}

it('should invoke handler when notifications are received', async () => {
const mocks = await arrangeActNotificationReceived(
Expand Down Expand Up @@ -351,7 +388,8 @@ describe('createSubscribeToPushNotifications() tests', () => {
notification: { data: notificationData },
});

// Act
// Act - Testing service worker notification click event
// eslint-disable-next-line no-restricted-globals
self.dispatchEvent(mockNotificationEvent);

// Assert Click Notification Event & Handler Calls
Expand Down
25 changes: 14 additions & 11 deletions ...notification-services-controller/src/NotificationServicesPushController/web/push-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ declare const self: ServiceWorkerGlobalScope;
// eslint-disable-next-line import-x/no-mutable-exports
export let supportedCache: boolean | null = null;

const getPushAvailability = async () => {
const getPushAvailability = async (): Promise<boolean> => {
// Race condition is acceptable here - worst case is isSupported() is called
// multiple times during initialization, which is harmless for caching a boolean
// eslint-disable-next-line require-atomic-updates
supportedCache ??= await isSupported();
return supportedCache;
};
Expand Down Expand Up @@ -129,7 +132,7 @@ async function listenToPushNotificationsReceived(
const unsubscribePushNotifications = onBackgroundMessage(
messaging,
// eslint-disable-next-line @typescript-eslint/no-misused-promises
async (payload: MessagePayload) => {
async (payload: MessagePayload): Promise<void> => {
try {
// MessagePayload shapes are not known
// TODO - provide open-api unfied backend/frontend types
Expand Down Expand Up @@ -162,7 +165,7 @@ async function listenToPushNotificationsReceived(
},
);

const unsubscribe = () => unsubscribePushNotifications();
const unsubscribe = (): void => unsubscribePushNotifications();
return unsubscribe;
}

Expand All @@ -174,15 +177,15 @@ async function listenToPushNotificationsReceived(
*/
function listenToPushNotificationsClicked(
handler: (e: NotificationEvent, notification: Types.INotification) => void,
) {
const clickHandler = (event: NotificationEvent) => {
): () => void {
const clickHandler = (event: NotificationEvent): void => {
// Get Data
const data: Types.INotification = event?.notification?.data;
handler(event, data);
};

self.addEventListener('notificationclick', clickHandler);
const unsubscribe = () =>
const unsubscribe = (): void =>
self.removeEventListener('notificationclick', clickHandler);
return unsubscribe;
}
Expand All @@ -208,11 +211,11 @@ export function createSubscribeToPushNotifications(props: {
notification: Types.INotification,
) => void;
messenger: NotificationServicesPushControllerMessenger;
}) {
return async function (env: PushNotificationEnv) {
}): (env: PushNotificationEnv) => Promise<() => void> {
return async function (env: PushNotificationEnv): Promise<() => void> {
const onBackgroundMessageSub = await listenToPushNotificationsReceived(
env,
async (notification) => {
async (notification): Promise<void> => {
props.messenger.publish(
'NotificationServicesPushController:onNewNotifications',
notification,
Expand All @@ -221,7 +224,7 @@ export function createSubscribeToPushNotifications(props: {
},
);
const onClickSub = listenToPushNotificationsClicked(
(event, notification) => {
(event, notification): void => {
props.messenger.publish(
'NotificationServicesPushController:pushNotificationClicked',
notification,
Expand All @@ -230,7 +233,7 @@ export function createSubscribeToPushNotifications(props: {
},
);

const unsubscribe = () => {
const unsubscribe = (): void => {
onBackgroundMessageSub?.();
onClickSub();
};
Expand Down
8 changes: 4 additions & 4 deletions packages/notification-services-controller/src/shared/is-onchain-notification.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import type { OnChainRawNotification } from '../NotificationServicesController';
/**
* Checks if the given value is an OnChainRawNotification object.
*
* @param n - The value to check.
* @param notification - The value to check.
* @returns True if the value is an OnChainRawNotification object, false otherwise.
*/
export function isOnChainRawNotification(
n: unknown,
): n is OnChainRawNotification {
const assumed = n as OnChainRawNotification;
notification: unknown,
): notification is OnChainRawNotification {
const assumed = notification as OnChainRawNotification;

// We don't have a validation/parsing library to check all possible types of an on chain notification
// It is safe enough just to check "some" fields, and catch any errors down the line if the shape is bad.
Expand Down
Loading