From 203db7f6837ec83884bd59d0dfe51e2bea9157ce Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Wed, 24 Sep 2025 15:54:55 +0200 Subject: [PATCH 01/12] refactor: migrate `ComposableController` to `@metamask/messenger` --- packages/composable-controller/package.json | 3 +- .../src/ComposableController.test.ts | 356 +++++++++--------- .../src/ComposableController.ts | 46 ++- yarn.lock | 1 + 4 files changed, 203 insertions(+), 203 deletions(-) diff --git a/packages/composable-controller/package.json b/packages/composable-controller/package.json index 27313b5a3b9..b1f305e9eec 100644 --- a/packages/composable-controller/package.json +++ b/packages/composable-controller/package.json @@ -47,7 +47,8 @@ "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" }, "dependencies": { - "@metamask/base-controller": "^8.2.0" + "@metamask/base-controller": "^8.2.0", + "@metamask/messenger": "^0.3.0" }, "devDependencies": { "@metamask/auto-changelog": "^3.4.4", diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index c5c8bce434f..f3304c186da 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -1,11 +1,23 @@ -import type { RestrictedMessenger } from '@metamask/base-controller'; -import { BaseController, Messenger } from '@metamask/base-controller'; +import { + BaseController, + ControllerStateChangeEvent, + type ControllerGetStateAction, + StateConstraint, +} from '@metamask/base-controller/next'; import { JsonRpcEngine } from '@metamask/json-rpc-engine'; +import { + MOCK_ANY_NAMESPACE, + Messenger, + MessengerActions, + MessengerEvents, + MockAnyNamespace, +} from '@metamask/messenger'; import type { Patch } from 'immer'; import * as sinon from 'sinon'; import type { ChildControllerStateChangeEvents, + ComposableControllerActions, ComposableControllerEvents, } from './ComposableController'; import { @@ -15,20 +27,29 @@ import { // Mock BaseController classes +type RootMessenger = Messenger< + MockAnyNamespace, + MessengerActions | MessengerActions, + MessengerEvents | MessengerEvents +>; + type FooControllerState = { foo: string; }; +type FooControllerAction = ControllerGetStateAction< + 'FooController', + FooControllerState +>; type FooControllerEvent = { type: `FooController:stateChange`; payload: [FooControllerState, Patch[]]; }; -type FooMessenger = RestrictedMessenger< +type FooMessenger = Messenger< 'FooController', - never, + FooControllerAction, FooControllerEvent | QuzControllerEvent, - never, - QuzControllerEvent['type'] + RootMessenger >; const fooControllerStateMetadata = { @@ -62,17 +83,20 @@ class FooController extends BaseController< type QuzControllerState = { quz: string; }; +type QuzControllerAction = ControllerGetStateAction< + 'QuzController', + QuzControllerState +>; type QuzControllerEvent = { type: `QuzController:stateChange`; payload: [QuzControllerState, Patch[]]; }; -type QuzMessenger = RestrictedMessenger< +type QuzMessenger = Messenger< 'QuzController', - never, + QuzControllerAction, QuzControllerEvent, - never, - never + RootMessenger >; const quzControllerStateMetadata = { @@ -103,50 +127,17 @@ class QuzController extends BaseController< } } -type ControllerWithoutStateChangeEventState = { - qux: string; -}; - -type ControllerWithoutStateChangeEventMessenger = RestrictedMessenger< - 'ControllerWithoutStateChangeEvent', - never, - QuzControllerEvent, - never, - QuzControllerEvent['type'] +type ComposableControllerMessenger = Messenger< + 'ComposableController', + ControllerGetStateAction<'ComposableController', State>, + | ControllerStateChangeEvent<'ComposableController', State> + | FooControllerEvent, + RootMessenger >; -const controllerWithoutStateChangeEventStateMetadata = { - qux: { - persist: true, - anonymous: true, - }, -}; - -class ControllerWithoutStateChangeEvent extends BaseController< - 'ControllerWithoutStateChangeEvent', - ControllerWithoutStateChangeEventState, - ControllerWithoutStateChangeEventMessenger -> { - constructor(messagingSystem: ControllerWithoutStateChangeEventMessenger) { - super({ - messenger: messagingSystem, - metadata: controllerWithoutStateChangeEventStateMetadata, - name: 'ControllerWithoutStateChangeEvent', - state: { qux: 'qux' }, - }); - } - - updateState(qux: string) { - super.update((state) => { - state.qux = qux; - }); - } -} - type ControllersMap = { FooController: FooController; QuzController: QuzController; - ControllerWithoutStateChangeEvent: ControllerWithoutStateChangeEvent; }; describe('ComposableController', () => { @@ -157,39 +148,39 @@ describe('ComposableController', () => { describe('BaseController', () => { it('should compose controller state', () => { type ComposableControllerState = { - FooController: FooControllerState; QuzController: QuzControllerState; + FooController: FooControllerState; }; - const messenger = new Messenger< - never, - | ComposableControllerEvents - | FooControllerEvent - | QuzControllerEvent - >(); - const fooMessenger = messenger.getRestricted< - 'FooController', - never, - QuzControllerEvent['type'] - >({ - name: 'FooController', - allowedActions: [], - allowedEvents: ['QuzController:stateChange'], + const messenger: RootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, }); - const quzMessenger = messenger.getRestricted({ - name: 'QuzController', - allowedActions: [], - allowedEvents: [], + const fooMessenger: FooMessenger = new Messenger({ + namespace: 'FooController', + parent: messenger, + }); + messenger.delegate({ + messenger: fooMessenger, + events: ['QuzController:stateChange'], + }); + const quzMessenger: QuzMessenger = new Messenger({ + namespace: 'QuzController', + parent: messenger, }); const fooController = new FooController(fooMessenger); const quzController = new QuzController(quzMessenger); - const composableControllerMessenger = messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: [ - 'FooController:stateChange', - 'QuzController:stateChange', - ], + const composableControllerMessenger = new Messenger< + 'ComposableController', + never, + FooControllerEvent | QuzControllerEvent, + RootMessenger + >({ + namespace: 'ComposableController', + parent: messenger, + }); + composableControllerMessenger.delegate({ + messenger: fooMessenger, + events: ['FooController:stateChange', 'QuzController:stateChange'], }); const composableController = new ComposableController< ComposableControllerState, @@ -212,20 +203,32 @@ describe('ComposableController', () => { FooController: FooControllerState; }; const messenger = new Messenger< - never, - | ComposableControllerEvents + MockAnyNamespace, + | FooControllerAction + | ComposableControllerActions, | FooControllerEvent - >(); - const fooControllerMessenger = messenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], + | ComposableControllerEvents + >({ + namespace: MOCK_ANY_NAMESPACE, + }); + const fooControllerMessenger = new Messenger< + 'FooController', + FooControllerAction, + FooControllerEvent, + typeof messenger + >({ + namespace: 'FooController', + parent: messenger, }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], + const composableControllerMessenger: ComposableControllerMessenger = + new Messenger({ + namespace: 'ComposableController', + parent: messenger, + }); + composableControllerMessenger.delegate({ + messenger: fooControllerMessenger, + events: ['FooController:stateChange'], }); new ComposableController< ComposableControllerState, @@ -256,26 +259,47 @@ describe('ComposableController', () => { FooController: FooControllerState; }; const messenger = new Messenger< - never, + MockAnyNamespace, + | ComposableControllerActions + | QuzControllerAction + | FooControllerAction, | ComposableControllerEvents | ChildControllerStateChangeEvents - >(); - const quzControllerMessenger = messenger.getRestricted({ - name: 'QuzController', - allowedActions: [], - allowedEvents: [], + >({ namespace: MOCK_ANY_NAMESPACE }); + const quzControllerMessenger = new Messenger< + 'QuzController', + QuzControllerAction, + QuzControllerEvent, + typeof messenger + >({ + namespace: 'QuzController', + parent: messenger, }); const quzController = new QuzController(quzControllerMessenger); - const fooControllerMessenger = messenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], + const fooControllerMessenger = new Messenger< + 'FooController', + FooControllerAction, + FooControllerEvent, + typeof messenger + >({ + namespace: 'FooController', + parent: messenger, }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['QuzController:stateChange', 'FooController:stateChange'], + const composableControllerMessenger = new Messenger< + 'ComposableController', + ComposableControllerActions, + | ComposableControllerEvents + | FooControllerEvent + | QuzControllerEvent, + typeof messenger + >({ + namespace: 'ComposableController', + parent: messenger, + }); + messenger.delegate({ + messenger: composableControllerMessenger, + events: ['QuzController:stateChange', 'FooController:stateChange'], }); new ComposableController< ComposableControllerState, @@ -304,17 +328,29 @@ describe('ComposableController', () => { }); it('should throw if controller messenger not provided', () => { - const messenger = new Messenger(); - const quzControllerMessenger = messenger.getRestricted({ - name: 'QuzController', - allowedActions: [], - allowedEvents: [], + const messenger = new Messenger< + MockAnyNamespace, + QuzControllerAction | FooControllerAction, + QuzControllerEvent | FooControllerEvent + >({ namespace: MOCK_ANY_NAMESPACE }); + const quzControllerMessenger = new Messenger< + 'QuzController', + QuzControllerAction, + QuzControllerEvent, + typeof messenger + >({ + namespace: 'QuzController', + parent: messenger, }); const quzController = new QuzController(quzControllerMessenger); - const fooControllerMessenger = messenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], + const fooControllerMessenger = new Messenger< + 'FooController', + FooControllerAction, + FooControllerEvent, + typeof messenger + >({ + namespace: 'FooController', + parent: messenger, }); const fooController = new FooController(fooControllerMessenger); expect( @@ -335,19 +371,34 @@ describe('ComposableController', () => { }; const notController = new JsonRpcEngine(); const messenger = new Messenger< - never, + MockAnyNamespace, + | ComposableControllerActions + | FooControllerAction, ComposableControllerEvents | FooControllerEvent - >(); - const fooControllerMessenger = messenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], + >({ namespace: MOCK_ANY_NAMESPACE }); + const fooControllerMessenger = new Messenger< + 'FooController', + FooControllerAction, + FooControllerEvent, + typeof messenger + >({ + namespace: 'FooController', + parent: messenger, }); const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], + const composableControllerMessenger = new Messenger< + 'ComposableController', + ComposableControllerActions, + | ComposableControllerEvents + | FooControllerEvent, + typeof messenger + >({ + namespace: 'ComposableController', + parent: messenger, + }); + messenger.delegate({ + messenger: composableControllerMessenger, + events: ['FooController:stateChange'], }); expect( () => @@ -369,71 +420,4 @@ describe('ComposableController', () => { }), ).toThrow(INVALID_CONTROLLER_ERROR); }); - - it('should not throw if composing a controller without a `stateChange` event', () => { - const messenger = new Messenger(); - const controllerWithoutStateChangeEventMessenger = messenger.getRestricted({ - name: 'ControllerWithoutStateChangeEvent', - allowedActions: [], - allowedEvents: [], - }); - const controllerWithoutStateChangeEvent = - new ControllerWithoutStateChangeEvent( - controllerWithoutStateChangeEventMessenger, - ); - const fooControllerMessenger = messenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], - }); - const fooController = new FooController(fooControllerMessenger); - expect( - () => - new ComposableController({ - controllers: { - ControllerWithoutStateChangeEvent: - controllerWithoutStateChangeEvent, - FooController: fooController, - }, - messenger: messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], - }), - }), - ).not.toThrow(); - }); - - it('should not throw if a child controller `stateChange` event is missing from the messenger events allowlist', () => { - const messenger = new Messenger< - never, - FooControllerEvent | QuzControllerEvent - >(); - const QuzControllerMessenger = messenger.getRestricted({ - name: 'QuzController', - allowedActions: [], - allowedEvents: [], - }); - const quzController = new QuzController(QuzControllerMessenger); - const fooControllerMessenger = messenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], - }); - const fooController = new FooController(fooControllerMessenger); - expect( - () => - new ComposableController({ - controllers: { - QuzController: quzController, - FooController: fooController, - }, - messenger: messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], - }), - }), - ).not.toThrow(); - }); }); diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 8b2d908fb79..e802c97877e 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -1,12 +1,13 @@ import type { - RestrictedMessenger, StateConstraint, StateMetadata, StateMetadataConstraint, ControllerStateChangeEvent, + ControllerGetStateAction, BaseControllerInstance as ControllerInstance, -} from '@metamask/base-controller'; -import { BaseController } from '@metamask/base-controller'; +} from '@metamask/base-controller/next'; +import { BaseController } from '@metamask/base-controller/next'; +import type { Messenger } from '@metamask/messenger'; export const controllerName = 'ComposableController'; @@ -20,6 +21,15 @@ export type ComposableControllerStateConstraint = { [controllerName: string]: StateConstraint; }; +/** + * The `getState` action type for the {@link ComposableControllerMessenger}. + * + * @template ComposableControllerState - A type object that maps controller names to their state types. + */ +export type ComposableControllerGetStateAction< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ControllerGetStateAction; + /** * The `stateChange` event type for the {@link ComposableControllerMessenger}. * @@ -41,6 +51,15 @@ export type ComposableControllerEvents< ComposableControllerState extends ComposableControllerStateConstraint, > = ComposableControllerStateChangeEvent; +/** + * A union type of action types available to the {@link ComposableControllerMessenger}. + * + * @template ComposableControllerState - A type object that maps controller names to their state types. + */ +export type ComposableControllerActions< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ComposableControllerGetStateAction; + /** * A utility type that extracts controllers from the {@link ComposableControllerState} type, * and derives a union type of all of their corresponding `stateChange` events. @@ -75,13 +94,11 @@ export type AllowedEvents< */ export type ComposableControllerMessenger< ComposableControllerState extends ComposableControllerStateConstraint, -> = RestrictedMessenger< +> = Messenger< typeof controllerName, - never, + ComposableControllerActions, | ComposableControllerEvents - | AllowedEvents, - never, - AllowedEvents['type'] + | AllowedEvents >; /** @@ -106,7 +123,7 @@ export class ComposableController< * * @param options - Initial options used to configure this controller * @param options.controllers - An object that contains child controllers keyed by their names. - * @param options.messenger - A restricted messenger. + * @param options.messenger - A controller messenger. */ constructor({ controllers, @@ -141,6 +158,7 @@ export class ComposableController< }, {} as never, ), + // @ts-expect-error "Property 'messagingSystem' is missing in type ..." messenger, }); @@ -161,16 +179,12 @@ export class ComposableController< delete this.metadata[name]; delete this.state[name]; // eslint-disable-next-line no-empty - } catch (_) {} - // False negative. `name` is a string type. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + } catch {} throw new Error(`${name} - ${INVALID_CONTROLLER_ERROR}`); } try { - this.messagingSystem.subscribe( - // False negative. `name` is a string type. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `${name}:stateChange`, + this.messenger.subscribe( + `${controllerName}:stateChange`, (childState: StateConstraint) => { this.update((state) => { // Type assertion is necessary for property assignment to a generic type. This does not pollute or widen the type of the asserted variable. diff --git a/yarn.lock b/yarn.lock index 6ba685274fa..1e0f3f157e3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2864,6 +2864,7 @@ __metadata: "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/base-controller": "npm:^8.2.0" "@metamask/json-rpc-engine": "npm:^10.0.3" + "@metamask/messenger": "npm:^0.1.0" "@types/jest": "npm:^27.4.1" deepmerge: "npm:^4.2.2" immer: "npm:^9.0.6" From b4365edd7b8d90ea500ee47e1a7c2fa0e2639647 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Thu, 2 Oct 2025 14:50:07 +0200 Subject: [PATCH 02/12] revert rename `name` to `controllerName` --- packages/composable-controller/src/ComposableController.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 3bad5a93b6f..04be89d88fb 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -160,7 +160,6 @@ export class ComposableController< }, {} as never, ), - // @ts-expect-error "Property 'messagingSystem' is missing in type ..." messenger, }); @@ -186,7 +185,7 @@ export class ComposableController< } try { this.messenger.subscribe( - `${controllerName}:stateChange`, + `${name}:stateChange`, (childState: StateConstraint) => { this.update((state) => { // Type assertion is necessary for property assignment to a generic type. This does not pollute or widen the type of the asserted variable. From 534d5bb3439ce32af831bac33653d034d4393b44 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Mon, 20 Oct 2025 12:42:22 +0200 Subject: [PATCH 03/12] rename `anonymous` to `includeInDebugSnapshot` --- packages/base-controller/src/next/BaseController.ts | 2 +- packages/composable-controller/src/ComposableController.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/base-controller/src/next/BaseController.ts b/packages/base-controller/src/next/BaseController.ts index cf4dafe793c..7d2c0059ae2 100644 --- a/packages/base-controller/src/next/BaseController.ts +++ b/packages/base-controller/src/next/BaseController.ts @@ -112,7 +112,7 @@ export type StateDeriverConstraint = (value: never) => Json; * This type can be assigned to any `StatePropertyMetadata` type. */ export type StatePropertyMetadataConstraint = { - anonymous: boolean | StateDeriverConstraint; + includeInDebugSnapshot: boolean | StateDeriverConstraint; includeInStateLogs?: boolean | StateDeriverConstraint; persist: boolean | StateDeriverConstraint; usedInUi?: boolean; diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 04be89d88fb..06421a64110 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -145,7 +145,7 @@ export class ComposableController< (metadata as StateMetadataConstraint)[name] = { includeInStateLogs: false, persist: true, - anonymous: true, + includeInDebugSnapshot: true, usedInUi: false, }; return metadata; From ec56365f0a2add4e3c16e8e4e185f1a05f25c678 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Mon, 20 Oct 2025 12:52:29 +0200 Subject: [PATCH 04/12] update metadata tests --- .../src/ComposableController.test.ts | 158 ++++++++++++------ 1 file changed, 103 insertions(+), 55 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index 6058e7287c7..7b67a1e7d12 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -56,7 +56,9 @@ type FooMessenger = Messenger< const fooControllerStateMetadata = { foo: { persist: true, - anonymous: true, + includeInDebugSnapshot: true, + usedInUi: false, + includeInStateLogs: false, }, }; @@ -103,7 +105,9 @@ type QuzMessenger = Messenger< const quzControllerStateMetadata = { quz: { persist: true, - anonymous: true, + includeInDebugSnapshot: true, + usedInUi: false, + includeInStateLogs: false, }, }; @@ -428,24 +432,35 @@ describe('ComposableController', () => { FooController: FooControllerState; }; const messenger = new Messenger< - never, + MockAnyNamespace, + | ComposableControllerActions + | FooControllerAction, | ComposableControllerEvents | FooControllerEvent - >(); - const fooMessenger = messenger.getRestricted< + >({ namespace: MOCK_ANY_NAMESPACE }); + const fooControllerMessenger = new Messenger< 'FooController', - never, - never + FooControllerAction, + FooControllerEvent, + typeof messenger >({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], + namespace: 'FooController', + parent: messenger, }); - const fooController = new FooController(fooMessenger); - const composableControllerMessenger = messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], + const fooController = new FooController(fooControllerMessenger); + const composableControllerMessenger = new Messenger< + 'ComposableController', + ComposableControllerActions, + | ComposableControllerEvents + | FooControllerEvent, + typeof messenger + >({ + namespace: 'ComposableController', + parent: messenger, + }); + messenger.delegate({ + messenger: composableControllerMessenger, + events: ['FooController:stateChange'], }); const controller = new ComposableController< ComposableControllerState, @@ -461,7 +476,7 @@ describe('ComposableController', () => { deriveStateFromMetadata( controller.state, controller.metadata, - 'anonymous', + 'includeInDebugSnapshot', ), ).toMatchInlineSnapshot(` Object { @@ -477,24 +492,35 @@ describe('ComposableController', () => { FooController: FooControllerState; }; const messenger = new Messenger< - never, + MockAnyNamespace, + | ComposableControllerActions + | FooControllerAction, | ComposableControllerEvents | FooControllerEvent - >(); - const fooMessenger = messenger.getRestricted< + >({ namespace: MOCK_ANY_NAMESPACE }); + const fooControllerMessenger = new Messenger< 'FooController', - never, - never + FooControllerAction, + FooControllerEvent, + typeof messenger >({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], + namespace: 'FooController', + parent: messenger, }); - const fooController = new FooController(fooMessenger); - const composableControllerMessenger = messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], + const fooController = new FooController(fooControllerMessenger); + const composableControllerMessenger = new Messenger< + 'ComposableController', + ComposableControllerActions, + | ComposableControllerEvents + | FooControllerEvent, + typeof messenger + >({ + namespace: 'ComposableController', + parent: messenger, + }); + messenger.delegate({ + messenger: composableControllerMessenger, + events: ['FooController:stateChange'], }); const controller = new ComposableController< ComposableControllerState, @@ -520,24 +546,35 @@ describe('ComposableController', () => { FooController: FooControllerState; }; const messenger = new Messenger< - never, + MockAnyNamespace, + | ComposableControllerActions + | FooControllerAction, | ComposableControllerEvents | FooControllerEvent - >(); - const fooMessenger = messenger.getRestricted< + >({ namespace: MOCK_ANY_NAMESPACE }); + const fooControllerMessenger = new Messenger< 'FooController', - never, - never + FooControllerAction, + FooControllerEvent, + typeof messenger >({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], + namespace: 'FooController', + parent: messenger, }); - const fooController = new FooController(fooMessenger); - const composableControllerMessenger = messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], + const fooController = new FooController(fooControllerMessenger); + const composableControllerMessenger = new Messenger< + 'ComposableController', + ComposableControllerActions, + | ComposableControllerEvents + | FooControllerEvent, + typeof messenger + >({ + namespace: 'ComposableController', + parent: messenger, + }); + messenger.delegate({ + messenger: composableControllerMessenger, + events: ['FooController:stateChange'], }); const controller = new ComposableController< ComposableControllerState, @@ -569,24 +606,35 @@ describe('ComposableController', () => { FooController: FooControllerState; }; const messenger = new Messenger< - never, + MockAnyNamespace, + | ComposableControllerActions + | FooControllerAction, | ComposableControllerEvents | FooControllerEvent - >(); - const fooMessenger = messenger.getRestricted< + >({ namespace: MOCK_ANY_NAMESPACE }); + const fooControllerMessenger = new Messenger< 'FooController', - never, - never + FooControllerAction, + FooControllerEvent, + typeof messenger >({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], + namespace: 'FooController', + parent: messenger, }); - const fooController = new FooController(fooMessenger); - const composableControllerMessenger = messenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], + const fooController = new FooController(fooControllerMessenger); + const composableControllerMessenger = new Messenger< + 'ComposableController', + ComposableControllerActions, + | ComposableControllerEvents + | FooControllerEvent, + typeof messenger + >({ + namespace: 'ComposableController', + parent: messenger, + }); + messenger.delegate({ + messenger: composableControllerMessenger, + events: ['FooController:stateChange'], }); const controller = new ComposableController< ComposableControllerState, From b3f16e624d4ad9315d0bcad089c2f458e112e85d Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 22 Oct 2025 02:23:24 +0900 Subject: [PATCH 05/12] fix: `Messenger` type errors in `ComposableController` (#6904) ## Explanation - Fix incompatibility of `ChildControllerStateChangeEvents` type with `BaseController` (when used in the `Events` type argument of `ComposableControllerMessenger`) by removing unnecessary nested logic from definition. - Update generic parameter names `ControllerName` and `ControllerState` to `ChildControllerName`, `ChildControllerState` for reduced ambiguity. ## References - Branches from https://github.com/MetaMask/core/pull/6710 ## 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](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --- > [!NOTE] > Simplifies `ChildControllerStateChangeEvents` and types the messenger `subscribe` call to fix `ComposableController` Messenger type errors; updates changelog. > > - **Composable Controller**: > - **Types**: Simplify `ChildControllerStateChangeEvents` by removing nested conditional logic and renaming generics to `ChildControllerName`/`ChildControllerState`. > - **Messenger**: Type the `subscribe` call for child `stateChange` events, updating composed state with the received child state (with an inline ts-expect-error to bypass an unnecessary overload constraint). > - **Docs**: > - Update `packages/composable-controller/CHANGELOG.md` with a Fixed entry describing the type compatibility change. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b3444fb970ecbb8b37742b4450464cf2e5a5730c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .vscode/settings.json | 3 ++ eslint-warning-thresholds.json | 3 -- packages/composable-controller/CHANGELOG.md | 5 ++++ .../src/ComposableController.ts | 30 +++++++++---------- 4 files changed, 23 insertions(+), 18 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000000..25fa6215fdd --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "typescript.tsdk": "node_modules/typescript/lib" +} diff --git a/eslint-warning-thresholds.json b/eslint-warning-thresholds.json index a16cd0d14f3..fcfb3d02a3f 100644 --- a/eslint-warning-thresholds.json +++ b/eslint-warning-thresholds.json @@ -107,9 +107,6 @@ "packages/composable-controller/src/ComposableController.test.ts": { "import-x/namespace": 3 }, - "packages/composable-controller/src/ComposableController.ts": { - "@typescript-eslint/no-unused-vars": 1 - }, "packages/controller-utils/jest.environment.js": { "n/prefer-global/text-encoder": 1, "n/prefer-global/text-decoder": 1, diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index e68d9c2e7b3..8bfbf05ac4c 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Resolve incompatibility of `ChildControllerStateChangeEvents` type with `BaseController` (when used in the `Events` type argument of `ComposableControllerMessenger`) by removing unnecessary nested logic from definition ([#6904](https://github.com/MetaMask/core/pull/6904)) + - Also update generic parameter names `ControllerName` and `ControllerState` to `ChildControllerName`, `ChildControllerState` for reduced ambiguity. + ## [11.1.0] ### Added diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 06421a64110..03bdb317341 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -70,12 +70,10 @@ export type ChildControllerStateChangeEvents< ComposableControllerState extends ComposableControllerStateConstraint, > = ComposableControllerState extends Record< - infer ControllerName extends string, - infer ControllerState + infer ChildControllerName extends string, + infer ChildControllerState extends StateConstraint > - ? ControllerState extends StateConstraint - ? ControllerStateChangeEvent - : never + ? ControllerStateChangeEvent : never; /** @@ -184,16 +182,18 @@ export class ComposableController< throw new Error(`${name} - ${INVALID_CONTROLLER_ERROR}`); } try { - this.messenger.subscribe( - `${name}:stateChange`, - (childState: StateConstraint) => { - this.update((state) => { - // Type assertion is necessary for property assignment to a generic type. This does not pollute or widen the type of the asserted variable. - // @ts-expect-error "Type instantiation is excessively deep" - (state as ComposableControllerStateConstraint)[name] = childState; - }); - }, - ); + this.messenger.subscribe< + // The type intersection with "ComposableController:stateChange" is added by one of the `Messenger.subscribe` overloads, but that constraint is unnecessary here, + // since this method only subscribes the messenger to child controller `stateChange` events. + // @ts-expect-error "Type '`${string}:stateChange`' is not assignable to parameter of type '"ComposableController:stateChange" & ChildControllerStateChangeEvents["type"]'." + ChildControllerStateChangeEvents['type'] + >(`${name}:stateChange`, (childState: StateConstraint) => { + this.update((state) => { + // Type assertion is necessary for property assignment to a generic type. This does not pollute or widen the type of the asserted variable. + // @ts-expect-error "Type instantiation is excessively deep" + (state as ComposableControllerStateConstraint)[name] = childState; + }); + }); } catch (error: unknown) { // False negative. `name` is a string type. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions From 57bec0b8c3be51942581bf6f373aa3e7784b49c4 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 21 Oct 2025 19:40:48 +0200 Subject: [PATCH 06/12] add test case for child state change subscription failure --- .../src/ComposableController.test.ts | 59 ++++++++++++++++++- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index 7b67a1e7d12..d9e81c01e2a 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -231,8 +231,8 @@ describe('ComposableController', () => { namespace: 'ComposableController', parent: messenger, }); - composableControllerMessenger.delegate({ - messenger: fooControllerMessenger, + messenger.delegate({ + messenger: composableControllerMessenger, events: ['FooController:stateChange'], }); new ComposableController< @@ -246,7 +246,10 @@ describe('ComposableController', () => { }); const listener = sinon.stub(); - messenger.subscribe('ComposableController:stateChange', listener); + composableControllerMessenger.subscribe( + 'ComposableController:stateChange', + listener, + ); fooController.updateFoo('qux'); expect(listener.calledOnce).toBe(true); @@ -332,6 +335,56 @@ describe('ComposableController', () => { }); }); + it('should not throw if child state change event subscription fails', () => { + type ComposableControllerState = { + FooController: FooControllerState; + }; + const messenger = new Messenger< + MockAnyNamespace, + | ComposableControllerActions + | FooControllerAction, + ComposableControllerEvents | FooControllerEvent + >({ namespace: MOCK_ANY_NAMESPACE }); + const fooControllerMessenger = new Messenger< + 'FooController', + FooControllerAction, + FooControllerEvent, + typeof messenger + >({ + namespace: 'FooController', + parent: messenger, + }); + const fooController = new FooController(fooControllerMessenger); + const composableControllerMessenger = new Messenger< + 'ComposableController', + ComposableControllerActions, + | ComposableControllerEvents + | FooControllerEvent, + typeof messenger + >({ + namespace: 'ComposableController', + parent: messenger, + }); + messenger.delegate({ + messenger: composableControllerMessenger, + events: ['FooController:stateChange'], + }); + jest + .spyOn(composableControllerMessenger, 'subscribe') + .mockImplementation(() => { + throw new Error(); + }); + expect( + () => + new ComposableController({ + controllers: { + FooController: fooController, + }, + messenger: composableControllerMessenger, + }), + ).not.toThrow(); + }); + it('should throw if controller messenger not provided', () => { const messenger = new Messenger< MockAnyNamespace, From 030a9170a00a1e592816b9451fc8cb833bf7d049 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 21 Oct 2025 19:44:16 +0200 Subject: [PATCH 07/12] remove .vscode folder --- .vscode/settings.json | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 25fa6215fdd..00000000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "typescript.tsdk": "node_modules/typescript/lib" -} From 6287e52d6cec42110053c4301fd926a16c6c9c54 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 21 Oct 2025 19:46:10 +0200 Subject: [PATCH 08/12] update tsconfig and README files --- README.md | 1 + packages/composable-controller/tsconfig.build.json | 3 +++ packages/composable-controller/tsconfig.json | 3 +++ 3 files changed, 7 insertions(+) diff --git a/README.md b/README.md index ba3b7ae26f8..f51bcc2f582 100644 --- a/README.md +++ b/README.md @@ -198,6 +198,7 @@ linkStyle default opacity:0.5 chain_agnostic_permission --> network_controller; chain_agnostic_permission --> permission_controller; composable_controller --> base_controller; + composable_controller --> messenger; composable_controller --> json_rpc_engine; core_backend --> base_controller; core_backend --> controller_utils; diff --git a/packages/composable-controller/tsconfig.build.json b/packages/composable-controller/tsconfig.build.json index 779d385a6ab..249f327913d 100644 --- a/packages/composable-controller/tsconfig.build.json +++ b/packages/composable-controller/tsconfig.build.json @@ -8,6 +8,9 @@ "references": [ { "path": "../base-controller/tsconfig.build.json" + }, + { + "path": "../messenger/tsconfig.build.json" } ], "include": ["../../types", "./src"] diff --git a/packages/composable-controller/tsconfig.json b/packages/composable-controller/tsconfig.json index cc814f313b7..0d608a82545 100644 --- a/packages/composable-controller/tsconfig.json +++ b/packages/composable-controller/tsconfig.json @@ -7,6 +7,9 @@ { "path": "../base-controller" }, + { + "path": "../messenger" + }, { "path": "../json-rpc-engine" } From 2595e29d92809c00c60f5fd7313b004ea4914fb9 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 21 Oct 2025 19:48:25 +0200 Subject: [PATCH 09/12] update changelog --- packages/composable-controller/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index 8bfbf05ac4c..32a4bf9441b 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** Migrate `ComposableController` to new `Messenger` from `@metamask/messenger` ([#6710](https://github.com/MetaMask/core/pull/6710)) + - Previously, the controller accepted a `RestrictedMessenger` instance from `@metamask/base-controller`. + ### Fixed - Resolve incompatibility of `ChildControllerStateChangeEvents` type with `BaseController` (when used in the `Events` type argument of `ComposableControllerMessenger`) by removing unnecessary nested logic from definition ([#6904](https://github.com/MetaMask/core/pull/6904)) From 63e4718f5cb17c964d7a0195607f525447cfd4c4 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Fri, 24 Oct 2025 19:19:37 +0200 Subject: [PATCH 10/12] update changelog and readme --- README.md | 1 + packages/composable-controller/CHANGELOG.md | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9c8ef118646..618cbee74b0 100644 --- a/README.md +++ b/README.md @@ -217,6 +217,7 @@ linkStyle default opacity:0.5 earn_controller --> transaction_controller; eip_5792_middleware --> transaction_controller; eip_5792_middleware --> keyring_controller; + eip_7702_internal_rpc_middleware --> controller_utils; eip1193_permission_middleware --> chain_agnostic_permission; eip1193_permission_middleware --> controller_utils; eip1193_permission_middleware --> json_rpc_engine; diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index 0ed2942db74..85c3df4a74e 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -7,12 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- **BREAKING:** Migrate `ComposableController` to new `Messenger` from `@metamask/messenger` ([#6710](https://github.com/MetaMask/core/pull/6710)) + - Previously, the controller accepted a `RestrictedMessenger` instance from `@metamask/base-controller`. +- **BREAKING:** Metadata property `anonymous` renamed to `includeInDebugSnapshot` ([#6710](https://github.com/MetaMask/core/pull/6710)) + ## [11.1.1] ### Changed -- **BREAKING:** Migrate `ComposableController` to new `Messenger` from `@metamask/messenger` ([#6710](https://github.com/MetaMask/core/pull/6710)) - - Previously, the controller accepted a `RestrictedMessenger` instance from `@metamask/base-controller`. - Bump `@metamask/base-controller` from `^8.4.1` to `^8.4.2` ([#6917](https://github.com/MetaMask/core/pull/6917)) ### Fixed From 94d3033f3c9fd0a38cf4b6db71ce2ce8f7ba4c2d Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 24 Oct 2025 16:47:51 -0230 Subject: [PATCH 11/12] Fix changelog --- packages/composable-controller/CHANGELOG.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/composable-controller/CHANGELOG.md b/packages/composable-controller/CHANGELOG.md index 85c3df4a74e..0d445b11e8f 100644 --- a/packages/composable-controller/CHANGELOG.md +++ b/packages/composable-controller/CHANGELOG.md @@ -7,21 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + - **BREAKING:** Migrate `ComposableController` to new `Messenger` from `@metamask/messenger` ([#6710](https://github.com/MetaMask/core/pull/6710)) - Previously, the controller accepted a `RestrictedMessenger` instance from `@metamask/base-controller`. - **BREAKING:** Metadata property `anonymous` renamed to `includeInDebugSnapshot` ([#6710](https://github.com/MetaMask/core/pull/6710)) +### Fixed + +- Resolve incompatibility of `ChildControllerStateChangeEvents` type with `BaseController` (when used in the `Events` type argument of `ComposableControllerMessenger`) by removing unnecessary nested logic from definition ([#6904](https://github.com/MetaMask/core/pull/6904)) + - Also update generic parameter names `ControllerName` and `ControllerState` to `ChildControllerName`, `ChildControllerState` for reduced ambiguity. + ## [11.1.1] ### Changed - Bump `@metamask/base-controller` from `^8.4.1` to `^8.4.2` ([#6917](https://github.com/MetaMask/core/pull/6917)) -### Fixed - -- Resolve incompatibility of `ChildControllerStateChangeEvents` type with `BaseController` (when used in the `Events` type argument of `ComposableControllerMessenger`) by removing unnecessary nested logic from definition ([#6904](https://github.com/MetaMask/core/pull/6904)) - - Also update generic parameter names `ControllerName` and `ControllerState` to `ChildControllerName`, `ChildControllerState` for reduced ambiguity. - ## [11.1.0] ### Added From 17e90412cd24565bc99091062b5a21589935fe1b Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 24 Oct 2025 17:26:06 -0230 Subject: [PATCH 12/12] Fix broken delegation in unit test The test didn't actually rely upon subscriptions being setup correctly, so this mistake didn't impact the test result. It was only testing the constructor. The mistake has still been updated to avoid confusion. --- .../composable-controller/src/ComposableController.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index d9e81c01e2a..188742349f2 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -183,8 +183,8 @@ describe('ComposableController', () => { namespace: 'ComposableController', parent: messenger, }); - composableControllerMessenger.delegate({ - messenger: fooMessenger, + messenger.delegate({ + messenger: composableControllerMessenger, events: ['FooController:stateChange', 'QuzController:stateChange'], }); const composableController = new ComposableController<