Skip to content

Commit

Permalink
Move composable-controller utility types to base-controller (#4581)
Browse files Browse the repository at this point in the history
## Explanation

This commit moves `BaseController`-related types and functions in
`@metamask/composable-controller` to `@metamask/base-controller`.

Because applying these changes requires a concurrent major update of
`@metamask/base-controller`, this commit will be excluded from
`@metamask/composable-controller@8.0.0`
(#4467), so that complications can
be avoided while applying `8.0.0` to mobile.

## References

- Blocked by #4467
- Blocked by MetaMask/metamask-mobile#10441

## Changelog

### `@metamask/base-controller` (minor)

### Added

- Migrate from `@metamask/composable-controller@8.0.0` into
`@metamask/base-controller`: types `LegacyControllerStateConstraint`,
`RestrictedControllerMessengerConstraint` and type guard functions
`isBaseController`, `isBaseControllerV1`
([#4581](#4581))
- Add and export types `ControllerInstance`, `BaseControllerInstance`,
`StateDeriverConstraint`, `StateMetadataConstraint`,
`StatePropertyMetadataConstraint`, `BaseControllerV1Instance`,
`ConfigConstraintV1`, `StateConstraintV1`
([#4581](#4581))

### `@metamask/composable-controller` (major)

### Removed

- **BREAKING:** Remove exports for types
`LegacyControllerStateConstraint`,
`RestrictedControllerMessengerConstraint`, and type guard functions
`isBaseController`, `isBaseControllerV1`
([#4467](#4467))
  - These have been migrated to `@metamask/base-controller@6.2.0`.

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
  • Loading branch information
MajorLift authored Aug 21, 2024
1 parent 72425c3 commit c6f7e02
Show file tree
Hide file tree
Showing 13 changed files with 284 additions and 168 deletions.
2 changes: 1 addition & 1 deletion packages/base-controller/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 96.29,
branches: 100,
functions: 100,
lines: 100,
statements: 100,
Expand Down
1 change: 1 addition & 0 deletions packages/base-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
},
"devDependencies": {
"@metamask/auto-changelog": "^3.4.4",
"@metamask/json-rpc-engine": "^9.0.2",
"@types/jest": "^27.4.1",
"@types/sinon": "^9.0.10",
"deepmerge": "^4.2.2",
Expand Down
47 changes: 45 additions & 2 deletions packages/base-controller/src/BaseControllerV1.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,61 @@
import { JsonRpcEngine } from '@metamask/json-rpc-engine';
import * as sinon from 'sinon';

import type { BaseConfig, BaseState } from './BaseControllerV1';
import { BaseControllerV1 as BaseController } 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 };

class TestController extends BaseController<BaseConfig, BaseState> {
// eslint-disable-next-line jest/no-export
export class TestController extends BaseController<BaseConfig, BaseState> {
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();
Expand Down
51 changes: 51 additions & 0 deletions packages/base-controller/src/BaseControllerV1.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,34 @@
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
*/
Expand Down Expand Up @@ -31,6 +62,26 @@ 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<ConfigConstraint, StateConstraint>
>;

/**
* @deprecated This class has been renamed to BaseControllerV1 and is no longer recommended for use for controllers. Please use BaseController (formerly BaseControllerV2) instead.
*
Expand Down
43 changes: 37 additions & 6 deletions packages/base-controller/src/BaseControllerV2.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
/* eslint-disable jest/no-export */
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,
Expand All @@ -9,27 +12,28 @@ import {
BaseController,
getAnonymizedState,
getPersistentState,
isBaseController,
} from './BaseControllerV2';
import { ControllerMessenger } from './ControllerMessenger';
import type { RestrictedControllerMessenger } from './RestrictedControllerMessenger';

const countControllerName = 'CountController';
export const countControllerName = 'CountController';

type CountControllerState = {
count: number;
};

type CountControllerAction = ControllerGetStateAction<
export type CountControllerAction = ControllerGetStateAction<
typeof countControllerName,
CountControllerState
>;

type CountControllerEvent = ControllerStateChangeEvent<
export type CountControllerEvent = ControllerStateChangeEvent<
typeof countControllerName,
CountControllerState
>;

const countControllerStateMetadata = {
export const countControllerStateMetadata = {
count: {
persist: true,
anonymous: true,
Expand All @@ -50,7 +54,7 @@ type CountMessenger = RestrictedControllerMessenger<
* @param controllerMessenger - The controller messenger.
* @returns A restricted controller messenger for the Count controller.
*/
function getCountMessenger(
export function getCountMessenger(
controllerMessenger?: ControllerMessenger<
CountControllerAction,
CountControllerEvent
Expand All @@ -69,7 +73,7 @@ function getCountMessenger(
});
}

class CountController extends BaseController<
export class CountController extends BaseController<
typeof countControllerName,
CountControllerState,
CountMessenger
Expand Down Expand Up @@ -177,6 +181,33 @@ class MessagesController extends BaseController<
}
}

describe('isBaseController', () => {
it('should return true 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(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);
});
});

describe('BaseController', () => {
afterEach(() => {
sinon.restore();
Expand Down
87 changes: 85 additions & 2 deletions packages/base-controller/src/BaseControllerV2.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,53 @@
import type { Json } from '@metamask/utils';
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 } from './RestrictedControllerMessenger';
import type {
RestrictedControllerMessenger,
RestrictedControllerMessengerConstraint,
} from './RestrictedControllerMessenger';

enablePatches();

/**
* Determines if the given controller is an instance of `BaseController`
*
* @param controller - Controller instance to check
* @returns True if the controller is an instance of `BaseController`
*/
export function isBaseController(
controller: ControllerInstance,
): controller is BaseControllerInstance {
return (
'name' in controller &&
typeof controller.name === 'string' &&
'state' in controller &&
typeof controller.state === 'object' &&
'metadata' in controller &&
typeof controller.metadata === 'object'
);
}

/**
* A type that constrains the state of all controllers.
*
* In other words, the narrowest supertype encompassing all controller state.
*/
export type StateConstraint = Record<string, Json>;

/**
* 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.
*
Expand Down Expand Up @@ -72,6 +106,55 @@ export type StatePropertyMetadata<T extends Json> = {
anonymous: boolean | StateDeriver<T>;
};

/**
* A universal supertype of `StateDeriver` types.
* This type can be assigned to any `StateDeriver` type.
*/
export type StateDeriverConstraint = (value: never) => Json;

/**
* A universal supertype of `StatePropertyMetadata` types.
* This type can be assigned to any `StatePropertyMetadata` type.
*/
export type StatePropertyMetadataConstraint = {
[P in 'anonymous' | 'persist']: boolean | StateDeriverConstraint;
};

/**
* A universal supertype of `StateMetadata` types.
* This type can be assigned to any `StateMetadata` type.
*/
export type StateMetadataConstraint = Record<
string,
StatePropertyMetadataConstraint
>;

/**
* The widest subtype of all controller instances that inherit from `BaseController` (formerly `BaseControllerV2`).
* Any `BaseController` subclass instance can be assigned to this type.
*/
export type BaseControllerInstance = Omit<
PublicInterface<
BaseController<
string,
StateConstraint,
RestrictedControllerMessengerConstraint
>
>,
'metadata'
> & {
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,
Expand Down
17 changes: 17 additions & 0 deletions packages/base-controller/src/RestrictedControllerMessenger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@ import type {
SelectorFunction,
} from './ControllerMessenger';

/**
* A universal supertype of all `RestrictedControllerMessenger` instances.
* This type can be assigned to any `RestrictedControllerMessenger` type.
*
* @template ControllerName - Name of the controller. Optionally can be used to
* narrow this type to a constraint for the messenger of a specific controller.
*/
export type RestrictedControllerMessengerConstraint<
ControllerName extends string = string,
> = RestrictedControllerMessenger<
ControllerName,
ActionConstraint,
EventConstraint,
string,
string
>;

/**
* A restricted controller messenger.
*
Expand Down
Loading

0 comments on commit c6f7e02

Please sign in to comment.