Skip to content
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.

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

chore: Remove BaseControllerV1 #5018

Conversation

MajorLift
Copy link
Contributor

@MajorLift MajorLift commented Dec 3, 2024

Explanation

Removes the deprecated BaseControllerV1 and its associated types and methods, replacing them with the V2 BaseController, its derived classes, and helper methods.

This is the culmination of work completed over multiple quarters, and represents a major step forward for performative and best practice-compliant state management in our clients.

References

Changelog

@metamask/base-controller

Changed

  • Widen input parameter for type guard isBaseController from ControllerInstance to unknown.

Removed

  • BREAKING: Remove class BaseControllerV1 and type guard isBaseControllerV1.
  • BREAKING: Remove types BaseConfig, BaseControllerV1Instance, BaseState, ConfigConstraintV1, Listener, StateConstraintV1, LegacyControllerStateConstraint, ControllerInstance.

@metamask/composable-controller

Changed

  • BREAKING: Re-define ComposableControllerStateConstraint type using StateConstraint instead of LegacyControllerStateConstraint.
  • BREAKING: Constrain the ComposableControllerState generic argument for the ComposableController class using ComposableControllerStateConstraint instead of LegacyComposableControllerStateConstraint.

@metamask/polling-controller

Removed

  • BREAKING: Remove BlockTrackerPollingControllerV1, StaticIntervalPollingControllerV1.

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 highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@MajorLift MajorLift force-pushed the jongsun/perf/composable-controller/241124-remove-expensive-reduce branch from a0b32c1 to 244f6c8 Compare December 3, 2024 15:04
@MajorLift MajorLift force-pushed the jongsun/refactor/base-controller/241203-remove-BaseControllerV1 branch 3 times, most recently from 5f12a92 to 7800e9b Compare December 3, 2024 15:36
Base automatically changed from jongsun/perf/composable-controller/241124-remove-expensive-reduce to main December 5, 2024 02:33
@MajorLift MajorLift force-pushed the jongsun/refactor/base-controller/241203-remove-BaseControllerV1 branch 2 times, most recently from 380b220 to a098f82 Compare January 27, 2025 16:45
@MajorLift MajorLift force-pushed the jongsun/refactor/base-controller/241203-remove-BaseControllerV1 branch from af99108 to 9a3f4b6 Compare January 29, 2025 21:16
@MajorLift MajorLift marked this pull request as ready for review January 29, 2025 21:28
@MajorLift MajorLift requested a review from a team as a code owner January 29, 2025 21:28
@MajorLift MajorLift enabled auto-merge (squash) January 29, 2025 21:29
@MajorLift MajorLift force-pushed the jongsun/refactor/base-controller/241203-remove-BaseControllerV1 branch from d0e584f to 4bc8b22 Compare January 29, 2025 21:34
@MajorLift MajorLift disabled auto-merge January 29, 2025 21:38
@MajorLift MajorLift force-pushed the jongsun/refactor/base-controller/241203-remove-BaseControllerV1 branch from 4bc8b22 to 2f1adbb Compare January 31, 2025 09:58
mcmire
mcmire previously approved these changes Jan 31, 2025
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Woohoo! Looks great to me! (edit: oh, aside from the conflicts, boo. I will keep an eye on this PR then)

@MajorLift MajorLift force-pushed the jongsun/refactor/base-controller/241203-remove-BaseControllerV1 branch from 2f1adbb to e80228f Compare February 2, 2025 23:03
@MajorLift MajorLift enabled auto-merge (squash) February 3, 2025 01:41
@MajorLift MajorLift requested a review from mcmire February 3, 2025 01:41
cryptodev-2s
cryptodev-2s previously approved these changes Feb 3, 2025
Copy link
Contributor

@cryptodev-2s cryptodev-2s left a comment

Choose a reason for hiding this comment

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

LGTM! We want to merge this change along with the removal of Old Messenger Aliases, which is currently in draft: MetaMask/core#5260.

@mcmire
Copy link
Contributor

mcmire commented Feb 3, 2025

Good call @cryptodev-2s, I'm going to mark this as DO-NOT-MERGE until that PR is ready to merge.

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

LGTM!

@cryptodev-2s cryptodev-2s enabled auto-merge (squash) February 5, 2025 15:18
@cryptodev-2s cryptodev-2s merged commit 3c6ebfd into main Feb 5, 2025
127 checks passed
@cryptodev-2s cryptodev-2s deleted the jongsun/refactor/base-controller/241203-remove-BaseControllerV1 branch February 5, 2025 15:23
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.

Remove BaseControllerV1
3 participants