From 5f59bf96bb209aaf6d6355bf0e0b9a5187c0b68f Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 23 Apr 2024 10:43:16 -0400 Subject: [PATCH] stash --- .../src/ComposableController.test.ts | 187 +++++++++++++----- .../src/ComposableController.ts | 128 ++++++------ 2 files changed, 204 insertions(+), 111 deletions(-) diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index d0696e10a07..a043187855e 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'], @@ -460,4 +474,79 @@ describe('ComposableController', () => { ); }); }); + + let barController: BaseControllerV1; + let fooController: BaseController< + 'FooController', + FooControllerState, + FooMessenger + >; + type ComposableControllerState = { + BarController: BarControllerState; + FooController: FooControllerState; + }; + let controllerMessenger: ControllerMessenger< + never, + ComposableControllerEvents | FooControllerEvent + >; + beforeEach(() => { + barController = new BarController(); + controllerMessenger = new ControllerMessenger(); + const fooControllerMessenger = controllerMessenger.getRestricted({ + name: 'FooController', + allowedActions: [], + allowedEvents: [], + }); + fooController = new FooController(fooControllerMessenger); + }); + describe('messenger and child controllers type validation', () => { + it('should raise a type error if the provided controller messenger does not allow one or more required events', () => { + const composableControllerMessenger = controllerMessenger.getRestricted({ + name: 'ComposableController', + allowedActions: [], + allowedEvents: [], + }); + new ComposableController({ + controllers: [barController, fooController], + messenger: composableControllerMessenger, + }); + }); + }); + + it('should raise a type error if the provided controller messenger allows events that should not be allowed', () => { + const composableControllerMessenger = controllerMessenger.getRestricted({ + name: 'ComposableController', + allowedActions: [], + allowedEvents: ['FooController:stateChange', 'BazController:stateChange'], + }); + new ComposableController({ + controllers: [barController, fooController], + messenger: composableControllerMessenger, + }); + }); + + it('should raise a type error if the provided controllers array contains a controller that is not included in `ComposableControllerState`', () => { + const composableControllerMessenger = controllerMessenger.getRestricted({ + name: 'ComposableController', + allowedActions: [], + allowedEvents: ['FooController:stateChange'], + }); + const bazController = new BazController(); + new ComposableController({ + controllers: [barController, fooController, bazController], + messenger: composableControllerMessenger, + }); + }); + + it('should raise a type error if the provided controllers array does not contain a controller that is included in `ComposableControllerState`', () => { + const composableControllerMessenger = controllerMessenger.getRestricted({ + name: 'ComposableController', + allowedActions: [], + allowedEvents: ['FooController:stateChange'], + }); + new ComposableController({ + controllers: [barController], + messenger: composableControllerMessenger, + }); + }); }); diff --git a/packages/composable-controller/src/ComposableController.ts b/packages/composable-controller/src/ComposableController.ts index 74dfe6aeb86..48c7515193c 100644 --- a/packages/composable-controller/src/ComposableController.ts +++ b/packages/composable-controller/src/ComposableController.ts @@ -51,15 +51,16 @@ export type ControllerInstance = | BaseControllerInstance; /** - * A narrowest supertype of all `RestrictedControllerMessenger` instances. + * The narrowest supertype of all `RestrictedControllerMessenger` instances. */ -type RestrictedControllerMessengerConstraint = RestrictedControllerMessenger< - string, - ActionConstraint, - EventConstraint, - string, - string ->; +export type RestrictedControllerMessengerConstraint = + RestrictedControllerMessenger< + string, + ActionConstraint, + EventConstraint, + string, + string + >; /** * Determines if the given controller is an instance of `BaseControllerV1` @@ -106,89 +107,92 @@ 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 - | (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; +export type ComposableControllerStateChangeEvent< + ComposableControllerState extends ComposableControllerStateConstraint, +> = LegacyControllerStateChangeEvent< + typeof controllerName, + ComposableControllerState +>; + +export type ComposableControllerEvents< + ComposableControllerState extends ComposableControllerStateConstraint, +> = ComposableControllerStateChangeEvent; -type AnyControllerStateChangeEvent = { - type: `${string}:stateChange`; - payload: [ControllerInstance['state'], Patch[]]; -}; +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 = - Controller extends BaseControllerV1Instance - ? { - type: `${Controller['name']}:stateChange`; - payload: [Controller['state'], Patch[]]; - } - : ControllerStateChangeEvent; - /** * 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< - 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, - ComposedControllerState, - ComposedControllerMessenger + ComposableControllerState, + ComposableControllerMessenger > { /** * Creates a ComposableController instance. @@ -202,8 +206,8 @@ export class ComposableController< controllers, messenger, }: { - controllers: ChildControllers[]; - messenger: ComposedControllerMessenger; + controllers: ControllerInstance[]; + messenger: ComposableControllerMessenger; }) { if (messenger === undefined) { throw new Error(`Messaging system is required`); @@ -211,7 +215,7 @@ export class ComposableController< super({ name: controllerName, - metadata: controllers.reduce>( + metadata: controllers.reduce>( (metadata, controller) => ({ ...metadata, [controller.name]: isBaseController(controller) @@ -220,7 +224,7 @@ export class ComposableController< }), {} as never, ), - state: controllers.reduce( + state: controllers.reduce( (state, controller) => { return { ...state, [controller.name]: controller.state }; },