diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index 1967d59f687..6a79b27f936 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -144,6 +144,21 @@ class BazController extends BaseControllerV1 { } } +type ControllersMap = { + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/naming-convention + FooController: FooController; + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/naming-convention + QuzController: QuzController; + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/naming-convention + BarController: BarController; + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/naming-convention + BazController: BazController; +}; + describe('ComposableController', () => { afterEach(() => { sinon.restore(); @@ -159,6 +174,7 @@ describe('ComposableController', () => { // eslint-disable-next-line @typescript-eslint/naming-convention BazController: BazControllerState; }; + const composableMessenger = new ControllerMessenger< never, ComposableControllerEvents @@ -167,7 +183,10 @@ describe('ComposableController', () => { allowedActions: [], allowedEvents: [], }); - const controller = new ComposableController({ + const controller = new ComposableController< + ComposableControllerState, + ControllersMap[keyof ComposableControllerState] + >({ controllers: [new BarController(), new BazController()], messenger: composableMessenger, }); @@ -194,7 +213,10 @@ describe('ComposableController', () => { allowedEvents: [], }); const barController = new BarController(); - new ComposableController({ + new ComposableController< + ComposableControllerState, + ControllersMap[keyof ComposableControllerState] + >({ controllers: [barController], messenger: composableMessenger, }); @@ -255,11 +277,13 @@ describe('ComposableController', () => { 'QuzController:stateChange', ], }); - const composableController = - new ComposableController({ - controllers: [fooController, quzController], - messenger: composableControllerMessenger, - }); + const composableController = new ComposableController< + ComposableControllerState, + ControllersMap[keyof ComposableControllerState] + >({ + controllers: [fooController, quzController], + messenger: composableControllerMessenger, + }); expect(composableController.state).toStrictEqual({ FooController: { foo: 'foo' }, QuzController: { quz: 'quz' }, @@ -288,7 +312,10 @@ describe('ComposableController', () => { allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController< + ComposableControllerState, + ControllersMap[keyof ComposableControllerState] + >({ controllers: [fooController], messenger: composableControllerMessenger, }); @@ -336,11 +363,13 @@ describe('ComposableController', () => { allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - const composableController = - new ComposableController({ - controllers: [barController, fooController], - messenger: composableControllerMessenger, - }); + const composableController = new ComposableController< + ComposableControllerState, + ControllersMap[keyof ComposableControllerState] + >({ + controllers: [barController, fooController], + messenger: composableControllerMessenger, + }); expect(composableController.state).toStrictEqual({ BarController: { bar: 'bar' }, FooController: { foo: 'foo' }, @@ -373,7 +402,10 @@ describe('ComposableController', () => { allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController< + ComposableControllerState, + ControllersMap[keyof ComposableControllerState] + >({ controllers: [barController, fooController], messenger: composableControllerMessenger, }); @@ -421,7 +453,10 @@ describe('ComposableController', () => { allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController< + ComposableControllerState, + ControllersMap[keyof ComposableControllerState] + >({ controllers: [barController, fooController], messenger: composableControllerMessenger, }); @@ -490,13 +525,16 @@ describe('ComposableController', () => { }); expect( () => - new ComposableController({ + new ComposableController< + ComposableControllerState, + ControllersMap[keyof ComposableControllerState] + >({ // @ts-expect-error - Suppressing type error to test for runtime error handling controllers: [notController, fooController], messenger: composableControllerMessenger, }), ).toThrow( - 'Invalid controller: controller must extend from BaseController or BaseControllerV1', + 'Invalid component: component must be a MessengerConsumer or a controller inheriting from BaseControllerV1.', ); }); }); diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index e710efa32c4..bfc6ed2419d 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -1,4 +1,3 @@ -import { BaseController, BaseControllerV1 } from '@metamask/base-controller'; import type { ActionConstraint, BaseConfig, @@ -8,11 +7,18 @@ import type { StateConstraint, StateMetadata, ControllerStateChangeEvent, + Listener, } from '@metamask/base-controller'; +import { BaseController, BaseControllerV1 } from '@metamask/base-controller'; import type { Patch } from 'immer'; export const controllerName = 'ComposableController'; +type MessengerConsumerInstance = { + name: string; + messagingSystem: RestrictedControllerMessengerConstraint; +}; + /** * A universal subtype of all controller instances that extend from `BaseControllerV1`. * Any `BaseControllerV1` instance can be assigned to this type. @@ -20,10 +26,15 @@ export const controllerName = 'ComposableController'; * Note that this type is not the widest subtype or narrowest supertype of all `BaseControllerV1` instances. * This type is therefore unsuitable for general use as a type constraint, and is only intended for use within the ComposableController. */ -export type BaseControllerV1Instance = - // `any` is used so that all `BaseControllerV1` instances are assignable to this type. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - BaseControllerV1; +export type BaseControllerV1Instance = { + name: string; + config: BaseConfig & object; + defaultConfig: BaseConfig & object; + state: BaseState & object; + defaultState: BaseState & object; + subscribe: (listener: Listener) => void; + disabled: boolean; +} & Partial>; /** * A universal subtype of all controller instances that extend from `BaseController` (formerly `BaseControllerV2`). @@ -34,9 +45,12 @@ export type BaseControllerV1Instance = * * For this reason, we only look for `BaseController` properties that we use in the ComposableController (name and state). */ -export type BaseControllerInstance = { +export type BaseControllerInstance< + State extends StateConstraint = StateConstraint, +> = { name: string; - state: StateConstraint; + state: State; + metadata: Record; }; /** @@ -46,21 +60,39 @@ export type BaseControllerInstance = { * Note that this type is not the widest subtype or narrowest supertype of all `BaseController` and `BaseControllerV1` instances. * This type is therefore unsuitable for general use as a type constraint, and is only intended for use within the ComposableController. */ -export type ControllerInstance = +export type WalletComponentInstance = | BaseControllerV1Instance - | BaseControllerInstance; + | BaseControllerInstance + | MessengerConsumerInstance; + +export type ControllerInstance = Exclude< + WalletComponentInstance, + MessengerConsumerInstance +>; /** * The narrowest supertype of all `RestrictedControllerMessenger` instances. */ -export type RestrictedControllerMessengerConstraint = - RestrictedControllerMessenger< - string, - ActionConstraint, - EventConstraint, - string, - string - >; +export type RestrictedControllerMessengerConstraint< + ControllerName extends string = string, +> = RestrictedControllerMessenger< + ControllerName, + ActionConstraint, + EventConstraint, + string, + string +>; + +/** + * Determines if the given class has a messaging system. + * @param component - Component instance to check + * @returns True if the component is an instance of `MessengerConsumerInstance` + */ +export function isMessengerConsumer( + component: WalletComponentInstance, +): component is MessengerConsumerInstance { + return 'name' in component && 'messagingSystem' in component; +} /** * Determines if the given controller is an instance of `BaseControllerV1` @@ -68,20 +100,23 @@ export type RestrictedControllerMessengerConstraint = * @returns True if the controller is an instance of `BaseControllerV1` */ export function isBaseControllerV1( - controller: ControllerInstance, -): controller is BaseControllerV1< - BaseConfig & Record, - BaseState & Record -> { + controller: WalletComponentInstance, +): controller is BaseControllerV1Instance { return ( 'name' in controller && typeof controller.name === 'string' && + 'config' in controller && + typeof controller.config === 'object' && 'defaultConfig' in controller && typeof controller.defaultConfig === 'object' && + 'state' in controller && + typeof controller.state === 'object' && 'defaultState' in controller && typeof controller.defaultState === 'object' && 'disabled' in controller && typeof controller.disabled === 'boolean' && + 'subscribe' in controller && + typeof controller.subscribe === 'function' && controller instanceof BaseControllerV1 ); } @@ -92,17 +127,15 @@ export function isBaseControllerV1( * @returns True if the controller is an instance of `BaseController` */ export function isBaseController( - controller: ControllerInstance, -): controller is BaseController< - string, - StateConstraint, - RestrictedControllerMessengerConstraint -> { + controller: WalletComponentInstance, +): controller is BaseControllerInstance { return ( 'name' in controller && typeof controller.name === 'string' && 'state' in controller && typeof controller.state === 'object' && + 'metadata' in controller && + typeof controller.metadata === 'object' && controller instanceof BaseController ); } @@ -186,25 +219,13 @@ export type ComposableControllerMessenger< AllowedEvents['type'] >; -type GetChildControllers< - ComposableControllerState, - ControllerName extends keyof ComposableControllerState = keyof ComposableControllerState, -> = ControllerName extends string - ? ComposableControllerState[ControllerName] extends StateConstraint - ? { name: ControllerName; state: ComposableControllerState[ControllerName] } - : BaseControllerV1< - BaseConfig & Record, - BaseState & ComposableControllerState[ControllerName] - > - : never; - /** * Controller that can be used to compose multiple controllers together. * @template ChildControllerState - The composed state of the child controllers that are being used to instantiate the composable controller. */ export class ComposableController< ComposableControllerState extends LegacyComposableControllerStateConstraint, - ChildControllers extends ControllerInstance = GetChildControllers, + ChildControllers extends WalletComponentInstance, > extends BaseController< typeof controllerName, ComposableControllerState, @@ -242,7 +263,9 @@ export class ComposableController< ), state: controllers.reduce( (state, controller) => { - return { ...state, [controller.name]: controller.state }; + return 'state' in controller + ? { ...state, [controller.name]: controller.state } + : state; }, {} as never, ), @@ -258,34 +281,29 @@ export class ComposableController< * Constructor helper that subscribes to child controller state changes. * @param controller - Controller instance to update */ - #updateChildController(controller: ControllerInstance): void { - if (!isBaseController(controller) && !isBaseControllerV1(controller)) { - throw new Error( - 'Invalid controller: controller must extend from BaseController or BaseControllerV1', - ); - } - + #updateChildController(controller: WalletComponentInstance): void { const { name } = controller; - if ( - (isBaseControllerV1(controller) && 'messagingSystem' in controller) || - isBaseController(controller) - ) { + if (isBaseController(controller) || isMessengerConsumer(controller)) { this.messagingSystem.subscribe( // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions `${name}:stateChange`, - (childState: Record) => { + (childState: object) => { this.update((state) => { Object.assign(state, { [name]: childState }); }); }, ); } else if (isBaseControllerV1(controller)) { - controller.subscribe((childState) => { + controller.subscribe((childState: BaseState & object) => { this.update((state) => { Object.assign(state, { [name]: childState }); }); }); + } else { + throw new Error( + 'Invalid component: component must be a MessengerConsumer or a controller inheriting from BaseControllerV1.', + ); } } }