Skip to content

Commit 41daada

Browse files
committed
Deprecate <ControllerName>:stateChange in favor of :stateChanged
Since the `:stateChange` event was added to `BaseController`, we have added a guideline asking engineers to use past tense for all event names. This commit updates BaseController to align with that guideline. To achieve backward compatibility, we add a new event, `:stateChanged`, rather than replacing the existing event. We deprecate `:stateChange` by adding some custom ESLint rules.
1 parent a618f17 commit 41daada

File tree

10 files changed

+581
-232
lines changed

10 files changed

+581
-232
lines changed

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ When adding or updating controllers in packages, follow these guidelines:
191191
- Controller classes should extend `BaseController`.
192192
- Controllers should not be stateless; if a controller does not have state, it should be a service.
193193
- The controller should define a public messenger type.
194-
- All messenger actions and events should be publicly defined. The default set should include the `:getState` action and `:stateChange` event.
194+
- All messenger actions and events should be publicly defined. The default set should include the `:getState` action and `:stateChanged` event.
195195
- All actions and events the messenger uses from other controllers and services should also be declared in the messenger type.
196196
- Controllers should initialize state by combining default and provided state. Provided state should be optional.
197197
- The constructor should take `messenger` and `state` options at a minimum.

docs/code-guidelines/controller-guidelines.md

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class FooController extends BaseController</* ... */> {
6464

6565
## Provide a default representation of state
6666

67-
Each controller needs a default representation in order to fully initialize itself when [receiving a partial representation of state](#accept-an-optional-partial-representation-of-state). A default representation of state is also useful when testing interactions with a controller's `*:stateChange` event.
67+
Each controller needs a default representation in order to fully initialize itself when [receiving a partial representation of state](#accept-an-optional-partial-representation-of-state). A default representation of state is also useful when testing interactions with a controller's `*:stateChanged` event.
6868

6969
A function which returns this default representation should be defined and exported. It should be called `getDefault${ControllerName}State`.
7070

@@ -226,7 +226,7 @@ const fooController = new FooController({
226226

227227
If the recipient controller uses a messenger, however, the callback pattern is unnecessary. Using the messenger not only aligns the controller with `BaseController`, but also reduces the number of options that consumers need to remember in order to use the controller:
228228

229-
**The constructor subscribes to the `BarController:stateChange` event**
229+
**The constructor subscribes to the `BarController:stateChanged` event**
230230

231231
```typescript
232232
/* === This repo: packages/foo-controller/src/FooController.ts === */
@@ -247,7 +247,7 @@ class FooController extends BaseController<
247247
constructor({ messenger /*, ... */ }, { messenger: FooControllerMessenger }) {
248248
super({ messenger /* ... */ });
249249

250-
messenger.subscribe('BarController:stateChange', (state) => {
250+
messenger.subscribe('BarController:stateChanged', (state) => {
251251
// do something with the state
252252
});
253253
}
@@ -280,7 +280,7 @@ const fooControllerMessenger = new Messenger<
280280
parent: rootMessenger,
281281
});
282282
rootMessenger.delegate({
283-
events: ['BarController:stateChange'],
283+
events: ['BarController:stateChanged'],
284284
messenger: fooControllerMessenger,
285285
});
286286
const fooController = new FooController({
@@ -541,16 +541,16 @@ type FooControllerGetStateAction = ControllerGetStateAction<
541541
>;
542542
```
543543

544-
## Define the `*:stateChange` event using the `ControllerStateChangeEvent` utility type
544+
## Define the `*:stateChanged` event using the `ControllerStateChangedEvent` utility type
545545

546-
Each controller needs a type for its `*:stateChange` event. The `ControllerStateChangeEvent` utility type from the `@metamask/base-controller` package should be used to define this type.
546+
Each controller needs a type for its `*:stateChanged` event. The `ControllerStateChangedEvent` utility type from the `@metamask/base-controller` package should be used to define this type.
547547

548-
The name of this type should be `${ControllerName}StateChangeEvent`.
548+
The name of this type should be `${ControllerName}StateChangedEvent`.
549549

550550
```typescript
551-
import type { ControllerStateChangeEvent } from '@metamask/base-controller';
551+
import type { ControllerStateChangedEvent } from '@metamask/base-controller';
552552

553-
type FooControllerStateChangeEvent = ControllerStateChangeEvent<
553+
type FooControllerStateChangedEvent = ControllerStateChangedEvent<
554554
'FooController',
555555
FooControllerState
556556
>;
@@ -890,7 +890,7 @@ This type should include:
890890
- This should always include `${controllerName}GetStateAction`
891891
- Actions imported from other controllers that the controller calls (i.e., _external actions_)
892892
- Events defined and exported by the controller that it publishes and expects consumers to subscribe to (i.e., _internal events_)
893-
- This should always include `${controllerName}StateChangeEvent`
893+
- This should always include `${controllerName}StateChangedEvent`
894894
- Events imported from other controllers that the controller subscribes to (i.e., _external events_)
895895

896896
The name of this type should be `${ControllerName}Messenger`.
@@ -923,7 +923,7 @@ export type AllowedActions =
923923
| ApprovalControllerAddApprovalRequestAction
924924
| ApprovalControllerAcceptApprovalRequestAction;
925925

926-
export type SwapsControllerStateChangeEvent = ControllerStateChangeEvent<
926+
export type SwapsControllerStateChangedEvent = ControllerStateChangedEvent<
927927
'SwapsController',
928928
SwapsControllerState
929929
>;
@@ -934,7 +934,7 @@ export type SwapsControllerSwapCreatedEvent = {
934934
};
935935

936936
export type SwapsControllerEvents =
937-
| SwapsControllerStateChangeEvent
937+
| SwapsControllerStateChangedEvent
938938
| SwapsControllerSwapCreatedEvent;
939939

940940
export type AllowedEvents =
@@ -1045,7 +1045,7 @@ class GasFeeController extends BaseController</* ... */> {
10451045
// ...
10461046

10471047
messenger.subscribe(
1048-
'NetworkController:stateChange',
1048+
'NetworkController:stateChanged',
10491049
(networkControllerState) => {
10501050
this.#updateGasFees(networkControllerState.selectedNetworkClientId);
10511051
},
@@ -1054,9 +1054,9 @@ class GasFeeController extends BaseController</* ... */> {
10541054
}
10551055
```
10561056

1057-
One way to fix this is to check if the other controller (the one being subscribed to) has a more suitable, granular event for the data being acted upon. For instance, `NetworkController` has a `networkDidChange` event which can be used in place of `NetworkController:stateChange` if the subscribing controller needs to know when the network has been switched:
1057+
One way to fix this is to check if the other controller (the one being subscribed to) has a more suitable, granular event for the data being acted upon. For instance, `NetworkController` has a `networkDidChange` event which can be used in place of `NetworkController:stateChanged` if the subscribing controller needs to know when the network has been switched:
10581058

1059-
**`NetworkController:networkDidChange` is used instead of `NetworkController:stateChange`**
1059+
**`NetworkController:networkDidChange` is used instead of `NetworkController:stateChanged`**
10601060

10611061
```typescript
10621062
class GasFeeController extends BaseController</* ... */> {
@@ -1098,7 +1098,7 @@ class TokensController extends BaseController</* ... */> {
10981098
let selectedAccount = accountsController.internalAccounts.selectedAccount;
10991099

11001100
messenger.subscribe(
1101-
'AccountsController:stateChange',
1101+
'AccountsController:stateChanged',
11021102
(newAccountsControllerState) => {
11031103
if (newAccountsControllerState.selectedAccount !== selectedAccount) {
11041104
this.#updateTokens(
@@ -1125,7 +1125,7 @@ class NftController extends BaseController/*<...>*/ {
11251125
);
11261126

11271127
messenger.subscribe(
1128-
'PreferencesController:stateChange',
1128+
'PreferencesController:stateChanged',
11291129
(newPreferencesControllerState) => {
11301130
if (
11311131
preferencesControllerState.ipfsGateway !== newPreferencesControllerState.ipfsGateway,
@@ -1190,7 +1190,7 @@ class NftController extends BaseController /*<...>*/ {
11901190
// ...
11911191

11921192
messenger.subscribe(
1193-
'PreferencesController:stateChange',
1193+
'PreferencesController:stateChanged',
11941194
({ ipfsGateway, openSeaEnabled, isIpfsGatewayEnabled }) => {
11951195
this.#updateNfts(ipfsGateway, openSeaEnabled, isIpfsGatewayEnabled);
11961196
},

0 commit comments

Comments
 (0)