[composable-controller] Enable handling of non-controller input

packages/base-controller/src/BaseControllerV1.test.ts

@@ -51,7 +51,6 @@ describe('isBaseControllerV1', () => {
 
   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);
   });
 });

packages/base-controller/src/BaseControllerV1.ts

@@ -1,6 +1,4 @@
-import type { PublicInterface } from '@metamask/utils';
-
-import type { ControllerInstance } from './BaseControllerV2';
+import { isNullOrUndefined, type PublicInterface } from '@metamask/utils';
 
 /**
  * Determines if the given controller is an instance of `BaseControllerV1`
@@ -9,9 +7,11 @@ import type { ControllerInstance } from './BaseControllerV2';
  * @returns True if the controller is an instance of `BaseControllerV1`
  */
 export function isBaseControllerV1(
-  controller: ControllerInstance,
+  controller: unknown,
 ): controller is BaseControllerV1Instance {
   return (
+    typeof controller === 'object' &&
+    !isNullOrUndefined(controller) &&
     'name' in controller &&
     typeof controller.name === 'string' &&
     'config' in controller &&

packages/base-controller/src/BaseControllerV2.test.ts

@@ -203,7 +203,6 @@ describe('isBaseController', () => {
 
   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);
   });
 });

packages/base-controller/src/BaseControllerV2.ts

@@ -1,4 +1,8 @@
-import type { Json, PublicInterface } from '@metamask/utils';
+import {
+  isNullOrUndefined,
+  type Json,
+  type PublicInterface,
+} from '@metamask/utils';
 import { enablePatches, produceWithPatches, applyPatches, freeze } from 'immer';
 import type { Draft, Patch } from 'immer';
 
@@ -21,9 +25,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' &&
+    !isNullOrUndefined(controller) &&
     'name' in controller &&
     typeof controller.name === 'string' &&
     'state' in controller &&
@@ -130,7 +136,7 @@ export type StateMetadataConstraint = Record<
 >;
 
 /**
- * The widest subtype of all controller instances that inherit from `BaseController` (formerly `BaseControllerV2`).
+ * The narrowest supertype of all controller instances that inherit from `BaseController` (formerly `BaseControllerV2`).
  * Any `BaseController` subclass instance can be assigned to this type.
  */
 export type BaseControllerInstance = Omit<
@@ -147,7 +153,7 @@ export type BaseControllerInstance = Omit<
 };
 
 /**
- * A widest subtype of all controller instances that inherit from `BaseController` (formerly `BaseControllerV2`) or `BaseControllerV1`.
+ * A narrowest supertype 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.

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

@@ -15,10 +15,7 @@ import type {
   ChildControllerStateChangeEvents,
   ComposableControllerEvents,
 } from './ComposableController';
-import {
-  ComposableController,
-  INVALID_CONTROLLER_ERROR,
-} from './ComposableController';
+import { ComposableController } from './ComposableController';
 
 // Mock BaseController classes
 
@@ -531,7 +528,7 @@ describe('ComposableController', () => {
       ).toThrow('Messaging system is required');
     });
 
-    it('should throw if composing a controller that does not extend from BaseController', () => {
+    it('should not throw if composing a controller that does not extend from BaseController', () => {
       type ComposableControllerState = {
         // TODO: Either fix this lint violation or explain why it's necessary to ignore.
         // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -564,7 +561,7 @@ describe('ComposableController', () => {
             controllers: [notController, fooController],
             messenger: composableControllerMessenger,
           }),
-      ).toThrow(INVALID_CONTROLLER_ERROR);
+      ).not.toThrow();
     });
   });
 });

packages/composable-controller/src/ComposableController.ts

@@ -16,8 +16,32 @@ import type { Patch } from 'immer';
 
 export const controllerName = 'ComposableController';
 
-export const INVALID_CONTROLLER_ERROR =
-  'Invalid controller: controller must have a `messagingSystem` or be a class inheriting from `BaseControllerV1`.';
+// TODO: Replace with type guard once superclass for messageable non-controllers has been defined.
+export const STATELESS_NONCONTROLLER_NAMES = [
+  'AssetsContractController',
+  'NftDetectionController',
+  'TokenDetectionController',
+] as const;
+
+/**
+ * A type guard for the {@link ControllerInstance} type that specifically filters out non-controllers that have empty state.
+ * @param controller - The controller instance to check.
+ * @returns A type predicate that evaluates to true if the input is a {@link ControllerInstance} type.
+ */
+export function isController(
+  controller: unknown,
+): controller is ControllerInstance {
+  return (
+    (isBaseController(controller) || isBaseControllerV1(controller)) &&
+    !STATELESS_NONCONTROLLER_NAMES.find((name) => name === controller.name)
+  );
+}
+
+/**
+ * A universal supertype for modules with a 'string'-type `name` property.
+ * This type is intended to encompass controller and non-controller input that can be passed into the {@link ComposableController} `controllers` constructor option.
+ */
+export type NamedModule = { name: string };
 
 /**
  * A universal supertype for the composable controller state object.
@@ -30,7 +54,7 @@ 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<string, any>;
+  [controllerName: string]: Record<string, any>;
 };
 
 /**
@@ -39,7 +63,7 @@ type LegacyComposableControllerStateConstraint = {
  */
 // TODO: Replace with `{ [name: string]: StateConstraint }` once BaseControllerV2 migrations are completed for all controllers.
 export type ComposableControllerStateConstraint = {
-  [name: string]: LegacyControllerStateConstraint;
+  [controllerName: string]: LegacyControllerStateConstraint;
 };
 
 /**
@@ -125,12 +149,12 @@ export type ComposableControllerMessenger<
 /**
  * Controller that composes multiple child controllers and maintains up-to-date composed state.
  *
- * @template ComposableControllerState - A type object containing the names and state types of the child controllers.
- * @template ChildControllers - A union type of the child controllers being used to instantiate the {@link ComposableController}.
+ * @template ComposableControllerState - A type object containing the names and state types of the child controllers. Any non-controllers with empty state should be omitted from this type argument.
+ * @template ChildControllers - A union type of the child controllers being used to instantiate the {@link ComposableController}. Non-controllers that are passed into the `controllers` constructor option at runtime should be included in this type argument.
  */
 export class ComposableController<
   ComposableControllerState extends LegacyComposableControllerStateConstraint,
