From 83b976b660ac9056455bc4a8c60d39da73afe514 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Sun, 3 Mar 2024 12:53:51 -0500 Subject: [PATCH 01/23] Establish API for generic controller class - Implements https://github.com/MetaMask/core/issues/3627 --- .../src/ComposableController.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 8ee6bbe3a50..b14812fa6b6 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -132,10 +132,26 @@ export type ComposableControllerMessenger = RestrictedControllerMessenger< /** * Controller that can be used to compose multiple controllers together. */ -export class ComposableController extends BaseController< +export class ComposableController< + ChildControllers extends ControllerInstance, + ComposedControllerState extends ComposableControllerState, + ComposedControllerStateChangeEvent extends EventConstraint & { + type: `${typeof controllerName}:stateChange`; + }, + ChildControllersStateChangeEvents extends EventConstraint & { + type: `${string}:stateChange`; + }, + ComposedControllerMessenger extends ComposableControllerMessenger = RestrictedControllerMessenger< + typeof controllerName, + never, + ComposedControllerStateChangeEvent | ChildControllersStateChangeEvents, + never, + ChildControllersStateChangeEvents['type'] + >, +> extends BaseController< typeof controllerName, - ComposableControllerState, - ComposableControllerMessenger + ComposedControllerState, + ComposedControllerMessenger > { /** * Creates a ComposableController instance. @@ -149,8 +165,8 @@ export class ComposableController extends BaseController< controllers, messenger, }: { - controllers: ControllerInstance[]; - messenger: ComposableControllerMessenger; + controllers: ChildControllers[]; + messenger: ComposedControllerMessenger; }) { if (messenger === undefined) { throw new Error(`Messaging system is required`); From 24de570228e54c27df3341d7aa12e7db00b77d68 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Sun, 3 Mar 2024 12:55:05 -0500 Subject: [PATCH 02/23] Fix types for `metadata`, `state` class fields --- .../src/ComposableController.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index b14812fa6b6..799ed87a14d 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -6,6 +6,7 @@ import type { EventConstraint, RestrictedControllerMessenger, StateConstraint, + StateMetadata, } from '@metamask/base-controller'; import type { Patch } from 'immer'; @@ -174,18 +175,21 @@ export class ComposableController< super({ name: controllerName, - metadata: controllers.reduce( + metadata: controllers.reduce>( (metadata, controller) => ({ ...metadata, [controller.name]: isBaseController(controller) ? controller.metadata : { persist: true, anonymous: true }, }), - {}, + {} as never, + ), + state: controllers.reduce( + (state, controller) => { + return { ...state, [controller.name]: controller.state }; + }, + {} as never, ), - state: controllers.reduce((state, controller) => { - return { ...state, [controller.name]: controller.state }; - }, {}), messenger, }); From 24247eadd1f526b9ab5d7c831e26f70de0469613 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Sun, 3 Mar 2024 13:00:18 -0500 Subject: [PATCH 03/23] Implement `ComposedControllerState` using mapped type and key remapping --- packages/composable-controller/src/ComposableController.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 799ed87a14d..0927eb363a6 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -135,7 +135,9 @@ export type ComposableControllerMessenger = RestrictedControllerMessenger< */ export class ComposableController< ChildControllers extends ControllerInstance, - ComposedControllerState extends ComposableControllerState, + ComposedControllerState extends ComposableControllerState = { + [P in ChildControllers as P['name']]: P['state']; + }, ComposedControllerStateChangeEvent extends EventConstraint & { type: `${typeof controllerName}:stateChange`; }, From f3dfd649d94fe6423ca81098d54d87a2d6a5c089 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Sun, 3 Mar 2024 13:01:25 -0500 Subject: [PATCH 04/23] Derive `ComposedControllerStateChangeEvent` using `ControllerStateChangeEvent` and `ComposedControllerState` --- packages/composable-controller/src/ComposableController.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 0927eb363a6..bfaa48c5cd1 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -7,6 +7,7 @@ import type { RestrictedControllerMessenger, StateConstraint, StateMetadata, + ControllerStateChangeEvent, } from '@metamask/base-controller'; import type { Patch } from 'immer'; @@ -140,7 +141,10 @@ export class ComposableController< }, ComposedControllerStateChangeEvent extends EventConstraint & { type: `${typeof controllerName}:stateChange`; - }, + } = ControllerStateChangeEvent< + typeof controllerName, + ComposedControllerState + >, ChildControllersStateChangeEvents extends EventConstraint & { type: `${string}:stateChange`; }, From 340678e5777cc1096bcc3bcb1cbbe17e3da0b189 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Sun, 3 Mar 2024 13:02:28 -0500 Subject: [PATCH 05/23] Derive `ChildControllersStateChangeEvents` using distributive property of generic conditional types --- .../composable-controller/src/ComposableController.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index bfaa48c5cd1..6ce8a199846 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -131,6 +131,14 @@ export type ComposableControllerMessenger = RestrictedControllerMessenger< AllowedEvents['type'] >; +type GetStateChangeEvents = + Controller extends BaseControllerV1Instance + ? { + type: `${Controller['name']}:stateChange`; + payload: [Controller['state'], Patch[]]; + } + : ControllerStateChangeEvent; + /** * Controller that can be used to compose multiple controllers together. */ @@ -147,7 +155,7 @@ export class ComposableController< >, ChildControllersStateChangeEvents extends EventConstraint & { type: `${string}:stateChange`; - }, + } = GetStateChangeEvents, ComposedControllerMessenger extends ComposableControllerMessenger = RestrictedControllerMessenger< typeof controllerName, never, From ec26f6f1eb4c875bde2fd90779a8b67ab45a45b3 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Sun, 3 Mar 2024 13:02:45 -0500 Subject: [PATCH 06/23] Update changelog --- packages/composable-controller/CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index eb07c36f282..3c3cf34232b 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -25,10 +25,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **BREAKING:** Passing a non-controller into `controllers` constructor option now throws an error ([#3904](https://github.com/MetaMask/core/pull/3904)) -- **BREAKING:** The `AllowedAction` 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)) +- **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)) +- **BREAKING:** The `ComposableController` class is now a generic class that expects one generic argument `ChildControllers` in the form of a union of the controllers passed into the `controllers` array constructor option ([#3941](https://github.com/MetaMask/core/pull/3941)) + - Child controllers that extend `BaseControllerV1` must have an overridden `name` property that is defined using the `as const` assertion for the `ComposableController` class to be typed correctly. +- Relax `payload` in `ComposableControllerStateChangeEvent` to use `Record` rather than `Record` ([#3949](https://github.com/MetaMask/core/pull/3949)) - **BREAKING:** Bump `@metamask/base-controller` to `^5.0.0` ([#4039](https://github.com/MetaMask/core/pull/4039)) - This version has a number of breaking changes. See the changelog for more. -- Relax `payload` in `ComposableControllerStateChangeEvent` to use `Record` rather than `Record` ([#3949](https://github.com/MetaMask/core/pull/3949)) - Bump `@metamask/json-rpc-engine` to `^8.0.0` ([#4039](https://github.com/MetaMask/core/pull/4039)) ### Removed From 5c7771b093b4eed406607e3b58f7b2e930067621 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Mar 2024 14:31:53 -0400 Subject: [PATCH 07/23] Add const assertions to `name` class field of all V1 controllers --- packages/address-book-controller/src/AddressBookController.ts | 2 +- packages/assets-controllers/src/AssetsContractController.ts | 2 +- packages/assets-controllers/src/TokensController.ts | 2 +- .../composable-controller/src/ComposableController.test.ts | 4 ++-- packages/message-manager/src/AbstractMessageManager.ts | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/address-book-controller/src/AddressBookController.ts b/packages/address-book-controller/src/AddressBookController.ts index 1101dffc94a..cd96f5355c1 100644 --- a/packages/address-book-controller/src/AddressBookController.ts +++ b/packages/address-book-controller/src/AddressBookController.ts @@ -78,7 +78,7 @@ export class AddressBookController extends BaseControllerV1< /** * Name of this controller used during composition */ - override name = 'AddressBookController'; + override name = 'AddressBookController' as const; /** * Creates an AddressBookController instance. diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 71cad853906..b9dd83602a6 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -107,7 +107,7 @@ export class AssetsContractController extends BaseControllerV1< /** * Name of this controller used during composition */ - override name = 'AssetsContractController'; + override name = 'AssetsContractController' as const; private readonly getNetworkClientById: NetworkController['getNetworkClientById']; diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index a96de9cd6c4..fff4a4814be 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -211,7 +211,7 @@ export class TokensController extends BaseControllerV1< /** * Name of this controller used during composition */ - override name = 'TokensController'; + override name = 'TokensController' as const; /** * Creates a TokensController instance. diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index ca5df665efd..0e483863fff 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -71,7 +71,7 @@ class BarController extends BaseControllerV1 { bar: 'bar', }; - override name = 'BarController'; + override name = 'BarController' as const; constructor() { super(); @@ -92,7 +92,7 @@ class BazController extends BaseControllerV1 { baz: 'baz', }; - override name = 'BazController'; + override name = 'BazController' as const; constructor() { super(); diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 849d9799f61..af24834fa49 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -200,7 +200,7 @@ export abstract class AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'AbstractMessageManager'; + override name = 'AbstractMessageManager' as const; /** * Creates an AbstractMessageManager instance. From 2457e2ad48bbf4cb1559756eec29126c758f9e7d Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Mar 2024 14:32:34 -0400 Subject: [PATCH 08/23] test: Add new mock controller to test composing two V2 controllers --- .../src/ComposableController.test.ts | 74 +++++++++++++++++-- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index 0e483863fff..d0696e10a07 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -27,9 +27,9 @@ type FooControllerEvent = { type FooMessenger = RestrictedControllerMessenger< 'FooController', never, - FooControllerEvent, + FooControllerEvent | QuzControllerEvent, never, - never + QuzControllerEvent['type'] >; const fooControllerStateMetadata = { @@ -60,6 +60,50 @@ class FooController extends BaseController< } } +type QuzControllerState = { + quz: string; +}; +type QuzControllerEvent = { + type: `QuzController:stateChange`; + payload: [QuzControllerState, Patch[]]; +}; + +type QuzMessenger = RestrictedControllerMessenger< + 'QuzController', + never, + QuzControllerEvent, + never, + never +>; + +const quzControllerStateMetadata = { + quz: { + persist: true, + anonymous: true, + }, +}; + +class QuzController extends BaseController< + 'QuzController', + QuzControllerState, + QuzMessenger +> { + constructor(messagingSystem: QuzMessenger) { + super({ + messenger: messagingSystem, + metadata: quzControllerStateMetadata, + name: 'QuzController', + state: { quz: 'quz' }, + }); + } + + updateQuz(quz: string) { + super.update((state) => { + state.quz = quz; + }); + } +} + // Mock BaseControllerV1 classes type BarControllerState = BaseState & { @@ -161,30 +205,44 @@ describe('ComposableController', () => { it('should compose controller state', () => { const controllerMessenger = new ControllerMessenger< never, - FooControllerEvent + FooControllerEvent | QuzControllerEvent >(); - const fooControllerMessenger = controllerMessenger.getRestricted({ + const fooMessenger = controllerMessenger.getRestricted< + 'FooController', + never, + QuzControllerEvent['type'] + >({ name: 'FooController', allowedActions: [], + allowedEvents: ['QuzController:stateChange'], + }); + const quzMessenger = controllerMessenger.getRestricted({ + name: 'QuzController', + allowedActions: [], allowedEvents: [], }); - const fooController = new FooController(fooControllerMessenger); + const fooController = new FooController(fooMessenger); + const quzController = new QuzController(quzMessenger); const composableControllerMessenger = controllerMessenger.getRestricted< 'ComposableController', never, - FooControllerEvent['type'] + (FooControllerEvent | QuzControllerEvent)['type'] >({ name: 'ComposableController', allowedActions: [], - allowedEvents: ['FooController:stateChange'], + allowedEvents: [ + 'FooController:stateChange', + 'QuzController:stateChange', + ], }); const composableController = new ComposableController({ - controllers: [fooController], + controllers: [fooController, quzController], messenger: composableControllerMessenger, }); expect(composableController.state).toStrictEqual({ FooController: { foo: 'foo' }, + QuzController: { quz: 'quz' }, }); }); From a99a6d35facd04bd4cdd4f7b1f9658daf1156cae Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 17 Apr 2024 17:10:08 -0400 Subject: [PATCH 09/23] Clarify usage of "widest subtype" in jsdoc --- packages/composable-controller/src/ComposableController.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 6ce8a199846..fad3b01aca1 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -17,7 +17,7 @@ export const controllerName = 'ComposableController'; * 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. + * 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 = @@ -29,7 +29,7 @@ export type BaseControllerV1Instance = * A universal subtype of all controller instances that extend from `BaseController` (formerly `BaseControllerV2`). * Any `BaseController` instance can be assigned to this type. * - * Note that this type is not the greatest subtype or narrowest supertype of all `BaseController` instances. + * Note that this type is not the widest 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. * * For this reason, we only look for `BaseController` properties that we use in the ComposableController (name and state). @@ -43,7 +43,7 @@ export type BaseControllerInstance = { * 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. + * 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 = From 33d93e08fc6dad45029d2920edf3bd7cc29f2e9c Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 17 Apr 2024 17:10:31 -0400 Subject: [PATCH 10/23] Define narrowest supertype of `RestrictedControllerMessenger` --- packages/composable-controller/CHANGELOG.md | 1 + .../src/ComposableController.ts | 20 ++++++++++++------- packages/composable-controller/src/index.ts | 1 + 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index 3c3cf34232b..59f40a44a6c 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING**: Add ESM build ([#3998](https://github.com/MetaMask/core/pull/3998)) - It's no longer possible to import files from `./dist` directly. - Add and export functions `isBaseControllerV1` and `isBaseController`, which are type guards for validating controller instances ([#3904](https://github.com/MetaMask/core/pull/3904)) +- Add the `RestrictedControllerMessengerConstraint` type, which is the narrowest supertype of all controller-messenger instances ([#3904](https://github.com/MetaMask/core/pull/3904)) - `ComposableController` now accommodates `BaseControllerV1` controllers that use a messenger (specifically, which have a `messagingSystem` property which is an instance of `RestrictedControllerMessenger`), by subscribing to the `stateChange` event of the messenger instead of using the `subscribe` method on the controller ([#3964](https://github.com/MetaMask/core/pull/3964)) ### Changed diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index fad3b01aca1..473aa348d25 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -50,6 +50,18 @@ export type ControllerInstance = | BaseControllerV1Instance | BaseControllerInstance; +/** + * The narrowest supertype of all `RestrictedControllerMessenger` instances. + */ +export type RestrictedControllerMessengerConstraint = + RestrictedControllerMessenger< + string, + ActionConstraint, + EventConstraint, + string, + string + >; + /** * Determines if the given controller is an instance of `BaseControllerV1` * @param controller - Controller instance to check @@ -84,13 +96,7 @@ export function isBaseController( ): controller is BaseController< string, StateConstraint, - RestrictedControllerMessenger< - string, - ActionConstraint, - EventConstraint, - string, - string - > + RestrictedControllerMessengerConstraint > { return ( 'name' in controller && diff --git a/packages/composable-controller/src/index.ts b/packages/composable-controller/src/index.ts index 9deb15385bb..3f6b8535a57 100644 --- a/packages/composable-controller/src/index.ts +++ b/packages/composable-controller/src/index.ts @@ -3,6 +3,7 @@ export type { ComposableControllerStateChangeEvent, ComposableControllerEvents, ComposableControllerMessenger, + RestrictedControllerMessengerConstraint, } from './ComposableController'; export { ComposableController, From fc91ff876e974c5ff82b05e799273beb1deae12f Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 17 Apr 2024 17:10:56 -0400 Subject: [PATCH 11/23] Define a type for state-change events for both v1 and v2 controllers --- .../src/ComposableController.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 473aa348d25..2aec15c35f0 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -107,6 +107,20 @@ export function isBaseController( ); } +/** + * A controller state change event for any controller instance that extends from either `BaseControllerV1` or `BaseControllerV2`. + */ +// TODO: Replace all instances with `ControllerStateChangeEvent` once `BaseControllerV2` migrations are completed for all controllers. +type LegacyControllerStateChangeEvent< + ControllerName extends string, + ControllerState extends + | (BaseState & Record) + | StateConstraint, +> = { + type: `${ControllerName}:stateChange`; + payload: [ControllerState, Patch[]]; +}; + // 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`. From 4b8f6c738b90bd27c41179d27422c73741c14228 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 17 Apr 2024 17:11:51 -0400 Subject: [PATCH 12/23] Rename narrowest supertype for composable controller state to use "Constraint" suffix --- packages/composable-controller/src/ComposableController.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 2aec15c35f0..e509818fbc0 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -121,8 +121,11 @@ type LegacyControllerStateChangeEvent< payload: [ControllerState, Patch[]]; }; +/** + * A narrowest supertype for the composable controller state object. + */ // TODO: Replace `any` with `Json` once `BaseControllerV2` migrations are completed for all controllers. -export type ComposableControllerState = { +export type ComposableControllerStateConstraint = { // `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`. // eslint-disable-next-line @typescript-eslint/no-explicit-any From 186d637bfaaa8c7db3e8d8779faf88c9579234dc Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 11:51:24 -0400 Subject: [PATCH 13/23] Remove references to `ComposedControllerMessenger` and `ComposedControllerState` --- .../composable-controller/src/ComposableController.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index e509818fbc0..8f1f781b267 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -188,8 +188,8 @@ export class ComposableController< >, > extends BaseController< typeof controllerName, - ComposedControllerState, - ComposedControllerMessenger + ComposableControllerState, + ComposableControllerMessenger > { /** * Creates a ComposableController instance. @@ -204,7 +204,7 @@ export class ComposableController< messenger, }: { controllers: ChildControllers[]; - messenger: ComposedControllerMessenger; + messenger: ComposableControllerMessenger; }) { if (messenger === undefined) { throw new Error(`Messaging system is required`); @@ -212,7 +212,7 @@ export class ComposableController< super({ name: controllerName, - metadata: controllers.reduce>( + metadata: controllers.reduce>( (metadata, controller) => ({ ...metadata, [controller.name]: isBaseController(controller) @@ -221,7 +221,7 @@ export class ComposableController< }), {} as never, ), - state: controllers.reduce( + state: controllers.reduce( (state, controller) => { return { ...state, [controller.name]: controller.state }; }, From 46228aaa424ef1767af4e02b8e5d1a93ec64df60 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 11:53:36 -0400 Subject: [PATCH 14/23] Define state constraint types --- .../src/ComposableController.ts | 54 ++++++++----------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 8f1f781b267..cb315e65393 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -107,36 +107,47 @@ export function isBaseController( ); } +/** + * A universal supertype for the controller state object, encompassing both `BaseControllerV1` and `BaseControllerV2` state. + */ +export type LegacyControllerStateConstraint = BaseState | StateConstraint; + /** * A controller state change event for any controller instance that extends from either `BaseControllerV1` or `BaseControllerV2`. */ // TODO: Replace all instances with `ControllerStateChangeEvent` once `BaseControllerV2` migrations are completed for all controllers. type LegacyControllerStateChangeEvent< ControllerName extends string, - ControllerState extends - | (BaseState & Record) - | StateConstraint, + ControllerState extends LegacyControllerStateConstraint, > = { type: `${ControllerName}:stateChange`; payload: [ControllerState, Patch[]]; }; /** - * A narrowest supertype for the composable controller state object. + * A universal supertype for the composable controller state object. + * + * This type is only intended to be used for disabling the generic constraint on the `ControllerState` type argument in the `BaseController` type as a temporary solution for ensuring compatibility with BaseControllerV1 child controllers. + * Note that it is unsuitable for general use as a type constraint. */ -// TODO: Replace `any` with `Json` once `BaseControllerV2` migrations are completed for all controllers. -export type ComposableControllerStateConstraint = { - // `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 with `ComposableControllerStateConstraint` once BaseControllerV2 migrations are completed for all controllers. +type LegacyComposableControllerStateConstraint = { + // `any` is used here to disable the generic constraint on the `ControllerState` type argument in the `BaseController` type, + // enabling composable controller state types with BaseControllerV1 state objects to be. // eslint-disable-next-line @typescript-eslint/no-explicit-any [name: string]: Record; }; -export type ComposableControllerStateChangeEvent = { - type: `${typeof controllerName}:stateChange`; - payload: [ComposableControllerState, Patch[]]; +/** + * The narrowest supertype for the composable controller state object. + * This is also a widest subtype of the 'LegacyComposableControllerStateConstraint' type. + */ +// TODO: Replace with `{ [name: string]: StateConstraint }` once BaseControllerV2 migrations are completed for all controllers. +export type ComposableControllerStateConstraint = { + [name: string]: LegacyControllerStateConstraint; }; + export type ComposableControllerEvents = ComposableControllerStateChangeEvent; type AnyControllerStateChangeEvent = { @@ -166,26 +177,7 @@ type GetStateChangeEvents = * Controller that can be used to compose multiple controllers together. */ export class ComposableController< - ChildControllers extends ControllerInstance, - ComposedControllerState extends ComposableControllerState = { - [P in ChildControllers as P['name']]: P['state']; - }, - ComposedControllerStateChangeEvent extends EventConstraint & { - type: `${typeof controllerName}:stateChange`; - } = ControllerStateChangeEvent< - typeof controllerName, - ComposedControllerState - >, - ChildControllersStateChangeEvents extends EventConstraint & { - type: `${string}:stateChange`; - } = GetStateChangeEvents, - ComposedControllerMessenger extends ComposableControllerMessenger = RestrictedControllerMessenger< - typeof controllerName, - never, - ComposedControllerStateChangeEvent | ChildControllersStateChangeEvents, - never, - ChildControllersStateChangeEvents['type'] - >, + ComposableControllerState extends LegacyComposableControllerStateConstraint, > extends BaseController< typeof controllerName, ComposableControllerState, From 5248fcf201c4b8156cf37d05826afe4f6053eb9b Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 11:54:18 -0400 Subject: [PATCH 15/23] Define `stateChange` events and controller-messenger types --- .../src/ComposableController.ts | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index cb315e65393..b1000621d7a 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -147,22 +147,55 @@ export type ComposableControllerStateConstraint = { [name: string]: LegacyControllerStateConstraint; }; +/** + * A controller state change event for any controller instance that extends from either `BaseControllerV1` or `BaseControllerV2`. + */ +// TODO: Replace all instances with `ControllerStateChangeEvent` once `BaseControllerV2` migrations are completed for all controllers. +type LegacyControllerStateChangeEvent< + ControllerName extends string, + ControllerState extends LegacyControllerStateConstraint, +> = { + type: `${ControllerName}:stateChange`; + payload: [ControllerState, Patch[]]; +}; -export type ComposableControllerEvents = ComposableControllerStateChangeEvent; +export type ComposableControllerStateChangeEvent< + ComposableControllerState extends ComposableControllerStateConstraint, +> = LegacyControllerStateChangeEvent< + typeof controllerName, + ComposableControllerState +>; -type AnyControllerStateChangeEvent = { - type: `${string}:stateChange`; - payload: [ControllerInstance['state'], Patch[]]; -}; +export type ComposableControllerEvents< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ComposableControllerStateChangeEvent; + +type ChildControllerStateChangeEvents< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ComposableControllerState extends Record< + infer ControllerName extends string, + infer ControllerState +> + ? ControllerState extends StateConstraint + ? ControllerStateChangeEvent + : ControllerState extends Record + ? LegacyControllerStateChangeEvent + : never + : never; -type AllowedEvents = AnyControllerStateChangeEvent; +type AllowedEvents< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ChildControllerStateChangeEvents; -export type ComposableControllerMessenger = RestrictedControllerMessenger< +export type ComposableControllerMessenger< + ComposableControllerState extends ComposableControllerStateConstraint, +> = RestrictedControllerMessenger< typeof controllerName, never, - ComposableControllerEvents | AllowedEvents, + | ComposableControllerEvents + | AllowedEvents, never, - AllowedEvents['type'] + AllowedEvents['type'] >; type GetStateChangeEvents = From cd9294bf29361d20a7ad9787a10f8a043e06bc9d Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 11:54:32 -0400 Subject: [PATCH 16/23] Define type for child controllers --- .../src/ComposableController.ts | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index b1000621d7a..b78857bae12 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -198,19 +198,25 @@ export type ComposableControllerMessenger< AllowedEvents['type'] >; -type GetStateChangeEvents = - Controller extends BaseControllerV1Instance - ? { - type: `${Controller['name']}:stateChange`; - payload: [Controller['state'], Patch[]]; - } - : ControllerStateChangeEvent; +export type GetChildControllers< + ComposableControllerState, + P extends keyof ComposableControllerState = keyof ComposableControllerState, +> = P extends string + ? ComposableControllerState[P] extends StateConstraint + ? { name: P; state: ComposableControllerState[P] } + : BaseControllerV1< + BaseConfig & Record, + BaseState & ComposableControllerState[P] + > + : 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, > extends BaseController< typeof controllerName, ComposableControllerState, From 7f2b0a0dcc8364f815579ed6c0a1d1fb6ccb3451 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 11:58:18 -0400 Subject: [PATCH 17/23] Remove duplicate from merge conflict --- .../src/ComposableController.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index b78857bae12..d3c12e280a6 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -112,18 +112,6 @@ export function isBaseController( */ export type LegacyControllerStateConstraint = BaseState | StateConstraint; -/** - * A controller state change event for any controller instance that extends from either `BaseControllerV1` or `BaseControllerV2`. - */ -// TODO: Replace all instances with `ControllerStateChangeEvent` once `BaseControllerV2` migrations are completed for all controllers. -type LegacyControllerStateChangeEvent< - ControllerName extends string, - ControllerState extends LegacyControllerStateConstraint, -> = { - type: `${ControllerName}:stateChange`; - payload: [ControllerState, Patch[]]; -}; - /** * A universal supertype for the composable controller state object. * From 719a365adc1eecbb6f45baaa22b83ba16a7586eb Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 12:01:55 -0400 Subject: [PATCH 18/23] Add package-level exports --- packages/composable-controller/CHANGELOG.md | 5 ++++- packages/composable-controller/src/ComposableController.ts | 2 +- packages/composable-controller/src/index.ts | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index 59f40a44a6c..7639514eca2 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -20,7 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING**: Add ESM build ([#3998](https://github.com/MetaMask/core/pull/3998)) - It's no longer possible to import files from `./dist` directly. - Add and export functions `isBaseControllerV1` and `isBaseController`, which are type guards for validating controller instances ([#3904](https://github.com/MetaMask/core/pull/3904)) -- Add the `RestrictedControllerMessengerConstraint` type, which is the narrowest supertype of all controller-messenger instances ([#3904](https://github.com/MetaMask/core/pull/3904)) +- Adds and exports new types: ([#3904](https://github.com/MetaMask/core/pull/3904)) + - `RestrictedControllerMessengerConstraint`, which is the narrowest supertype of all controller-messenger instances. + - `LegacyControllerStateConstraint`, a universal supertype for the controller state object, encompassing both BaseControllerV1 and BaseControllerV2 state. + - `ComposableControllerStateConstraint`, the narrowest supertype for the composable controller state object. - `ComposableController` now accommodates `BaseControllerV1` controllers that use a messenger (specifically, which have a `messagingSystem` property which is an instance of `RestrictedControllerMessenger`), by subscribing to the `stateChange` event of the messenger instead of using the `subscribe` method on the controller ([#3964](https://github.com/MetaMask/core/pull/3964)) ### Changed diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index d3c12e280a6..4d6d727f3c2 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -186,7 +186,7 @@ export type ComposableControllerMessenger< AllowedEvents['type'] >; -export type GetChildControllers< +type GetChildControllers< ComposableControllerState, P extends keyof ComposableControllerState = keyof ComposableControllerState, > = P extends string diff --git a/packages/composable-controller/src/index.ts b/packages/composable-controller/src/index.ts index 3f6b8535a57..fc000563346 100644 --- a/packages/composable-controller/src/index.ts +++ b/packages/composable-controller/src/index.ts @@ -1,8 +1,9 @@ export type { - ComposableControllerState, + ComposableControllerStateConstraint, ComposableControllerStateChangeEvent, ComposableControllerEvents, ComposableControllerMessenger, + LegacyControllerStateConstraint, RestrictedControllerMessengerConstraint, } from './ComposableController'; export { From 58ba8bcce692f6e190166f765c16d46da940576c Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 12:02:23 -0400 Subject: [PATCH 19/23] Adapt tests --- .../src/ComposableController.test.ts | 112 ++++++++++-------- 1 file changed, 63 insertions(+), 49 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index d0696e10a07..6ab79f223c8 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -151,9 +151,13 @@ describe('ComposableController', () => { describe('BaseControllerV1', () => { it('should compose controller state', () => { + type ComposableControllerState = { + BarController: BarControllerState; + BazController: BazControllerState; + }; const composableMessenger = new ControllerMessenger< never, - ComposableControllerEvents + ComposableControllerEvents >().getRestricted({ name: 'ComposableController', allowedActions: [], @@ -171,9 +175,12 @@ describe('ComposableController', () => { }); it('should notify listeners of nested state change', () => { + type ComposableControllerState = { + BarController: BarControllerState; + }; const controllerMessenger = new ControllerMessenger< never, - ComposableControllerEvents + ComposableControllerEvents >(); const composableMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', @@ -203,9 +210,15 @@ describe('ComposableController', () => { describe('BaseControllerV2', () => { it('should compose controller state', () => { + type ComposableControllerState = { + FooController: FooControllerState; + QuzController: QuzControllerState; + }; const controllerMessenger = new ControllerMessenger< never, - FooControllerEvent | QuzControllerEvent + | ComposableControllerEvents + | FooControllerEvent + | QuzControllerEvent >(); const fooMessenger = controllerMessenger.getRestricted< 'FooController', @@ -224,11 +237,7 @@ describe('ComposableController', () => { const fooController = new FooController(fooMessenger); const quzController = new QuzController(quzMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - (FooControllerEvent | QuzControllerEvent)['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: [ @@ -236,10 +245,11 @@ describe('ComposableController', () => { 'QuzController:stateChange', ], }); - const composableController = new ComposableController({ - controllers: [fooController, quzController], - messenger: composableControllerMessenger, - }); + const composableController = + new ComposableController({ + controllers: [fooController, quzController], + messenger: composableControllerMessenger, + }); expect(composableController.state).toStrictEqual({ FooController: { foo: 'foo' }, QuzController: { quz: 'quz' }, @@ -247,9 +257,13 @@ describe('ComposableController', () => { }); it('should notify listeners of nested state change', () => { + type ComposableControllerState = { + FooController: FooControllerState; + }; const controllerMessenger = new ControllerMessenger< never, - ComposableControllerEvents | FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -257,16 +271,12 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController({ controllers: [fooController], messenger: composableControllerMessenger, }); @@ -289,10 +299,15 @@ describe('ComposableController', () => { describe('Mixed BaseControllerV1 and BaseControllerV2', () => { it('should compose controller state', () => { + type ComposableControllerState = { + BarController: BarControllerState; + FooController: FooControllerState; + }; const barController = new BarController(); const controllerMessenger = new ControllerMessenger< never, - FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -300,19 +315,16 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - const composableController = new ComposableController({ - controllers: [barController, fooController], - messenger: composableControllerMessenger, - }); + const composableController = + new ComposableController({ + controllers: [barController, fooController], + messenger: composableControllerMessenger, + }); expect(composableController.state).toStrictEqual({ BarController: { bar: 'bar' }, FooController: { foo: 'foo' }, @@ -320,10 +332,15 @@ describe('ComposableController', () => { }); it('should notify listeners of BaseControllerV1 state change', () => { + type ComposableControllerState = { + BarController: BarControllerState; + FooController: FooControllerState; + }; const barController = new BarController(); const controllerMessenger = new ControllerMessenger< never, - ComposableControllerEvents | FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -331,16 +348,12 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController({ controllers: [barController, fooController], messenger: composableControllerMessenger, }); @@ -363,10 +376,15 @@ describe('ComposableController', () => { }); it('should notify listeners of BaseControllerV2 state change', () => { + type ComposableControllerState = { + BarController: BarControllerState; + FooController: FooControllerState; + }; const barController = new BarController(); const controllerMessenger = new ControllerMessenger< never, - ComposableControllerEvents | FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -374,16 +392,12 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], }); - new ComposableController({ + new ComposableController({ controllers: [barController, fooController], messenger: composableControllerMessenger, }); @@ -428,10 +442,14 @@ describe('ComposableController', () => { }); it('should throw if composing a controller that does not extend from BaseController', () => { + type ComposableControllerState = { + FooController: FooControllerState; + }; const notController = new JsonRpcEngine(); const controllerMessenger = new ControllerMessenger< never, - FooControllerEvent + | ComposableControllerEvents + | FooControllerEvent >(); const fooControllerMessenger = controllerMessenger.getRestricted({ name: 'FooController', @@ -439,11 +457,7 @@ describe('ComposableController', () => { allowedEvents: [], }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted< - 'ComposableController', - never, - FooControllerEvent['type'] - >({ + const composableControllerMessenger = controllerMessenger.getRestricted({ name: 'ComposableController', allowedActions: [], allowedEvents: ['FooController:stateChange'], From aa05cf12e3d8282f56a7b01d7e84709d75b0f01d Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 12:11:48 -0400 Subject: [PATCH 20/23] Fix changelog by removing unreleased changes --- packages/composable-controller/CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index 7639514eca2..3c3cf34232b 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -20,10 +20,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING**: Add ESM build ([#3998](https://github.com/MetaMask/core/pull/3998)) - It's no longer possible to import files from `./dist` directly. - Add and export functions `isBaseControllerV1` and `isBaseController`, which are type guards for validating controller instances ([#3904](https://github.com/MetaMask/core/pull/3904)) -- Adds and exports new types: ([#3904](https://github.com/MetaMask/core/pull/3904)) - - `RestrictedControllerMessengerConstraint`, which is the narrowest supertype of all controller-messenger instances. - - `LegacyControllerStateConstraint`, a universal supertype for the controller state object, encompassing both BaseControllerV1 and BaseControllerV2 state. - - `ComposableControllerStateConstraint`, the narrowest supertype for the composable controller state object. - `ComposableController` now accommodates `BaseControllerV1` controllers that use a messenger (specifically, which have a `messagingSystem` property which is an instance of `RestrictedControllerMessenger`), by subscribing to the `stateChange` event of the messenger instead of using the `subscribe` method on the controller ([#3964](https://github.com/MetaMask/core/pull/3964)) ### Changed From e36ab328b1c0894ebc53dc3e6c7ed56ec7ab256c Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 25 Apr 2024 12:17:27 -0400 Subject: [PATCH 21/23] Add `as const` assertions to overrides of `name` property in v1 controllers --- packages/assets-controllers/src/AccountTrackerController.ts | 2 +- packages/assets-controllers/src/NftController.ts | 2 +- packages/assets-controllers/src/NftDetectionController.ts | 2 +- packages/assets-controllers/src/TokenRatesController.ts | 2 +- packages/message-manager/src/AbstractMessageManager.ts | 2 +- packages/message-manager/src/DecryptMessageManager.ts | 2 +- packages/message-manager/src/EncryptionPublicKeyManager.ts | 2 +- packages/message-manager/src/MessageManager.ts | 2 +- packages/message-manager/src/PersonalMessageManager.ts | 2 +- packages/message-manager/src/TypedMessageManager.ts | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/assets-controllers/src/AccountTrackerController.ts b/packages/assets-controllers/src/AccountTrackerController.ts index 6c8479dc2b7..3596790358b 100644 --- a/packages/assets-controllers/src/AccountTrackerController.ts +++ b/packages/assets-controllers/src/AccountTrackerController.ts @@ -112,7 +112,7 @@ export class AccountTrackerController extends StaticIntervalPollingControllerV1< /** * Name of this controller used during composition */ - override name = 'AccountTrackerController'; + override name = 'AccountTrackerController' as const; private readonly getIdentities: () => PreferencesState['identities']; diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 9061b8f1809..1ff52ab688e 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -932,7 +932,7 @@ export class NftController extends BaseControllerV1 { /** * Name of this controller used during composition */ - override name = 'NftController'; + override name = 'NftController' as const; private readonly getERC721AssetName: AssetsContractController['getERC721AssetName']; diff --git a/packages/assets-controllers/src/NftDetectionController.ts b/packages/assets-controllers/src/NftDetectionController.ts index 488d468d861..65eb9ac6bdb 100644 --- a/packages/assets-controllers/src/NftDetectionController.ts +++ b/packages/assets-controllers/src/NftDetectionController.ts @@ -405,7 +405,7 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< /** * Name of this controller used during composition */ - override name = 'NftDetectionController'; + override name = 'NftDetectionController' as const; private readonly getOpenSeaApiKey: () => string | undefined; diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index 2863bb7b96c..ce63dbd29ce 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -152,7 +152,7 @@ export class TokenRatesController extends StaticIntervalPollingControllerV1< /** * Name of this controller used during composition */ - override name = 'TokenRatesController'; + override name = 'TokenRatesController' as const; private readonly getNetworkClientById: NetworkController['getNetworkClientById']; diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index af24834fa49..849d9799f61 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -200,7 +200,7 @@ export abstract class AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'AbstractMessageManager' as const; + override name = 'AbstractMessageManager'; /** * Creates an AbstractMessageManager instance. diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index 2ef29ff7380..dbfd3a4a39e 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -68,7 +68,7 @@ export class DecryptMessageManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'DecryptMessageManager'; + override name = 'DecryptMessageManager' as const; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index cf9288ac50e..ab4f40f7d14 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -65,7 +65,7 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'EncryptionPublicKeyManager'; + override name = 'EncryptionPublicKeyManager' as const; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/MessageManager.ts b/packages/message-manager/src/MessageManager.ts index 2f98cf92fc5..8a561cbfe69 100644 --- a/packages/message-manager/src/MessageManager.ts +++ b/packages/message-manager/src/MessageManager.ts @@ -70,7 +70,7 @@ export class MessageManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'MessageManager'; + override name = 'MessageManager' as const; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/PersonalMessageManager.ts b/packages/message-manager/src/PersonalMessageManager.ts index 5f3db2aab47..f0f58e17baf 100644 --- a/packages/message-manager/src/PersonalMessageManager.ts +++ b/packages/message-manager/src/PersonalMessageManager.ts @@ -74,7 +74,7 @@ export class PersonalMessageManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'PersonalMessageManager'; + override name = 'PersonalMessageManager' as const; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/TypedMessageManager.ts b/packages/message-manager/src/TypedMessageManager.ts index 9193d3018eb..75682dedcc3 100644 --- a/packages/message-manager/src/TypedMessageManager.ts +++ b/packages/message-manager/src/TypedMessageManager.ts @@ -95,7 +95,7 @@ export class TypedMessageManager extends AbstractMessageManager< /** * Name of this controller used during composition */ - override name = 'TypedMessageManager'; + override name = 'TypedMessageManager' as const; /** * Creates a new TypedMessage with an 'unapproved' status using the passed messageParams. From 25f2f0b6574dc5dda7cc7832cbc5a8833f240e75 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 29 May 2024 09:36:16 -0400 Subject: [PATCH 22/23] Descriptive generic param name --- .../composable-controller/src/ComposableController.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 4d6d727f3c2..4c2ec8ad3bb 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -188,13 +188,13 @@ export type ComposableControllerMessenger< type GetChildControllers< ComposableControllerState, - P extends keyof ComposableControllerState = keyof ComposableControllerState, -> = P extends string - ? ComposableControllerState[P] extends StateConstraint - ? { name: P; state: ComposableControllerState[P] } + ControllerName extends keyof ComposableControllerState = keyof ComposableControllerState, +> = ControllerName extends string + ? ComposableControllerState[ControllerName] extends StateConstraint + ? { name: ControllerName; state: ComposableControllerState[ControllerName] } : BaseControllerV1< BaseConfig & Record, - BaseState & ComposableControllerState[P] + BaseState & ComposableControllerState[ControllerName] > : never; From 365aa0214cce99a9065e2901738ac3116b2619a5 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 29 May 2024 10:43:28 -0400 Subject: [PATCH 23/23] Update changelog --- packages/composable-controller/CHANGELOG.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index 3c3cf34232b..ee846caee9a 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Adds and exports new types: ([#3952](https://github.com/MetaMask/core/pull/3952)) + - `RestrictedControllerMessengerConstraint`, which is the narrowest supertype of all controller-messenger instances. + - `LegacyControllerStateConstraint`, a universal supertype for the controller state object, encompassing both BaseControllerV1 and BaseControllerV2 state. + - `ComposableControllerStateConstraint`, the narrowest supertype for the composable controller state object. + +### Changed + +- **BREAKING:** The `ComposableController` class is now a generic class that expects one generic argument `ComposableControllerState` ([#3952](https://github.com/MetaMask/core/pull/3952)). + - **BREAKING:** For the `ComposableController` class to be typed correctly, any of its child controllers that extend `BaseControllerV1` must have an overridden `name` property that is defined using the `as const` assertion. + ## [6.0.1] ### Fixed @@ -25,12 +37,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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)) -- **BREAKING:** The `ComposableController` class is now a generic class that expects one generic argument `ChildControllers` in the form of a union of the controllers passed into the `controllers` array constructor option ([#3941](https://github.com/MetaMask/core/pull/3941)) - - Child controllers that extend `BaseControllerV1` must have an overridden `name` property that is defined using the `as const` assertion for the `ComposableController` class to be typed correctly. -- Relax `payload` in `ComposableControllerStateChangeEvent` to use `Record` rather than `Record` ([#3949](https://github.com/MetaMask/core/pull/3949)) +- **BREAKING:** The `AllowedAction` 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)) - **BREAKING:** Bump `@metamask/base-controller` to `^5.0.0` ([#4039](https://github.com/MetaMask/core/pull/4039)) - This version has a number of breaking changes. See the changelog for more. +- Relax `payload` in `ComposableControllerStateChangeEvent` to use `Record` rather than `Record` ([#3949](https://github.com/MetaMask/core/pull/3949)) - Bump `@metamask/json-rpc-engine` to `^8.0.0` ([#4039](https://github.com/MetaMask/core/pull/4039)) ### Removed