Skip to content

Commit

Permalink
Deprecate BaseControllerV1 and use BaseControllerV2 as default (#…
Browse files Browse the repository at this point in the history
…2078)

## Motivation

The controller-messenger pattern implemented by `BaseControllerV2` is
intended to be used by all of our controllers. Currently, the default
`BaseController` name points to the older base controller
implementation, which needs to be deprecated and only used for temporary
backwards compatibility while all controllers are brought up-to-date.

## Explanation

In this commit, we export `BaseControllerV2` as the default
`BaseController`, and add a `@deprecated` tsdoc tag to the old
`BaseController` class as well as export it under the new name
`BaseControllerV1` to discourage future usage.

This aligns with the `PollingController` naming scheme, where the
up-to-date version is the default, and outdated mixins are named with
qualifiers (`PollingControllerOnly`, `PollingControllerV1`).

## Impact

Existing controller classes in core that extend from the old base
controller are not affected — they now simply extend from
`BaseControllerV1` instead of `BaseController`.

External packages will need to update any instances of `extends
BaseController` to `extends BaseControllerV1` and `extends
BaseControllerV2` to `extends BaseController`, along with accompanying
changes to import statements.

---------

Co-authored-by: Elliot Winkler <[email protected]>
  • Loading branch information
MajorLift and mcmire authored Nov 22, 2023
1 parent 9c6e19d commit be55ceb
Show file tree
Hide file tree
Showing 32 changed files with 87 additions and 85 deletions.
4 changes: 2 additions & 2 deletions packages/accounts-controller/src/AccountsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type {
ControllerStateChangeEvent,
RestrictedControllerMessenger,
} from '@metamask/base-controller';
import { BaseControllerV2 } from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { SnapKeyring } from '@metamask/eth-snap-keyring';
import type { InternalAccount } from '@metamask/keyring-api';
import { EthAccountType } from '@metamask/keyring-api';
Expand Down Expand Up @@ -133,7 +133,7 @@ const defaultState: AccountsControllerState = {
* The accounts controller also listens for snap state changes and updates the internal accounts accordingly.
*
*/
export class AccountsController extends BaseControllerV2<
export class AccountsController extends BaseController<
typeof controllerName,
AccountsControllerState,
AccountsControllerMessenger
Expand Down
4 changes: 2 additions & 2 deletions packages/address-book-controller/src/AddressBookController.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { BaseConfig, BaseState } from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { BaseControllerV1 } from '@metamask/base-controller';
import {
normalizeEnsName,
isValidHexAddress,
Expand Down Expand Up @@ -70,7 +70,7 @@ export interface AddressBookState extends BaseState {
/**
* Controller that manages a list of recipient addresses associated with nicknames.
*/
export class AddressBookController extends BaseController<
export class AddressBookController extends BaseControllerV1<
BaseConfig,
AddressBookState
> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type {
ControllerStateChangeEvent,
RestrictedControllerMessenger,
} from '@metamask/base-controller';
import { BaseControllerV2 } from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';

type ViewedAnnouncement = {
[id: number]: boolean;
Expand Down Expand Up @@ -77,7 +77,7 @@ export type AnnouncementControllerMessenger = RestrictedControllerMessenger<
/**
* Controller for managing in-app announcements.
*/
export class AnnouncementController extends BaseControllerV2<
export class AnnouncementController extends BaseController<
typeof controllerName,
AnnouncementControllerState,
AnnouncementControllerMessenger
Expand Down
4 changes: 2 additions & 2 deletions packages/approval-controller/src/ApprovalController.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { ControllerGetStateAction } from '@metamask/base-controller';
import {
BaseControllerV2,
BaseController,
type ControllerStateChangeEvent,
type RestrictedControllerMessenger,
} from '@metamask/base-controller';
Expand Down Expand Up @@ -338,7 +338,7 @@ export type ApprovalControllerActions =
* Adding a request returns a promise that resolves or rejects when the request
* is approved or denied, respectively.
*/
export class ApprovalController extends BaseControllerV2<
export class ApprovalController extends BaseController<
typeof controllerName,
ApprovalControllerState,
ApprovalControllerMessenger
Expand Down
4 changes: 2 additions & 2 deletions packages/assets-controllers/src/AccountTrackerController.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { BaseConfig, BaseState } from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { BaseControllerV1 } from '@metamask/base-controller';
import {
BNToHex,
query,
Expand Down Expand Up @@ -54,7 +54,7 @@ export interface AccountTrackerState extends BaseState {
/**
* Controller that tracks the network balances for all user accounts.
*/
export class AccountTrackerController extends BaseController<
export class AccountTrackerController extends BaseControllerV1<
AccountTrackerConfig,
AccountTrackerState
> {
Expand Down
4 changes: 2 additions & 2 deletions packages/assets-controllers/src/AssetsContractController.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Contract } from '@ethersproject/contracts';
import { Web3Provider } from '@ethersproject/providers';
import type { BaseConfig, BaseState } from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { BaseControllerV1 } from '@metamask/base-controller';
import { IPFS_DEFAULT_GATEWAY_URL } from '@metamask/controller-utils';
import type {
NetworkClientId,
Expand Down Expand Up @@ -75,7 +75,7 @@ export interface BalanceMap {
/**
* Controller that interacts with contracts on mainnet through web3
*/
export class AssetsContractController extends BaseController<
export class AssetsContractController extends BaseControllerV1<
AssetsContractConfig,
BaseState
> {
Expand Down
4 changes: 2 additions & 2 deletions packages/assets-controllers/src/NftController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type {
BaseState,
RestrictedControllerMessenger,
} from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { BaseControllerV1 } from '@metamask/base-controller';
import {
safelyExecute,
handleFetch,
Expand Down Expand Up @@ -226,7 +226,7 @@ export type NftControllerMessenger = RestrictedControllerMessenger<
/**
* Controller that stores assets and exposes convenience methods
*/
export class NftController extends BaseController<NftConfig, NftState> {
export class NftController extends BaseControllerV1<NftConfig, NftState> {
private readonly mutex = new Mutex();

private readonly messagingSystem: NftControllerMessenger;
Expand Down
4 changes: 2 additions & 2 deletions packages/assets-controllers/src/TokenBalancesController.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { BaseConfig, BaseState } from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { BaseControllerV1 } from '@metamask/base-controller';
import { safelyExecute } from '@metamask/controller-utils';
import type { PreferencesState } from '@metamask/preferences-controller';
import { BN } from 'ethereumjs-util';
Expand Down Expand Up @@ -43,7 +43,7 @@ export interface TokenBalancesState extends BaseState {
* Controller that passively polls on a set interval token balances
* for tokens stored in the TokensController
*/
export class TokenBalancesController extends BaseController<
export class TokenBalancesController extends BaseControllerV1<
TokenBalancesConfig,
TokenBalancesState
> {
Expand Down
4 changes: 2 additions & 2 deletions packages/assets-controllers/src/TokensController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {
BaseState,
RestrictedControllerMessenger,
} from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { BaseControllerV1 } from '@metamask/base-controller';
import contractsMap from '@metamask/contract-metadata';
import {
toChecksumHexAddress,
Expand Down Expand Up @@ -124,7 +124,7 @@ export type TokensControllerMessenger = RestrictedControllerMessenger<
/**
* Controller that stores assets and exposes convenience methods
*/
export class TokensController extends BaseController<
export class TokensController extends BaseControllerV1<
TokensConfig,
TokensState
> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as sinon from 'sinon';

import type { BaseConfig, BaseState } from './BaseController';
import { BaseController } from './BaseController';
import type { BaseConfig, BaseState } from './BaseControllerV1';
import { BaseControllerV1 as BaseController } from './BaseControllerV1';

const STATE = { name: 'foo' };
const CONFIG = { disabled: true };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ export interface BaseState {
}

/**
* @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.
*/
export class BaseController<C extends BaseConfig, S extends BaseState> {
export class BaseControllerV1<C extends BaseConfig, S extends BaseState> {
/**
* Default options used to configure this controller
*/
Expand Down Expand Up @@ -68,7 +70,7 @@ export class BaseController<C extends BaseConfig, S extends BaseState> {
private readonly internalListeners: Listener<S>[] = [];

/**
* Creates a BaseController instance. Both initial state and initial
* 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.
Expand Down Expand Up @@ -190,4 +192,4 @@ export class BaseController<C extends BaseConfig, S extends BaseState> {
}
}

export default BaseController;
export default BaseControllerV1;
10 changes: 5 additions & 5 deletions packages/base-controller/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
export type { BaseConfig, BaseState, Listener } from './BaseController';
export { BaseController } from './BaseController';
export type { BaseConfig, BaseState, Listener } from './BaseControllerV1';
export { BaseControllerV1 } from './BaseControllerV1';
export type {
Listener as ListenerV2,
StateDeriver,
StateMetadata,
StatePropertyMetadata,
ControllerGetStateAction,
ControllerStateChangeEvent,
} from './BaseControllerV2';
export {
BaseController as BaseControllerV2,
BaseController,
getAnonymizedState,
getPersistentState,
type ControllerGetStateAction,
type ControllerStateChangeEvent,
} from './BaseControllerV2';
export * from './ControllerMessenger';
export * from './RestrictedControllerMessenger';
20 changes: 10 additions & 10 deletions packages/composable-controller/src/ComposableController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ import type {
} from '@metamask/base-controller';
import {
BaseController,
BaseControllerV2,
BaseControllerV1,
ControllerMessenger,
} from '@metamask/base-controller';
import type { Patch } from 'immer';
import * as sinon from 'sinon';

import { ComposableController } from './ComposableController';

// Mock BaseControllerV2 classes
// Mock BaseController classes

type FooControllerState = {
foo: string;
Expand All @@ -37,7 +37,7 @@ const fooControllerStateMetadata = {
},
};

class FooController extends BaseControllerV2<
class FooController extends BaseController<
'FooController',
FooControllerState,
FooMessenger
Expand All @@ -58,13 +58,13 @@ class FooController extends BaseControllerV2<
}
}

// Mock BaseController classes
// Mock BaseControllerV1 classes

type BarControllerState = BaseState & {
bar: string;
};

class BarController extends BaseController<never, BarControllerState> {
class BarController extends BaseControllerV1<never, BarControllerState> {
defaultState = {
bar: 'bar',
};
Expand All @@ -85,7 +85,7 @@ type BazControllerState = BaseState & {
baz: string;
};

class BazController extends BaseController<never, BazControllerState> {
class BazController extends BaseControllerV1<never, BazControllerState> {
defaultState = {
baz: 'baz',
};
Expand All @@ -103,7 +103,7 @@ describe('ComposableController', () => {
sinon.restore();
});

describe('BaseController', () => {
describe('BaseControllerV1', () => {
it('should compose controller state', () => {
const composableMessenger = new ControllerMessenger().getRestricted<
'ComposableController',
Expand Down Expand Up @@ -264,7 +264,7 @@ describe('ComposableController', () => {
});
});

describe('Mixed BaseController and BaseControllerV2', () => {
describe('Mixed BaseControllerV1 and BaseControllerV2', () => {
it('should compose controller state', () => {
const barController = new BarController();
const controllerMessenger = new ControllerMessenger<
Expand Down Expand Up @@ -329,7 +329,7 @@ describe('ComposableController', () => {
});
});

it('should notify listeners of BaseController state change', () => {
it('should notify listeners of BaseControllerV1 state change', () => {
const barController = new BarController();
const controllerMessenger = new ControllerMessenger<
never,
Expand Down Expand Up @@ -430,7 +430,7 @@ describe('ComposableController', () => {
expect(
() => new ComposableController([barController, fooController]),
).toThrow(
'Messaging system required if any BaseControllerV2 controllers are used',
'Messaging system required if any BaseController controllers are used',
);
});
});
Expand Down
20 changes: 10 additions & 10 deletions packages/composable-controller/src/ComposableController.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import type { RestrictedControllerMessenger } from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { BaseControllerV1 } from '@metamask/base-controller';

/**
* List of child controller instances
*
* This type encompasses controllers based up either BaseController or
* BaseControllerV2. The BaseControllerV2 type can't be included directly
* This type encompasses controllers based up either BaseControllerV1 or
* BaseController. The BaseController type can't be included directly
* because the generic parameters it expects require knowing the exact state
* shape, so instead we look for an object with the BaseControllerV2 properties
* shape, so instead we look for an object with the BaseController properties
* that we use in the ComposableController (name and state).
*/
export type ControllerList = (
| BaseController<any, any>
| BaseControllerV1<any, any>
| { name: string; state: Record<string, unknown> }
)[];

Expand All @@ -21,7 +21,7 @@ export type ComposableControllerRestrictedMessenger =
/**
* Controller that can be used to compose multiple controllers together.
*/
export class ComposableController extends BaseController<never, any> {
export class ComposableController extends BaseControllerV1<never, any> {
private readonly controllers: ControllerList = [];

private readonly messagingSystem?: ComposableControllerRestrictedMessenger;
Expand All @@ -35,7 +35,7 @@ export class ComposableController extends BaseController<never, any> {
* Creates a ComposableController instance.
*
* @param controllers - Map of names to controller instances.
* @param messenger - The controller messaging system, used for communicating with BaseControllerV2 controllers.
* @param messenger - The controller messaging system, used for communicating with BaseController controllers.
*/
constructor(
controllers: ControllerList,
Expand All @@ -53,8 +53,8 @@ export class ComposableController extends BaseController<never, any> {
this.messagingSystem = messenger;
this.controllers.forEach((controller) => {
const { name } = controller;
if ((controller as BaseController<any, any>).subscribe !== undefined) {
(controller as BaseController<any, any>).subscribe((state) => {
if ((controller as BaseControllerV1<any, any>).subscribe !== undefined) {
(controller as BaseControllerV1<any, any>).subscribe((state) => {
this.update({ [name]: state });
});
} else if (this.messagingSystem) {
Expand All @@ -66,7 +66,7 @@ export class ComposableController extends BaseController<never, any> {
);
} else {
throw new Error(
`Messaging system required if any BaseControllerV2 controllers are used`,
`Messaging system required if any BaseController controllers are used`,
);
}
});
Expand Down
4 changes: 2 additions & 2 deletions packages/ens-controller/src/EnsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {
} from '@ethersproject/providers';
import { Web3Provider } from '@ethersproject/providers';
import type { RestrictedControllerMessenger } from '@metamask/base-controller';
import { BaseControllerV2 } from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import type { ChainId } from '@metamask/controller-utils';
import {
normalizeEnsName,
Expand Down Expand Up @@ -77,7 +77,7 @@ const ZERO_X_ERROR_ADDRESS = '0x';
* Controller that manages a list ENS names and their resolved addresses
* by chainId. A null address indicates an unresolved ENS name.
*/
export class EnsController extends BaseControllerV2<
export class EnsController extends BaseController<
typeof name,
EnsControllerState,
EnsControllerMessenger
Expand Down
Loading

0 comments on commit be55ceb

Please sign in to comment.