Skip to content

Conversation

@Prithpal-Sooriya
Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya commented Dec 15, 2025

Explanation

  • What is the current state of things and why does it need to change?
    The packages/notification-services-controller contained ESLint suppressions for @typescript-eslint/explicit-function-return-type and @typescript-eslint/prefer-nullish-coalescing. These suppressions indicated areas where type safety and modern JavaScript practices were not fully enforced, leading to potential maintainability issues and reduced code clarity.

  • What is the solution your changes offer and how does it work?
    This PR addresses these suppressions by:

    1. Adding explicit return types to functions in NotificationServicesPushController.test.ts, NotificationServicesPushController.ts, and __fixtures__/mockServices.ts to satisfy the @typescript-eslint/explicit-function-return-type rule.
    2. Replacing a logical OR (||) with the nullish coalescing operator (??) in NotificationServicesPushController.test.ts to fix the @typescript-eslint/prefer-nullish-coalescing rule.
    3. Removing the corresponding entries for these files and rules from eslint-suppressions.json.
  • Are there any changes whose purpose might not obvious to those unfamiliar with the domain?
    No, the changes are straightforward type annotations and a minor logical operator update.

References

  • Fixes ASSETS-2100

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • 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
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Open in Cursor Open in Web


Note

Adds explicit return types across NotificationServicesPushController code/tests, adopts nullish coalescing in tests, and removes related ESLint suppressions.

  • Notification Services Push Controller:
    • Add explicit return types to #getAndAssertBearerToken, #updatePushState, subscribeToPushNotifications, enablePushNotifications, disablePushNotifications, and updateTriggerPushNotifications in NotificationServicesPushController.ts.
  • Tests:
    • Add explicit return types to helpers (mockErrorLog, arrangeServicesMocks, arrangeMockMessenger, mockAuthBearerTokenCall) and improve Jest typings in NotificationServicesPushController.test.ts.
    • Use nullish coalescing (??) for config defaults.
  • Mocks:
    • Add return type to mockEndpointUpdatePushNotificationLinks in __fixtures__/mockServices.ts.
  • ESLint:
    • Remove suppressions for @typescript-eslint/explicit-function-return-type and @typescript-eslint/prefer-nullish-coalescing entries related to these files in eslint-suppressions.json.

Written by Cursor Bugbot for commit f5e64a1. This will update automatically on new commits. Configure here.

@cursor
Copy link

cursor bot commented Dec 15, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

core Outdated
Copy link
Contributor Author

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

Choose a reason for hiding this comment

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

Taking a look!

Open in Cursor Open in Web

@Prithpal-Sooriya
Copy link
Contributor Author

@cursor update the packages/notification-services-controller/CHANGELOG.md changelog to reflect changes made.

@cursor
Copy link

cursor bot commented Dec 15, 2025

I've completed both tasks:

  1. Updated the CHANGELOG.md - Added a new entry under the [Unreleased] section documenting the type safety improvements:
- Improve type safety by adding explicit return types to functions in `NotificationServicesPushController` and related files ([#7481](https://github.com/MetaMask/core/pull/7481))
  1. Removed the core file - This was a ~35MB core dump file that shouldn't be in the repository.

The current changes in this PR now include:

  • eslint-suppressions.json - Removed suppressions for the fixed files
  • packages/notification-services-controller/CHANGELOG.md - Added changelog entry
  • packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts - Added return types and fixed nullish coalescing
  • packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts - Added return types
  • packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts - Added return type

Open in Cursor Open in Web

Learn more about Cursor Agents

@Prithpal-Sooriya Prithpal-Sooriya marked this pull request as ready for review December 15, 2025 11:13
@Prithpal-Sooriya Prithpal-Sooriya requested review from a team as code owners December 15, 2025 11:13
@Prithpal-Sooriya Prithpal-Sooriya changed the title Notification services controller lint cleanup chore: Notification services controller lint cleanup Dec 15, 2025
@Prithpal-Sooriya
Copy link
Contributor Author

@cursor please remove the changelog changes, as these eslint changes does not impact the implementation

@cursor
Copy link

cursor bot commented Dec 15, 2025

Done! I've removed the changelog changes. The PR now only contains the ESLint fix changes:

  • eslint-suppressions.json - Removed suppressions for the fixed files
  • NotificationServicesPushController.test.ts - Added return types and fixed nullish coalescing
  • NotificationServicesPushController.ts - Added return types
  • __fixtures__/mockServices.ts - Added return type

The CHANGELOG.md has been reset to the original version from main, as these ESLint cleanup changes don't impact the implementation.


Open in Cursor Open in Web

Learn more about Cursor Agents

juanmigdr
juanmigdr previously approved these changes Dec 15, 2025
bergarces
bergarces previously approved these changes Dec 15, 2025
@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Dec 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Dec 15, 2025
@Prithpal-Sooriya Prithpal-Sooriya dismissed stale reviews from bergarces and juanmigdr via 52123bd December 15, 2025 13:06
@Prithpal-Sooriya Prithpal-Sooriya force-pushed the cursor/notification-services-controller-lint-cleanup-c8e9 branch from 52123bd to f5e64a1 Compare December 15, 2025 13:49
@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Dec 15, 2025
Merged via the queue into main with commit 69dd293 Dec 15, 2025
285 checks passed
@Prithpal-Sooriya Prithpal-Sooriya deleted the cursor/notification-services-controller-lint-cleanup-c8e9 branch December 15, 2025 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants