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

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

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 (nnotification, 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.

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

Removes unused ESLint suppressions and refactors notification processing logic. Updates API interaction functions for clarity and type safety.

Co-authored-by: prithpal.sooriya <[email protected]>
@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. Deleted the core file - This was an unneeded file (likely a core dump) that shouldn't be in the repository.

  2. Updated the changelog - Added a new entry under the [Unreleased] section documenting the ESLint cleanup:

- Improve code quality by fixing ESLint violations ([#7482](https://github.com/MetaMask/core/pull/7482))
  - Added explicit return types to functions for improved type safety
  - Renamed short variable names to be more descriptive (e.g., `n``notification`, `a``addr`)
  - Refactored to avoid parameter reassignment, promoting immutability

The changes are ready to be staged and committed. Would you like me to stage these changes?


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:14
@Prithpal-Sooriya Prithpal-Sooriya requested review from a team as code owners December 15, 2025 11:14
@Prithpal-Sooriya Prithpal-Sooriya changed the title Notification services eslint cleanup chore: Notification services eslint 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! The changelog changes have been removed. The working tree is now clean with no pending changes. The ESLint cleanup changes in the code files remain intact, but the changelog entry has been reverted since these are internal code quality improvements that don't impact the package's public API or behavior.


Open in Cursor Open in Web

Learn more about Cursor Agents

@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Dec 15, 2025
Merged via the queue into main with commit cec1249 Dec 15, 2025
285 checks passed
@Prithpal-Sooriya Prithpal-Sooriya deleted the cursor/notification-services-eslint-cleanup-0a99 branch December 15, 2025 12:59
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.

5 participants