Skip to content

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Aug 14, 2025

Explanation

Migrate the upcoming BaseController class to use the new @metamask/messenger package. This includes replacing RestrictedMessenger with the new Messenger class that supports delegation.

References

Relates to #5626

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, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@Gudahtt Gudahtt force-pushed the messenger-migrate-base-controller branch from 2b24ae4 to 52e6756 Compare August 14, 2025 22:15
@Gudahtt Gudahtt force-pushed the create-next-base-controller-export branch from 5f65d0e to afa4dc5 Compare August 19, 2025 13:05
Base automatically changed from create-next-base-controller-export to main August 19, 2025 13:11
@Gudahtt Gudahtt force-pushed the messenger-migrate-base-controller branch 4 times, most recently from 6d9f187 to 513bb4b Compare August 19, 2025 16:18
@Gudahtt Gudahtt marked this pull request as ready for review August 19, 2025 16:25
@Gudahtt Gudahtt requested a review from a team as a code owner August 19, 2025 16:25
/**
* The controller messenger. This is used to interact with other parts of the application.
*/
protected messagingSystem: ControllerMessenger;
Copy link
Member Author

Choose a reason for hiding this comment

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

We agreed recently in Slack to rename this property, but this will be done in a later PR.

* This is the same as the `messagingSystem` property, but has a type that only lets us use
* actions and events that are part of the `BaseController` class.
*/
readonly #messenger: Messenger<
Copy link
Member Author

Choose a reason for hiding this comment

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

I added a duplicate reference to the messenger here as a private instance variable just to avoid the need to cast it each time it's internally referenced.

}
}

it('should allow messaging between controllers', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test has the best complete example so far

cursor[bot]

This comment was marked as outdated.

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! Great to finally see this delegation in action, at least in the tests 🚀

@Gudahtt Gudahtt changed the title Migrate next BaseController class to new Messenger package feat: Migrate next BaseController class to new Messenger package Aug 19, 2025
@Gudahtt Gudahtt force-pushed the messenger-migrate-base-controller branch from 513bb4b to ae71891 Compare August 20, 2025 12:12
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!

@Gudahtt Gudahtt force-pushed the messenger-migrate-base-controller branch from ae71891 to 67c7853 Compare August 20, 2025 14:04
@Gudahtt Gudahtt enabled auto-merge (squash) August 20, 2025 14:06
@Gudahtt Gudahtt merged commit 2b1cd45 into main Aug 20, 2025
227 checks passed
@Gudahtt Gudahtt deleted the messenger-migrate-base-controller branch August 20, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants