chore: Notification controller eslint cleanup

eslint-suppressions.json

@@ -1638,27 +1638,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/phishing-controller/src/BulkTokenScan.test.ts": {
     "@typescript-eslint/explicit-function-return-type": {
       "count": 2

packages/notification-services-controller/src/NotificationServicesPushController/web/push-utils.test.ts

@@ -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);
@@ -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);
@@ -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');
@@ -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(),
@@ -94,7 +107,7 @@ describe('createRegToken() tests', () => {
     return {
       ...firebaseMocks,
     };
-  };
+  }
 
   afterEach(() => {
     jest.clearAllMocks();
@@ -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();
@@ -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();
@@ -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(),
@@ -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();
@@ -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);
 
@@ -303,7 +340,7 @@ describe('createSubscribeToPushNotifications() tests', () => {
     firebaseCallback(payload);
 
     return mocks;
-  };
+  }
 
   it('should invoke handler when notifications are received', async () => {
     const mocks = await arrangeActNotificationReceived(
@@ -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

packages/notification-services-controller/src/NotificationServicesPushController/web/push-utils.ts

@@ -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;
 };
@@ -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
@@ -162,7 +165,7 @@ async function listenToPushNotificationsReceived(
     },
   );
 
-  const unsubscribe = () => unsubscribePushNotifications();
+  const unsubscribe = (): void => unsubscribePushNotifications();
   return unsubscribe;
 }
 
@@ -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;
 }
@@ -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,
@@ -221,7 +224,7 @@ export function createSubscribeToPushNotifications(props: {
       },
     );
     const onClickSub = listenToPushNotificationsClicked(
-      (event, notification) => {
+      (event, notification): void => {
         props.messenger.publish(
           'NotificationServicesPushController:pushNotificationClicked',
           notification,
@@ -230,7 +233,7 @@ export function createSubscribeToPushNotifications(props: {
       },
     );
 
-    const unsubscribe = () => {
+    const unsubscribe = (): void => {
       onBackgroundMessageSub?.();
       onClickSub();
     };

packages/notification-services-controller/src/shared/is-onchain-notification.ts

@@ -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.