-  ChildControllers extends ControllerInstance,
+  ChildControllers extends NamedModule,
 > extends BaseController<
   typeof controllerName,
   ComposableControllerState,
@@ -140,7 +164,7 @@ export class ComposableController<
    * Creates a ComposableController instance.
    *
    * @param options - Initial options used to configure this controller
-   * @param options.controllers - List of child controller instances to compose.
+   * @param options.controllers - List of child controller instances to compose. Any non-controllers that are included here will be excluded from the composed state object and from `stateChange` event subscription.
    * @param options.messenger - A restricted controller messenger.
    */
 
@@ -158,18 +182,27 @@ export class ComposableController<
     super({
       name: controllerName,
       metadata: controllers.reduce<StateMetadata<ComposableControllerState>>(
-        (metadata, controller) => ({
-          ...metadata,
-          [controller.name]: isBaseController(controller)
-            ? controller.metadata
-            : { persist: true, anonymous: true },
-        }),
+        (metadata, controller) => {
+          if (!isController(controller)) {
+            return metadata;
+          }
+          return Object.assign(
+            metadata,
+            isBaseController(controller)
+              ? { [controller.name]: controller.metadata }
+              : { [controller.name]: { persist: true, anonymous: true } },
+          );
+        },
         {} as never,
       ),
       state: controllers.reduce<ComposableControllerState>(
-        (state, controller) => {
-          return { ...state, [controller.name]: controller.state };
-        },
+        (state, controller) =>
+          Object.assign(
+            state,
+            isController(controller)
+              ? { [controller.name]: controller.state }
+              : {},
+          ),
         {} as never,
       ),
       messenger,
@@ -185,31 +218,25 @@ export class ComposableController<
    *
    * @param controller - Controller instance to update
    */
-  #updateChildController(controller: ControllerInstance): void {
-    const { name } = controller;
-    if (!isBaseController(controller) && !isBaseControllerV1(controller)) {
-      // False negative. `name` is a string type.
-      // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
-      throw new Error(`${name} - ${INVALID_CONTROLLER_ERROR}`);
+  #updateChildController(controller: ChildControllers): void {
+    if (!isController(controller)) {
+      return;
     }
+    const { name } = controller;
     try {
       this.messagingSystem.subscribe(
-        // False negative. `name` is a string type.
-        // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
         `${name}:stateChange`,
         (childState: LegacyControllerStateConstraint) => {
           this.update((state) => {
             Object.assign(state, { [name]: childState });
           });
         },
       );
-    } catch (error: unknown) {
-      // False negative. `name` is a string type.
-      // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
-      console.error(`${name} - ${String(error)}`);
-    }
+      // Invalid/non-existent event names from V1 controllers and non-controllers are expected, and should be handled without blocking ComposableController instantiation in downstream clients.
+      // eslint-disable-next-line no-empty
+    } catch (error: unknown) {}
     if (isBaseControllerV1(controller)) {
-      controller.subscribe((childState: StateConstraintV1) => {
+      controller.subscribe((childState) => {
         this.update((state) => {
           Object.assign(state, { [name]: childState });
         });

packages/composable-controller/src/index.ts

@@ -4,4 +4,7 @@ export type {
   ComposableControllerEvents,
   ComposableControllerMessenger,
 } from './ComposableController';
-export { ComposableController } from './ComposableController';
+export {
+  ComposableController,
+  STATELESS_NONCONTROLLER_NAMES,
+} from './ComposableController';