diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index 32709313ae..d443ba6a74 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -10,13 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add and export functions `isBaseControllerV1` and `isBaseController`, which are type guards for validating controller instances ([#3904](https://github.com/MetaMask/core/pull/3904)) -- Add and export types `BaseControllerV1Instance`, `BaseControllerV2Instance`, `ControllerInstance` which are the narrowest supertypes for all controllers extending from, respectively, `BaseControllerV1`, `BaseController`, and both ([#3904](https://github.com/MetaMask/core/pull/3904)) ### Changed - **BREAKING:** Passing a non-controller into `controllers` constructor option now throws an error ([#3904](https://github.com/MetaMask/core/pull/3904)) - **BREAKING:** The `AllowedActions` parameter of the `ComposableControllerMessenger` type is narrowed from `string` to `never`, as `ComposableController` does not use any external controller actions. ([#3904](https://github.com/MetaMask/core/pull/3904)) -- Add `@metamask/utils` ^8.3.0 as a dependency. ([#3904](https://github.com/MetaMask/core/pull/3904)) +- Subscribe to the `stateChange` events of `BaseControllerV1` controllers that have a `messagingSystem` class field with an assigned instance of the `RestrictedControllerMessenger` class. ([#3964](https://github.com/MetaMask/core/pull/3964)) ### Removed diff --git a/packages/composable-controller/package.json b/packages/composable-controller/package.json index 6a1771ab08..f794a631df 100644 --- a/packages/composable-controller/package.json +++ b/packages/composable-controller/package.json @@ -31,8 +31,7 @@ "test:watch": "jest --watch" }, "dependencies": { - "@metamask/base-controller": "^4.1.1", - "@metamask/utils": "^8.3.0" + "@metamask/base-controller": "^4.1.1" }, "devDependencies": { "@metamask/auto-changelog": "^3.4.4", diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 2072156c85..8ee6bbe3a5 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -1,51 +1,57 @@ import { BaseController, BaseControllerV1 } from '@metamask/base-controller'; import type { - RestrictedControllerMessenger, - BaseState, + ActionConstraint, BaseConfig, - StateMetadata, + BaseState, + EventConstraint, + RestrictedControllerMessenger, StateConstraint, } from '@metamask/base-controller'; -import { isValidJson } from '@metamask/utils'; import type { Patch } from 'immer'; export const controllerName = 'ComposableController'; -// TODO: Remove this type once `BaseControllerV2` migrations are completed for all controllers. /** - * A type encompassing all controller instances that extend from `BaseControllerV1`. + * A universal subtype of all controller instances that extend from `BaseControllerV1`. + * Any `BaseControllerV1` instance can be assigned to this type. + * + * Note that this type is not the greatest 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 to include all `BaseControllerV1` instances. + // `any` is used so that all `BaseControllerV1` instances are assignable to this type. // eslint-disable-next-line @typescript-eslint/no-explicit-any BaseControllerV1; /** - * A type encompassing all controller instances that extend from `BaseController` (formerly `BaseControllerV2`). + * A universal subtype of all controller instances that extend from `BaseController` (formerly `BaseControllerV2`). + * Any `BaseController` instance can be assigned to this type. * - * The `BaseController` class itself can't be used directly as a type representing all of its subclasses, - * because the generic parameters it expects require knowing the exact shape of the controller's state and messenger. + * Note that this type is not the greatest subtype or narrowest supertype of all `BaseController` instances. + * This type is therefore unsuitable for general use as a type constraint, and is only intended for use within the ComposableController. * - * Instead, we look for an object with the `BaseController` properties that we use in the ComposableController (name and state). + * For this reason, we only look for `BaseController` properties that we use in the ComposableController (name and state). */ -export type BaseControllerV2Instance = { +export type BaseControllerInstance = { name: string; state: StateConstraint; }; -// TODO: Remove `BaseControllerV1Instance` member once `BaseControllerV2` migrations are completed for all controllers. /** - * A type encompassing all controller instances that extend from `BaseControllerV1` or `BaseController`. + * A universal subtype of all controller instances that extend from `BaseController` (formerly `BaseControllerV2`) or `BaseControllerV1`. + * Any `BaseController` or `BaseControllerV1` instance can be assigned to this type. + * + * Note that this type is not the greatest 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 = | BaseControllerV1Instance - | BaseControllerV2Instance; + | BaseControllerInstance; /** - * Determines if the given controller is an instance of BaseControllerV1 + * Determines if the given controller is an instance of `BaseControllerV1` * @param controller - Controller instance to check - * @returns True if the controller is an instance of BaseControllerV1 - * TODO: Deprecate once `BaseControllerV2` migrations are completed for all controllers. + * @returns True if the controller is an instance of `BaseControllerV1` */ export function isBaseControllerV1( controller: ControllerInstance, @@ -67,13 +73,23 @@ export function isBaseControllerV1( } /** - * Determines if the given controller is an instance of BaseController + * Determines if the given controller is an instance of `BaseController` * @param controller - Controller instance to check - * @returns True if the controller is an instance of BaseController + * @returns True if the controller is an instance of `BaseController` */ export function isBaseController( controller: ControllerInstance, -): controller is BaseController { +): controller is BaseController< + string, + StateConstraint, + RestrictedControllerMessenger< + string, + ActionConstraint, + EventConstraint, + string, + string + > +> { return ( 'name' in controller && typeof controller.name === 'string' && @@ -83,10 +99,10 @@ export function isBaseController( ); } +// TODO: Replace `any` with `Json` once `BaseControllerV2` migrations are completed for all controllers. export type ComposableControllerState = { // `any` is used here to disable the `BaseController` type constraint which expects state properties to extend `Record`. // `ComposableController` state needs to accommodate `BaseControllerV1` state objects that may have properties wider than `Json`. - // TODO: Replace `any` with `Json` once `BaseControllerV2` migrations are completed for all controllers. // eslint-disable-next-line @typescript-eslint/no-explicit-any [name: string]: Record; }; @@ -142,7 +158,7 @@ export class ComposableController extends BaseController< super({ name: controllerName, - metadata: controllers.reduce>( + metadata: controllers.reduce( (metadata, controller) => ({ ...metadata, [controller.name]: isBaseController(controller) @@ -151,12 +167,9 @@ export class ComposableController extends BaseController< }), {}, ), - state: controllers.reduce( - (state, controller) => { - return { ...state, [controller.name]: controller.state }; - }, - {}, - ), + state: controllers.reduce((state, controller) => { + return { ...state, [controller.name]: controller.state }; + }, {}), messenger, }); @@ -168,33 +181,33 @@ export class ComposableController extends BaseController< /** * Constructor helper that subscribes to child controller state changes. * @param controller - Controller instance to update - * TODO: Remove `isBaseControllerV1` branch once `BaseControllerV2` migrations are completed for all controllers. */ #updateChildController(controller: ControllerInstance): void { + if (!isBaseController(controller) && !isBaseControllerV1(controller)) { + throw new Error( + 'Invalid controller: controller must extend from BaseController or BaseControllerV1', + ); + } + const { name } = controller; - if (isBaseControllerV1(controller)) { - controller.subscribe((childState) => { - this.update((state) => ({ - ...state, - [name]: childState, - })); - }); - } else if (isBaseController(controller)) { + if ( + (isBaseControllerV1(controller) && 'messagingSystem' in controller) || + isBaseController(controller) + ) { this.messagingSystem.subscribe( `${name}:stateChange`, - (childState: StateConstraint) => { - if (isValidJson(childState)) { - this.update((state) => ({ - ...state, - [name]: childState, - })); - } + (childState: Record) => { + this.update((state) => { + Object.assign(state, { [name]: childState }); + }); }, ); - } else { - throw new Error( - 'Invalid controller: controller must extend from BaseController or BaseControllerV1', - ); + } else if (isBaseControllerV1(controller)) { + controller.subscribe((childState) => { + this.update((state) => { + Object.assign(state, { [name]: childState }); + }); + }); } } } diff --git a/packages/composable-controller/src/index.ts b/packages/composable-controller/src/index.ts index c803f27d44..9deb15385b 100644 --- a/packages/composable-controller/src/index.ts +++ b/packages/composable-controller/src/index.ts @@ -1,7 +1,4 @@ export type { - BaseControllerV1Instance, - BaseControllerV2Instance, - ControllerInstance, ComposableControllerState, ComposableControllerStateChangeEvent, ComposableControllerEvents, diff --git a/yarn.lock b/yarn.lock index 92d5d99df0..ca9da0bb27 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1712,7 +1712,6 @@ __metadata: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^4.1.1 "@metamask/json-rpc-engine": ^7.3.3 - "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 deepmerge: ^4.2.2 immer: ^9.0.6