core
core copied to clipboard
[composable-controller] Non-controllers with empty state should be rejected or excluded from composed state
Explanation
Some of our classes that were previously labeled as controllers have empty state, making them non-controllers.
As part of Wallet Framework team's Q2 2024 OKRs (O3KR1), we're currently in the process of upgrading these non-controllers to use the messenger pattern without inheriting from BaseController.
As downstream usage of the ComposableController includes passing in these non-controllers as child controllers, we should define a consistent behavior for handling non-controller input.
- a) We could simply reject non-controllers both at the type-level and at runtime, or
- b) we could accept non-controllers as child controllers but handle them so that the
ComposableControllerinstance doesn't subscribe to their non-existentstateChangeevents (i.e. The following message should not be logged to console for non-controllers: "Error: Event missing from allow list: ExampleController:stateChange"), and- b-1) exclude the non-controllers from the composed state object entirely, or
- b-2) include the non-controllers as properties in the composed state object, with empty objects assigned to the names of the non-controllers.
Proof of concept
- https://github.com/MetaMask/metamask-mobile/pull/10374/commits/a9204a54069c439853b525539541506dc4755111
- https://github.com/MetaMask/metamask-mobile/pull/10374#discussion_r1700941985
- https://github.com/MetaMask/metamask-mobile/pull/10374/commits/6ba9da2278532539ee2005cd55696d7f0f1947e8
- https://github.com/MetaMask/metamask-mobile/pull/10374#discussion_r1701131210
The following logic could be moved into the ComposableController constructor:
new ComposableController<
Exclude<EngineState, keyof NonControllers>,
Controllers[Exclude<keyof Controllers, keyof NonControllers>]
>({
controllers: controllers.filter(
(
controller,
): controller is Controllers[Exclude<
keyof Controllers,
keyof NonControllers
>] =>
'state' in controller &&
controller.state !== undefined &&
Object.keys(controller.state).length > 0,
),
messenger: ...
});
References
- See https://github.com/MetaMask/core/issues/4432