From 8da2edaeaab6b46b8c36d94a15b4ea947174f444 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 3 Dec 2024 10:28:14 -0500 Subject: [PATCH] stash --- .../src/BaseControllerV1.test.ts | 117 ----- .../base-controller/src/BaseControllerV1.ts | 251 ---------- .../src/BaseControllerV2.test.ts | 7 - .../base-controller/src/BaseControllerV2.ts | 25 +- packages/base-controller/src/index.ts | 11 - .../src/ComposableController.test.ts | 471 +++++------------- .../src/ComposableController.ts | 68 +-- 7 files changed, 132 insertions(+), 818 deletions(-) delete mode 100644 packages/base-controller/src/BaseControllerV1.test.ts delete mode 100644 packages/base-controller/src/BaseControllerV1.ts diff --git a/packages/base-controller/src/BaseControllerV1.test.ts b/packages/base-controller/src/BaseControllerV1.test.ts deleted file mode 100644 index f268f0f4a3a..00000000000 --- a/packages/base-controller/src/BaseControllerV1.test.ts +++ /dev/null @@ -1,117 +0,0 @@ -import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import * as sinon from 'sinon'; - -import type { BaseConfig, BaseState } from './BaseControllerV1'; -import { - BaseControllerV1 as BaseController, - isBaseControllerV1, -} from './BaseControllerV1'; -import type { - CountControllerAction, - CountControllerEvent, -} from './BaseControllerV2.test'; -import { - CountController, - countControllerName, - countControllerStateMetadata, - getCountMessenger, -} from './BaseControllerV2.test'; -import { ControllerMessenger } from './ControllerMessenger'; - -const STATE = { name: 'foo' }; -const CONFIG = { disabled: true }; - -// eslint-disable-next-line jest/no-export -export class TestController extends BaseController { - constructor(config?: BaseConfig, state?: BaseState) { - super(config, state); - this.initialize(); - } -} - -describe('isBaseControllerV1', () => { - it('should return false if passed a V1 controller', () => { - const controller = new TestController(); - expect(isBaseControllerV1(controller)).toBe(true); - }); - - it('should return false if passed a V2 controller', () => { - const controllerMessenger = new ControllerMessenger< - CountControllerAction, - CountControllerEvent - >(); - const controller = new CountController({ - messenger: getCountMessenger(controllerMessenger), - name: countControllerName, - state: { count: 0 }, - metadata: countControllerStateMetadata, - }); - expect(isBaseControllerV1(controller)).toBe(false); - }); - - it('should return false if passed a non-controller', () => { - const notController = new JsonRpcEngine(); - // @ts-expect-error Intentionally passing invalid input to test runtime behavior - expect(isBaseControllerV1(notController)).toBe(false); - }); -}); - -describe('BaseController', () => { - afterEach(() => { - sinon.restore(); - }); - - it('should set initial state', () => { - const controller = new TestController(undefined, STATE); - expect(controller.state).toStrictEqual(STATE); - }); - - it('should set initial config', () => { - const controller = new TestController(CONFIG); - expect(controller.config).toStrictEqual(CONFIG); - }); - - it('should overwrite state', () => { - const controller = new TestController(); - expect(controller.state).toStrictEqual({}); - controller.update(STATE, true); - expect(controller.state).toStrictEqual(STATE); - }); - - it('should overwrite config', () => { - const controller = new TestController(); - expect(controller.config).toStrictEqual({}); - controller.configure(CONFIG, true); - expect(controller.config).toStrictEqual(CONFIG); - }); - - it('should be able to partially update the config', () => { - const controller = new TestController(CONFIG); - expect(controller.config).toStrictEqual(CONFIG); - controller.configure({ disabled: false }, false, false); - expect(controller.config).toStrictEqual({ disabled: false }); - }); - - it('should notify all listeners', () => { - const controller = new TestController(undefined, STATE); - const listenerOne = sinon.stub(); - const listenerTwo = sinon.stub(); - controller.subscribe(listenerOne); - controller.subscribe(listenerTwo); - controller.notify(); - expect(listenerOne.calledOnce).toBe(true); - expect(listenerTwo.calledOnce).toBe(true); - expect(listenerOne.getCall(0).args[0]).toStrictEqual(STATE); - expect(listenerTwo.getCall(0).args[0]).toStrictEqual(STATE); - }); - - it('should not notify unsubscribed listeners', () => { - const controller = new TestController(); - const listener = sinon.stub(); - controller.subscribe(listener); - controller.unsubscribe(listener); - controller.unsubscribe(() => null); - controller.notify(); - expect(listener.called).toBe(false); - }); -}); diff --git a/packages/base-controller/src/BaseControllerV1.ts b/packages/base-controller/src/BaseControllerV1.ts deleted file mode 100644 index 97843f642e7..00000000000 --- a/packages/base-controller/src/BaseControllerV1.ts +++ /dev/null @@ -1,251 +0,0 @@ -import type { PublicInterface } from '@metamask/utils'; - -import type { ControllerInstance } from './BaseControllerV2'; - -/** - * Determines if the given controller is an instance of `BaseControllerV1` - * - * @param controller - Controller instance to check - * @returns True if the controller is an instance of `BaseControllerV1` - */ -export function isBaseControllerV1( - controller: ControllerInstance, -): controller is BaseControllerV1Instance { - return ( - 'name' in controller && - typeof controller.name === 'string' && - 'config' in controller && - typeof controller.config === 'object' && - 'defaultConfig' in controller && - typeof controller.defaultConfig === 'object' && - 'state' in controller && - typeof controller.state === 'object' && - 'defaultState' in controller && - typeof controller.defaultState === 'object' && - 'disabled' in controller && - typeof controller.disabled === 'boolean' && - 'subscribe' in controller && - typeof controller.subscribe === 'function' - ); -} - -/** - * State change callbacks - */ -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/naming-convention -export type Listener = (state: T) => void; - -/** - * @type BaseConfig - * - * Base controller configuration - * @property disabled - Determines if this controller is enabled - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface BaseConfig { - disabled?: boolean; -} - -/** - * @type BaseState - * - * Base state representation - * @property name - Unique name for this controller - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface BaseState { - name?: string; -} - -/** - * The narrowest supertype for `BaseControllerV1` config objects. - * This type can be assigned to any `BaseControllerV1` config object. - */ -export type ConfigConstraint = BaseConfig & object; - -/** - * The narrowest supertype for `BaseControllerV1` state objects. - * This type can be assigned to any `BaseControllerV1` state object. - */ -export type StateConstraint = BaseState & object; - -/** - * The widest subtype of all controller instances that extend from `BaseControllerV1`. - * Any `BaseControllerV1` instance can be assigned to this type. - */ -export type BaseControllerV1Instance = PublicInterface< - BaseControllerV1 ->; - -/** - * @deprecated This class has been renamed to BaseControllerV1 and is no longer recommended for use for controllers. Please use BaseController (formerly BaseControllerV2) instead. - * - * Controller class that provides configuration, state management, and subscriptions. - * - * The core purpose of every controller is to maintain an internal data object - * called "state". Each controller is responsible for its own state, and all global wallet state - * is tracked in a controller as state. - */ -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/naming-convention -export class BaseControllerV1 { - /** - * Default options used to configure this controller - */ - defaultConfig: C = {} as never; - - /** - * Default state set on this controller - */ - defaultState: S = {} as never; - - /** - * Determines if listeners are notified of state changes - */ - disabled = false; - - /** - * Name of this controller used during composition - */ - name = 'BaseController'; - - private readonly initialConfig: Partial; - - private readonly initialState: Partial; - - private internalConfig: C = this.defaultConfig; - - private internalState: S = this.defaultState; - - private readonly internalListeners: Listener[] = []; - - /** - * Creates a BaseControllerV1 instance. Both initial state and initial - * configuration options are merged with defaults upon initialization. - * - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. - */ - constructor(config: Partial = {}, state: Partial = {}) { - this.initialState = state; - this.initialConfig = config; - } - - /** - * Enables the controller. This sets each config option as a member - * variable on this instance and triggers any defined setters. This - * also sets initial state and triggers any listeners. - * - * @returns This controller instance. - */ - protected initialize() { - this.internalState = this.defaultState; - this.internalConfig = this.defaultConfig; - this.configure(this.initialConfig); - this.update(this.initialState); - return this; - } - - /** - * Retrieves current controller configuration options. - * - * @returns The current configuration. - */ - get config() { - return this.internalConfig; - } - - /** - * Retrieves current controller state. - * - * @returns The current state. - */ - get state() { - return this.internalState; - } - - /** - * Updates controller configuration. - * - * @param config - New configuration options. - * @param overwrite - Overwrite config instead of merging. - * @param fullUpdate - Boolean that defines if the update is partial or not. - */ - configure(config: Partial, overwrite = false, fullUpdate = true) { - if (fullUpdate) { - this.internalConfig = overwrite - ? (config as C) - : Object.assign(this.internalConfig, config); - - for (const key of Object.keys(this.internalConfig) as (keyof C)[]) { - const value = this.internalConfig[key]; - if (value !== undefined) { - (this as unknown as C)[key] = value; - } - } - } else { - for (const key of Object.keys(config) as (keyof C)[]) { - /* istanbul ignore else */ - if (this.internalConfig[key] !== undefined) { - const value = (config as C)[key]; - this.internalConfig[key] = value; - (this as unknown as C)[key] = value; - } - } - } - } - - /** - * Notifies all subscribed listeners of current state. - */ - notify() { - if (this.disabled) { - return; - } - - this.internalListeners.forEach((listener) => { - listener(this.internalState); - }); - } - - /** - * Adds new listener to be notified of state changes. - * - * @param listener - The callback triggered when state changes. - */ - subscribe(listener: Listener) { - this.internalListeners.push(listener); - } - - /** - * Removes existing listener from receiving state changes. - * - * @param listener - The callback to remove. - * @returns `true` if a listener is found and unsubscribed. - */ - unsubscribe(listener: Listener) { - const index = this.internalListeners.findIndex((cb) => listener === cb); - index > -1 && this.internalListeners.splice(index, 1); - return index > -1; - } - - /** - * Updates controller state. - * - * @param state - The new state. - * @param overwrite - Overwrite state instead of merging. - */ - update(state: Partial, overwrite = false) { - this.internalState = overwrite - ? Object.assign({}, state as S) - : Object.assign({}, this.internalState, state); - this.notify(); - } -} - -export default BaseControllerV1; diff --git a/packages/base-controller/src/BaseControllerV2.test.ts b/packages/base-controller/src/BaseControllerV2.test.ts index 92275050906..7ef20823532 100644 --- a/packages/base-controller/src/BaseControllerV2.test.ts +++ b/packages/base-controller/src/BaseControllerV2.test.ts @@ -3,7 +3,6 @@ import type { Draft, Patch } from 'immer'; import * as sinon from 'sinon'; import { JsonRpcEngine } from '../../json-rpc-engine/src'; -import { TestController } from './BaseControllerV1.test'; import type { ControllerGetStateAction, ControllerStateChangeEvent, @@ -196,14 +195,8 @@ describe('isBaseController', () => { expect(isBaseController(controller)).toBe(true); }); - it('should return false if passed a V1 controller', () => { - const controller = new TestController(); - expect(isBaseController(controller)).toBe(false); - }); - it('should return false if passed a non-controller', () => { const notController = new JsonRpcEngine(); - // @ts-expect-error Intentionally passing invalid input to test runtime behavior expect(isBaseController(notController)).toBe(false); }); }); diff --git a/packages/base-controller/src/BaseControllerV2.ts b/packages/base-controller/src/BaseControllerV2.ts index 355d156689a..036e2c21334 100644 --- a/packages/base-controller/src/BaseControllerV2.ts +++ b/packages/base-controller/src/BaseControllerV2.ts @@ -2,10 +2,6 @@ import type { Json, PublicInterface } from '@metamask/utils'; import { enablePatches, produceWithPatches, applyPatches, freeze } from 'immer'; import type { Draft, Patch } from 'immer'; -import type { - BaseControllerV1Instance, - StateConstraint as StateConstraintV1, -} from './BaseControllerV1'; import type { ActionConstraint, EventConstraint } from './ControllerMessenger'; import type { RestrictedControllerMessenger, @@ -21,9 +17,11 @@ enablePatches(); * @returns True if the controller is an instance of `BaseController` */ export function isBaseController( - controller: ControllerInstance, + controller: unknown, ): controller is BaseControllerInstance { return ( + typeof controller === 'object' && + controller !== null && 'name' in controller && typeof controller.name === 'string' && 'state' in controller && @@ -40,14 +38,6 @@ export function isBaseController( */ export type StateConstraint = Record; -/** - * A universal supertype for the controller state object, encompassing both `BaseControllerV1` and `BaseControllerV2` state. - */ -// TODO: Remove once BaseControllerV2 migrations are completed for all controllers. -export type LegacyControllerStateConstraint = - | StateConstraintV1 - | StateConstraint; - /** * A state change listener. * @@ -146,15 +136,6 @@ export type BaseControllerInstance = Omit< metadata: StateMetadataConstraint; }; -/** - * A widest subtype of all controller instances that inherit from `BaseController` (formerly `BaseControllerV2`) or `BaseControllerV1`. - * Any `BaseController` or `BaseControllerV1` subclass instance can be assigned to this type. - */ -// TODO: Remove once BaseControllerV2 migrations are completed for all controllers. -export type ControllerInstance = - | BaseControllerV1Instance - | BaseControllerInstance; - export type ControllerGetStateAction< ControllerName extends string, ControllerState extends StateConstraint, diff --git a/packages/base-controller/src/index.ts b/packages/base-controller/src/index.ts index a4eab5b5724..563b7d7f802 100644 --- a/packages/base-controller/src/index.ts +++ b/packages/base-controller/src/index.ts @@ -1,18 +1,7 @@ -export type { - BaseConfig, - BaseControllerV1Instance, - BaseState, - ConfigConstraint as ConfigConstraintV1, - Listener, - StateConstraint as StateConstraintV1, -} from './BaseControllerV1'; -export { BaseControllerV1, isBaseControllerV1 } from './BaseControllerV1'; export type { BaseControllerInstance, - ControllerInstance, Listener as ListenerV2, StateConstraint, - LegacyControllerStateConstraint, StateDeriver, StateDeriverConstraint, StateMetadata, diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index 9059923d428..9acc8257779 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -1,15 +1,8 @@ // `ComposableControllerState` type objects are keyed with controller names written in PascalCase. /* eslint-disable @typescript-eslint/naming-convention */ -import type { - BaseState, - RestrictedControllerMessenger, -} from '@metamask/base-controller'; -import { - BaseController, - BaseControllerV1, - ControllerMessenger, -} from '@metamask/base-controller'; +import type { RestrictedControllerMessenger } from '@metamask/base-controller'; +import { BaseController, ControllerMessenger } from '@metamask/base-controller'; import { JsonRpcEngine } from '@metamask/json-rpc-engine'; import type { Patch } from 'immer'; import * as sinon from 'sinon'; @@ -113,61 +106,6 @@ class QuzController extends BaseController< } } -// Mock BaseControllerV1 classes - -type BarControllerState = BaseState & { - bar: string; -}; - -class BarController extends BaseControllerV1 { - defaultState = { - bar: 'bar', - }; - - override name = 'BarController' as const; - - constructor() { - super(); - this.initialize(); - } - - updateBar(bar: string) { - super.update({ bar }); - } -} - -type BazControllerState = BaseState & { - baz: string; -}; -type BazControllerEvent = { - type: `BazController:stateChange`; - payload: [BazControllerState, Patch[]]; -}; - -type BazMessenger = RestrictedControllerMessenger< - 'BazController', - never, - BazControllerEvent, - never, - never ->; - -class BazController extends BaseControllerV1 { - defaultState = { - baz: 'baz', - }; - - override name = 'BazController' as const; - - protected messagingSystem: BazMessenger; - - constructor({ messenger }: { messenger: BazMessenger }) { - super(); - this.initialize(); - this.messagingSystem = messenger; - } -} - type QuxControllerState = { qux: string; }; @@ -211,8 +149,6 @@ class QuxController extends BaseController< type ControllersMap = { FooController: FooController; QuzController: QuzController; - BarController: BarController; - BazController: BazController; QuxController: QuzController; }; @@ -221,86 +157,6 @@ describe('ComposableController', () => { sinon.restore(); }); - describe('BaseControllerV1', () => { - it('should compose controller state', () => { - type ComposableControllerState = { - BarController: BarControllerState; - BazController: BazControllerState; - }; - - const composableMessenger = new ControllerMessenger< - never, - | ComposableControllerEvents - | ChildControllerStateChangeEvents - >().getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: [ - 'BarController:stateChange', - 'BazController:stateChange', - ], - }); - const controller = new ComposableController< - ComposableControllerState, - Pick - >({ - controllers: { - BarController: new BarController(), - BazController: new BazController({ - messenger: new ControllerMessenger().getRestricted({ - name: 'BazController', - allowedActions: [], - allowedEvents: [], - }), - }), - }, - messenger: composableMessenger, - }); - - expect(controller.state).toStrictEqual({ - BarController: { bar: 'bar' }, - BazController: { baz: 'baz' }, - }); - }); - - it('should notify listeners of nested state change', () => { - type ComposableControllerState = { - BarController: BarControllerState; - }; - const controllerMessenger = new ControllerMessenger< - never, - | ComposableControllerEvents - | ChildControllerStateChangeEvents - >(); - const composableMessenger = controllerMessenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['BarController:stateChange'], - }); - const barController = new BarController(); - new ComposableController< - ComposableControllerState, - Pick - >({ - controllers: { BarController: barController }, - messenger: composableMessenger, - }); - const listener = sinon.stub(); - controllerMessenger.subscribe( - 'ComposableController:stateChange', - listener, - ); - barController.updateBar('something different'); - - expect(listener.calledOnce).toBe(true); - expect(listener.getCall(0).args[0]).toStrictEqual({ - BarController: { - bar: 'something different', - }, - }); - }); - }); - describe('BaseControllerV2', () => { it('should compose controller state', () => { type ComposableControllerState = { @@ -389,231 +245,138 @@ describe('ComposableController', () => { 'ComposableController:stateChange', listener, ); - fooController.updateFoo('bar'); + fooController.updateFoo('qux'); expect(listener.calledOnce).toBe(true); expect(listener.getCall(0).args[0]).toStrictEqual({ FooController: { - foo: 'bar', + foo: 'qux', }, }); }); }); - 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, - | ComposableControllerEvents - | ChildControllerStateChangeEvents - >(); - const fooControllerMessenger = controllerMessenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], - }); - const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: [ - 'BarController:stateChange', - 'FooController:stateChange', - ], - }); - const composableController = new ComposableController< - ComposableControllerState, - Pick - >({ - controllers: { - BarController: barController, - FooController: fooController, - }, - messenger: composableControllerMessenger, - }); - expect(composableController.state).toStrictEqual({ - BarController: { bar: 'bar' }, - FooController: { foo: 'foo' }, - }); + it('should notify listeners of BaseControllerV2 state change', () => { + type ComposableControllerState = { + QuzController: QuzControllerState; + FooController: FooControllerState; + }; + const controllerMessenger = new ControllerMessenger< + never, + | ComposableControllerEvents + | ChildControllerStateChangeEvents + >(); + const quzControllerMessenger = controllerMessenger.getRestricted({ + name: 'QuzController', + allowedActions: [], + allowedEvents: [], }); - - it('should notify listeners of BaseControllerV1 state change', () => { - type ComposableControllerState = { - BarController: BarControllerState; - FooController: FooControllerState; - }; - const barController = new BarController(); - const controllerMessenger = new ControllerMessenger< - never, - | ComposableControllerEvents - | ChildControllerStateChangeEvents - >(); - const fooControllerMessenger = controllerMessenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], - }); - const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: [ - 'BarController:stateChange', - 'FooController:stateChange', - ], - }); - new ComposableController< - ComposableControllerState, - Pick - >({ - controllers: { - BarController: barController, - FooController: fooController, - }, - messenger: composableControllerMessenger, - }); - const listener = sinon.stub(); - controllerMessenger.subscribe( - 'ComposableController:stateChange', - listener, - ); - barController.updateBar('foo'); - - expect(listener.calledOnce).toBe(true); - expect(listener.getCall(0).args[0]).toStrictEqual({ - BarController: { - bar: 'foo', - }, - FooController: { - foo: 'foo', - }, - }); + const quzController = new QuzController(quzControllerMessenger); + const fooControllerMessenger = controllerMessenger.getRestricted({ + name: 'FooController', + allowedActions: [], + allowedEvents: [], + }); + const fooController = new FooController(fooControllerMessenger); + const composableControllerMessenger = controllerMessenger.getRestricted({ + name: 'ComposableController', + allowedActions: [], + allowedEvents: ['QuzController:stateChange', 'FooController:stateChange'], + }); + new ComposableController< + ComposableControllerState, + Pick + >({ + controllers: { + QuzController: quzController, + FooController: fooController, + }, + messenger: composableControllerMessenger, }); - it('should notify listeners of BaseControllerV2 state change', () => { - type ComposableControllerState = { - BarController: BarControllerState; - FooController: FooControllerState; - }; - const barController = new BarController(); - const controllerMessenger = new ControllerMessenger< - never, - | ComposableControllerEvents - | ChildControllerStateChangeEvents - >(); - const fooControllerMessenger = controllerMessenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], - }); - const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: [ - 'BarController:stateChange', - 'FooController:stateChange', - ], - }); - new ComposableController< - ComposableControllerState, - Pick - >({ - controllers: { - BarController: barController, - FooController: fooController, - }, - messenger: composableControllerMessenger, - }); - - const listener = sinon.stub(); - controllerMessenger.subscribe( - 'ComposableController:stateChange', - listener, - ); - fooController.updateFoo('bar'); - - expect(listener.calledOnce).toBe(true); - expect(listener.getCall(0).args[0]).toStrictEqual({ - BarController: { - bar: 'bar', - }, - FooController: { - foo: 'bar', - }, - }); + const listener = sinon.stub(); + controllerMessenger.subscribe('ComposableController:stateChange', listener); + fooController.updateFoo('qux'); + + expect(listener.calledOnce).toBe(true); + expect(listener.getCall(0).args[0]).toStrictEqual({ + QuzController: { + quz: 'quz', + }, + FooController: { + foo: 'qux', + }, }); + }); - it('should throw if controller messenger not provided', () => { - const barController = new BarController(); - const controllerMessenger = new ControllerMessenger< - never, - FooControllerEvent - >(); - const fooControllerMessenger = controllerMessenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], - }); - const fooController = new FooController(fooControllerMessenger); - expect( - () => - // @ts-expect-error - Suppressing type error to test for runtime error handling - new ComposableController({ - controllers: { - BarController: barController, - FooController: fooController, - }, - }), - ).toThrow('Messaging system is required'); + it('should throw if controller messenger not provided', () => { + const controllerMessenger = new ControllerMessenger< + never, + FooControllerEvent + >(); + const quzControllerMessenger = controllerMessenger.getRestricted({ + name: 'QuzController', + allowedActions: [], + allowedEvents: [], }); + const quzController = new QuzController(quzControllerMessenger); + const fooControllerMessenger = controllerMessenger.getRestricted({ + name: 'FooController', + allowedActions: [], + allowedEvents: [], + }); + const fooController = new FooController(fooControllerMessenger); + expect( + () => + // @ts-expect-error - Suppressing type error to test for runtime error handling + new ComposableController({ + controllers: { + QuzController: quzController, + FooController: fooController, + }, + }), + ).toThrow('Messaging system is required'); + }); - 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, - | ComposableControllerEvents - | FooControllerEvent - >(); - const fooControllerMessenger = controllerMessenger.getRestricted({ - name: 'FooController', - allowedActions: [], - allowedEvents: [], - }); - const fooController = new FooController(fooControllerMessenger); - const composableControllerMessenger = controllerMessenger.getRestricted({ - name: 'ComposableController', - allowedActions: [], - allowedEvents: ['FooController:stateChange'], - }); - expect( - () => - new ComposableController< - ComposableControllerState & { - JsonRpcEngine: Record; - }, - // @ts-expect-error - Suppressing type error to test for runtime error handling - { - JsonRpcEngine: typeof notController; - FooController: FooController; - } - >({ - controllers: { - JsonRpcEngine: notController, - FooController: fooController, - }, - messenger: composableControllerMessenger, - }), - ).toThrow(INVALID_CONTROLLER_ERROR); + 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, + ComposableControllerEvents | FooControllerEvent + >(); + const fooControllerMessenger = controllerMessenger.getRestricted({ + name: 'FooController', + allowedActions: [], + allowedEvents: [], + }); + const fooController = new FooController(fooControllerMessenger); + const composableControllerMessenger = controllerMessenger.getRestricted({ + name: 'ComposableController', + allowedActions: [], + allowedEvents: ['FooController:stateChange'], }); + expect( + () => + new ComposableController< + // @ts-expect-error - Suppressing type error to test for runtime error handling + ComposableControllerState & { + JsonRpcEngine: Record; + }, + { + JsonRpcEngine: typeof notController; + FooController: FooController; + } + >({ + controllers: { + JsonRpcEngine: notController, + FooController: fooController, + }, + messenger: composableControllerMessenger, + }), + ).toThrow(INVALID_CONTROLLER_ERROR); }); it('should not throw if composing a controller without a `stateChange` event', () => { diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index b7178227773..c923d61b293 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -1,58 +1,23 @@ import type { RestrictedControllerMessenger, StateConstraint, - StateConstraintV1, StateMetadata, StateMetadataConstraint, ControllerStateChangeEvent, - LegacyControllerStateConstraint, - ControllerInstance, + BaseControllerInstance as ControllerInstance, } from '@metamask/base-controller'; -import { - BaseController, - isBaseController, - isBaseControllerV1, -} from '@metamask/base-controller'; -import type { Patch } from 'immer'; +import { BaseController, isBaseController } from '@metamask/base-controller'; export const controllerName = 'ComposableController'; export const INVALID_CONTROLLER_ERROR = - 'Invalid controller: controller must have a `messagingSystem` or be a class inheriting from `BaseControllerV1`.'; - -/** - * 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 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; -}; + 'Invalid controller: controller must have a `messagingSystem` and inherit from `BaseController`.'; /** * 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; -}; - -/** - * A `stateChange` 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 StateConstraintV1, -> = { - type: `${ControllerName}:stateChange`; - payload: [ControllerState, Patch[]]; + [controllerName: string]: StateConstraint; }; /** @@ -62,7 +27,7 @@ type LegacyControllerStateChangeEvent< */ export type ComposableControllerStateChangeEvent< ComposableControllerState extends ComposableControllerStateConstraint, -> = LegacyControllerStateChangeEvent< +> = ControllerStateChangeEvent< typeof controllerName, ComposableControllerState >; @@ -92,9 +57,6 @@ export type ChildControllerStateChangeEvents< > ? ControllerState extends StateConstraint ? ControllerStateChangeEvent - : // TODO: Remove this conditional branch once `BaseControllerV2` migrations are completed for all controllers. - ControllerState extends StateConstraintV1 - ? LegacyControllerStateChangeEvent : never : never; @@ -130,7 +92,7 @@ export type ComposableControllerMessenger< * @template ChildControllers - A type object that specifies the child controllers which are used to instantiate the {@link ComposableController}. */ export class ComposableController< - ComposableControllerState extends LegacyComposableControllerStateConstraint, + ComposableControllerState extends ComposableControllerStateConstraint, ChildControllers extends Record< keyof ComposableControllerState, ControllerInstance @@ -171,8 +133,8 @@ export class ComposableController< }, {} as never), state: Object.values(controllers).reduce( (state, controller) => { - (state as ComposableControllerStateConstraint)[controller.name] = - controller.state; + // @ts-expect-error - Suppressing error to assign new property to generic type + state[controller.name] = controller.state; return state; }, {} as never, @@ -192,7 +154,7 @@ export class ComposableController< */ #updateChildController(controller: ControllerInstance): void { const { name } = controller; - if (!isBaseController(controller) && !isBaseControllerV1(controller)) { + if (!isBaseController(controller)) { try { delete this.metadata[name]; delete this.state[name]; @@ -207,9 +169,10 @@ export class ComposableController< // False negative. `name` is a string type. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions `${name}:stateChange`, - (childState: LegacyControllerStateConstraint) => { + (childState: StateConstraint) => { this.update((state) => { - (state as ComposableControllerStateConstraint)[name] = childState; + // @ts-expect-error - Suppressing error to assign new property to generic type + state[name] = childState; }); }, ); @@ -218,13 +181,6 @@ export class ComposableController< // eslint-disable-next-line @typescript-eslint/restrict-template-expressions console.error(`${name} - ${String(error)}`); } - if (isBaseControllerV1(controller)) { - controller.subscribe((childState: StateConstraintV1) => { - this.update((state) => { - (state as ComposableControllerStateConstraint)[name] = childState; - }); - }); - } } }