Skip to content

Commit 1a452ef

Browse files
MajorLiftMajorLift
committed
stash
1 parent a3a060c commit 1a452ef

File tree

2 files changed

+132
-75
lines changed

2 files changed

+132
-75
lines changed

‎packages/composable-controller/src/ComposableController.test.ts

+55-17
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,21 @@ class BazController extends BaseControllerV1<never, BazControllerState> {
144144
}
145145
}
146146

147+
type ControllersMap = {
148+
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
149+
// eslint-disable-next-line @typescript-eslint/naming-convention
150+
FooController: FooController;
151+
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
152+
// eslint-disable-next-line @typescript-eslint/naming-convention
153+
QuzController: QuzController;
154+
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
155+
// eslint-disable-next-line @typescript-eslint/naming-convention
156+
BarController: BarController;
157+
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
158+
// eslint-disable-next-line @typescript-eslint/naming-convention
159+
BazController: BazController;
160+
};
161+
147162
describe('ComposableController', () => {
148163
afterEach(() => {
149164
sinon.restore();
@@ -159,6 +174,7 @@ describe('ComposableController', () => {
159174
// eslint-disable-next-line @typescript-eslint/naming-convention
160175
BazController: BazControllerState;
161176
};
177+
162178
const composableMessenger = new ControllerMessenger<
163179
never,
164180
ComposableControllerEvents<ComposableControllerState>
@@ -167,7 +183,10 @@ describe('ComposableController', () => {
167183
allowedActions: [],
168184
allowedEvents: [],
169185
});
170-
const controller = new ComposableController({
186+
const controller = new ComposableController<
187+
ComposableControllerState,
188+
ControllersMap[keyof ComposableControllerState]
189+
>({
171190
controllers: [new BarController(), new BazController()],
172191
messenger: composableMessenger,
173192
});
@@ -194,7 +213,10 @@ describe('ComposableController', () => {
194213
allowedEvents: [],
195214
});
196215
const barController = new BarController();
197-
new ComposableController({
216+
new ComposableController<
217+
ComposableControllerState,
218+
ControllersMap[keyof ComposableControllerState]
219+
>({
198220
controllers: [barController],
199221
messenger: composableMessenger,
200222
});
@@ -255,11 +277,13 @@ describe('ComposableController', () => {
255277
'QuzController:stateChange',
256278
],
257279
});
258-
const composableController =
259-
new ComposableController<ComposableControllerState>({
260-
controllers: [fooController, quzController],
261-
messenger: composableControllerMessenger,
262-
});
280+
const composableController = new ComposableController<
281+
ComposableControllerState,
282+
ControllersMap[keyof ComposableControllerState]
283+
>({
284+
controllers: [fooController, quzController],
285+
messenger: composableControllerMessenger,
286+
});
263287
expect(composableController.state).toStrictEqual({
264288
FooController: { foo: 'foo' },
265289
QuzController: { quz: 'quz' },
@@ -288,7 +312,10 @@ describe('ComposableController', () => {
288312
allowedActions: [],
289313
allowedEvents: ['FooController:stateChange'],
290314
});
291-
new ComposableController<ComposableControllerState>({
315+
new ComposableController<
316+
ComposableControllerState,
317+
ControllersMap[keyof ComposableControllerState]
318+
>({
292319
controllers: [fooController],
293320
messenger: composableControllerMessenger,
294321
});
@@ -336,11 +363,13 @@ describe('ComposableController', () => {
336363
allowedActions: [],
337364
allowedEvents: ['FooController:stateChange'],
338365
});
339-
const composableController =
340-
new ComposableController<ComposableControllerState>({
341-
controllers: [barController, fooController],
342-
messenger: composableControllerMessenger,
343-
});
366+
const composableController = new ComposableController<
367+
ComposableControllerState,
368+
ControllersMap[keyof ComposableControllerState]
369+
>({
370+
controllers: [barController, fooController],
371+
messenger: composableControllerMessenger,
372+
});
344373
expect(composableController.state).toStrictEqual({
345374
BarController: { bar: 'bar' },
346375
FooController: { foo: 'foo' },
@@ -373,7 +402,10 @@ describe('ComposableController', () => {
373402
allowedActions: [],
374403
allowedEvents: ['FooController:stateChange'],
375404
});
376-
new ComposableController<ComposableControllerState>({
405+
new ComposableController<
406+
ComposableControllerState,
407+
ControllersMap[keyof ComposableControllerState]
408+
>({
377409
controllers: [barController, fooController],
378410
messenger: composableControllerMessenger,
379411
});
@@ -421,7 +453,10 @@ describe('ComposableController', () => {
421453
allowedActions: [],
422454
allowedEvents: ['FooController:stateChange'],
423455
});
424-
new ComposableController<ComposableControllerState>({
456+
new ComposableController<
457+
ComposableControllerState,
458+
ControllersMap[keyof ComposableControllerState]
459+
>({
425460
controllers: [barController, fooController],
426461
messenger: composableControllerMessenger,
427462
});
@@ -490,13 +525,16 @@ describe('ComposableController', () => {
490525
});
491526
expect(
492527
() =>
493-
new ComposableController({
528+
new ComposableController<
529+
ComposableControllerState,
530+
ControllersMap[keyof ComposableControllerState]
531+
>({
494532
// @ts-expect-error - Suppressing type error to test for runtime error handling
495533
controllers: [notController, fooController],
496534
messenger: composableControllerMessenger,
497535
}),
498536
).toThrow(
499-
'Invalid controller: controller must extend from BaseController or BaseControllerV1',
537+
'Invalid component: component must be a MessengerConsumer or a controller inheriting from BaseControllerV1.',
500538
);
501539
});
502540
});

‎packages/composable-controller/src/ComposableController.ts

+77-58
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { BaseController, BaseControllerV1 } from '@metamask/base-controller';
21
import type {
32
ActionConstraint,
43
BaseConfig,
@@ -8,22 +7,34 @@ import type {
87
StateConstraint,
98
StateMetadata,
109
ControllerStateChangeEvent,
10+
Listener,
1111
} from '@metamask/base-controller';
12+
import { BaseController, BaseControllerV1 } from '@metamask/base-controller';
1213
import type { Patch } from 'immer';
1314

1415
export const controllerName = 'ComposableController';
1516

17+
type MessengerConsumerInstance = {
18+
name: string;
19+
messagingSystem: RestrictedControllerMessengerConstraint;
20+
};
21+
1622
/**
1723
* A universal subtype of all controller instances that extend from `BaseControllerV1`.
1824
* Any `BaseControllerV1` instance can be assigned to this type.
1925
*
2026
* Note that this type is not the widest subtype or narrowest supertype of all `BaseControllerV1` instances.
2127
* This type is therefore unsuitable for general use as a type constraint, and is only intended for use within the ComposableController.
2228
*/
23-
export type BaseControllerV1Instance =
24-
// `any` is used so that all `BaseControllerV1` instances are assignable to this type.
25-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
26-
BaseControllerV1<any, any>;
29+
export type BaseControllerV1Instance = {
30+
name: string;
31+
config: BaseConfig & object;
32+
defaultConfig: BaseConfig & object;
33+
state: BaseState & object;
34+
defaultState: BaseState & object;
35+
subscribe: (listener: Listener<BaseState & object>) => void;
36+
disabled: boolean;
37+
} & Partial<Pick<MessengerConsumerInstance, 'messagingSystem'>>;
2738

2839
/**
2940
* A universal subtype of all controller instances that extend from `BaseController` (formerly `BaseControllerV2`).
@@ -34,9 +45,12 @@ export type BaseControllerV1Instance =
3445
*
3546
* For this reason, we only look for `BaseController` properties that we use in the ComposableController (name and state).
3647
*/
37-
export type BaseControllerInstance = {
48+
export type BaseControllerInstance<
49+
State extends StateConstraint = StateConstraint,
50+
> = {
3851
name: string;
39-
state: StateConstraint;
52+
state: State;
53+
metadata: Record<string, unknown>;
4054
};
4155

4256
/**
@@ -46,42 +60,63 @@ export type BaseControllerInstance = {
4660
* Note that this type is not the widest subtype or narrowest supertype of all `BaseController` and `BaseControllerV1` instances.
4761
* This type is therefore unsuitable for general use as a type constraint, and is only intended for use within the ComposableController.
4862
*/
49-
export type ControllerInstance =
63+
export type WalletComponentInstance =
5064
| BaseControllerV1Instance
51-
| BaseControllerInstance;
65+
| BaseControllerInstance
66+
| MessengerConsumerInstance;
67+
68+
export type ControllerInstance = Exclude<
69+
WalletComponentInstance,
70+
MessengerConsumerInstance
71+
>;
5272

5373
/**
5474
* The narrowest supertype of all `RestrictedControllerMessenger` instances.
5575
*/
56-
export type RestrictedControllerMessengerConstraint =
57-
RestrictedControllerMessenger<
58-
string,
59-
ActionConstraint,
60-
EventConstraint,
61-
string,
62-
string
63-
>;
76+
export type RestrictedControllerMessengerConstraint<
77+
ControllerName extends string = string,
78+
> = RestrictedControllerMessenger<
79+
ControllerName,
80+
ActionConstraint,
81+
EventConstraint,
82+
string,
83+
string
84+
>;
85+
86+
/**
87+
* Determines if the given class has a messaging system.
88+
* @param component - Component instance to check
89+
* @returns True if the component is an instance of `MessengerConsumerInstance`
90+
*/
91+
export function isMessengerConsumer(
92+
component: WalletComponentInstance,
93+
): component is MessengerConsumerInstance {
94+
return 'name' in component && 'messagingSystem' in component;
95+
}
6496

6597
/**
6698
* Determines if the given controller is an instance of `BaseControllerV1`
6799
* @param controller - Controller instance to check
68100
* @returns True if the controller is an instance of `BaseControllerV1`
69101
*/
70102
export function isBaseControllerV1(
71-
controller: ControllerInstance,
72-
): controller is BaseControllerV1<
73-
BaseConfig & Record<string, unknown>,
74-
BaseState & Record<string, unknown>
75-
> {
103+
controller: WalletComponentInstance,
104+
): controller is BaseControllerV1Instance {
76105
return (
77106
'name' in controller &&
78107
typeof controller.name === 'string' &&
108+
'config' in controller &&
109+
typeof controller.config === 'object' &&
79110
'defaultConfig' in controller &&
80111
typeof controller.defaultConfig === 'object' &&
112+
'state' in controller &&
113+
typeof controller.state === 'object' &&
81114
'defaultState' in controller &&
82115
typeof controller.defaultState === 'object' &&
83116
'disabled' in controller &&
84117
typeof controller.disabled === 'boolean' &&
118+
'subscribe' in controller &&
119+
typeof controller.subscribe === 'function' &&
85120
controller instanceof BaseControllerV1
86121
);
87122
}
@@ -92,17 +127,15 @@ export function isBaseControllerV1(
92127
* @returns True if the controller is an instance of `BaseController`
93128
*/
94129
export function isBaseController(
95-
controller: ControllerInstance,
96-
): controller is BaseController<
97-
string,
98-
StateConstraint,
99-
RestrictedControllerMessengerConstraint
100-
> {
130+
controller: WalletComponentInstance,
131+
): controller is BaseControllerInstance {
101132
return (
102133
'name' in controller &&
103134
typeof controller.name === 'string' &&
104135
'state' in controller &&
105136
typeof controller.state === 'object' &&
137+
'metadata' in controller &&
138+
typeof controller.metadata === 'object' &&
106139
controller instanceof BaseController
107140
);
108141
}
@@ -186,25 +219,13 @@ export type ComposableControllerMessenger<
186219
AllowedEvents<ComposableControllerState>['type']
187220
>;
188221

189-
type GetChildControllers<
190-
ComposableControllerState,
191-
ControllerName extends keyof ComposableControllerState = keyof ComposableControllerState,
192-
> = ControllerName extends string
193-
? ComposableControllerState[ControllerName] extends StateConstraint
194-
? { name: ControllerName; state: ComposableControllerState[ControllerName] }
195-
: BaseControllerV1<
196-
BaseConfig & Record<string, unknown>,
197-
BaseState & ComposableControllerState[ControllerName]
198-
>
199-
: never;
200-
201222
/**
202223
* Controller that can be used to compose multiple controllers together.
203224
* @template ChildControllerState - The composed state of the child controllers that are being used to instantiate the composable controller.
204225
*/
205226
export class ComposableController<
206227
ComposableControllerState extends LegacyComposableControllerStateConstraint,
207-
ChildControllers extends ControllerInstance = GetChildControllers<ComposableControllerState>,
228+
ChildControllers extends WalletComponentInstance,
208229
> extends BaseController<
209230
typeof controllerName,
210231
ComposableControllerState,
@@ -242,7 +263,9 @@ export class ComposableController<
242263
),
243264
state: controllers.reduce<ComposableControllerState>(
244265
(state, controller) => {
245-
return { ...state, [controller.name]: controller.state };
266+
return 'state' in controller
267+
? { ...state, [controller.name]: controller.state }
268+
: state;
246269
},
247270
{} as never,
248271
),
@@ -256,36 +279,32 @@ export class ComposableController<
256279

257280
/**
258281
* Constructor helper that subscribes to child controller state changes.
259-
* @param controller - Controller instance to update
282+
* @param component - Wallet component instance to update
260283
*/
261-
#updateChildController(controller: ControllerInstance): void {
262-
if (!isBaseController(controller) && !isBaseControllerV1(controller)) {
263-
throw new Error(
264-
'Invalid controller: controller must extend from BaseController or BaseControllerV1',
265-
);
266-
}
267-
268-
const { name } = controller;
284+
#updateChildController(component: WalletComponentInstance): void {
285+
const { name } = component;
269286
if (
270-
(isBaseControllerV1(controller) && 'messagingSystem' in controller) ||
271-
isBaseController(controller)
287+
isBaseController(component) ||
288+
(isBaseControllerV1(component) && isMessengerConsumer(component))
272289
) {
273290
this.messagingSystem.subscribe(
274-
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
275-
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
276291
`${name}:stateChange`,
277-
(childState: Record<string, unknown>) => {
292+
(childState: StateConstraint | (BaseState & object)) => {
278293
this.update((state) => {
279294
Object.assign(state, { [name]: childState });
280295
});
281296
},
282297
);
283-
} else if (isBaseControllerV1(controller)) {
284-
controller.subscribe((childState) => {
298+
} else if (isBaseControllerV1(component)) {
299+
component.subscribe((childState: BaseState & object) => {
285300
this.update((state) => {
286301
Object.assign(state, { [name]: childState });
287302
});
288303
});
304+
} else if (!isMessengerConsumer(component)) {
305+
throw new Error(
306+
'Invalid component: component must be a MessengerConsumer or a controller inheriting from BaseControllerV1.',
307+
);
289308
}
290309
}
291310
}

0 commit comments

Comments
 (0)