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.

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

Jump to bottom

Update ComposableController to BaseController v2 #2082

Closed
mcmire opened this issue Nov 22, 2023 · 1 comment · Fixed by #3590
Closed

Update ComposableController to BaseController v2 #2082

mcmire opened this issue Nov 22, 2023 · 1 comment · Fixed by #3590
Assignees
@MajorLift
Labels
team-wallet-framework

Comments

@mcmire
Copy link
Contributor

mcmire commented Nov 22, 2023

ComposableController still inherits from BaseController v1. In order to build a wallet library, we need to make all of our controllers consistent, and that means we need to migrate this controller to BaseController v2.

Please use #1705 (PhishingController) as a recent reference for this type of migration.
@mcmire mcmire mentioned this issue Nov 22, 2023
Open
28 tasks
@desi
Copy link

desi commented Nov 22, 2023

Hey team! Please add your planning poker estimate with Zenhub @cryptodev-2s @Gudahtt @MajorLift @mcmire @mikesposito

@MajorLift MajorLift self-assigned this Nov 24, 2023
This was referenced Nov 30, 2023
Merged
Closed
MajorLift added a commit that referenced this issue Dec 11, 2023
@MajorLift @mcmire
Upgrade ComposableController to extend BaseControllerV2 (#3590)
9ea6b88 
## Explanation

As part of the upcoming core wallet library initiative, we plan to
migrate all controllers to `BaseControllerV2`. This commit upgrades
`ComposableController`.

## References

- Closes #2082
- See #1509

## Changelog

### `@metamask/composable-controller`

## Added

- Add types `ComposableControllerState`,
`ComposableControllerStateChangeEvent`, `ComposableControllerEvents`,
`ComposableControllerMessenger`.

## Changed

- **BREAKING:** `ComposableController` is upgraded to extend
`BaseControllerV2`.
- The constructor now expects an options object with required properties
`controllers` and `messenger` as its only argument.
- `ComposableController` no longer has a `subscribe` method. Instead,
listeners for `ComposableController` events must be registered to the
controller messenger that generated the restricted messenger assigned to
the instance's `messagingSystem` class field.
- Any getters for `ComposableController` state that access the internal
class field directly should be refactored to instead use listeners that
are subscribed to `ComposableControllerStateChangeEvent`.

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Ellilot Winkler <elliot.winkler@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MajorLift MajorLift

Labels
team-wallet-framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade ComposableController to extend BaseControllerV2 MetaMask/core
3 participants
@mcmire @desi @MajorLift