Skip to content

Commit 36c3f4b

Browse files
authored
feat: auto register Messenger actions (#5927)
## Explanation ### Add `registerMethodActionHandlers` method for simplified bulk action handler registration #### 🎯 **Summary** This PR introduces a new `registerMethodActionHandlers` method to `BaseController`, `Messenger`, and `RestrictedMessenger` classes that simplifies the process of registering multiple action handlers at once, reducing boilerplate code and improving developer experience. #### 🚀 **Problem** Currently, controllers need to manually register each action handler individually, leading to repetitive boilerplate code: ```typescript // Current approach - lots of repetition this.messagingSystem.registerActionHandler('MyController:method1', this.method1.bind(this)); this.messagingSystem.registerActionHandler('MyController:method2', this.method2.bind(this)); this.messagingSystem.registerActionHandler('MyController:method3', this.method3.bind(this)); this.messagingSystem.registerActionHandler('MyController:method4', this.method4.bind(this)); ``` This approach is: - **Verbose**: Requires multiple lines for each handler - **Error-prone**: Easy to forget `.bind(this)` or make typos - **Difficult to maintain**: Changes require updating multiple lines - **Inconsistent**: Different controllers may use different patterns #### 💡 **Solution** The new `registerActionHandlers` method allows bulk registration with smart defaults: ```typescript // New approach - clean and concise this.registerActionHandlers(['method1', 'method2', 'method3', 'method4']); ``` #### ✨ **Key Features** ##### 1. **Bulk Registration** Register multiple action handlers with a single method call: ```typescript this.registerActionHandlers(['getAccounts', 'setSelectedAccount', 'updateAccount']); ``` ##### 2. **Automatic Method Binding** Methods are automatically bound to the controller instance - no more forgotten `.bind(this)`: ```typescript // The method is automatically bound to the controller instance this.registerActionHandlers(['getPrivateData']); ``` #### 📝 **Usage Examples** ##### Basic Usage ```typescript class MyController extends BaseController { constructor(messenger) { super(/* ... */); // Register multiple handlers at once this.registerActionHandlers([ 'getState', 'updateSettings', 'clearData', ]); } } ``` ##### Advanced Usage with Custom Exclusions and Exceptions ```typescript class AdvancedController extends BaseController { constructor(messenger) { super(/* ... */); this.registerActionHandlers(['method1', 'method2', 'method3', 'internalMethod']); } } ``` ### 🔄 **Migration Guide** This feature is optional and backward compatible. Controllers can gradually migrate to using `registerActionHandlers` for improved maintainability: ```typescript // Before this.messagingSystem.registerActionHandler('Controller:method1', this.method1.bind(this)); this.messagingSystem.registerActionHandler('Controller:method2', this.method2.bind(this)); // After this.registerActionHandlers(['method1', 'method2']); ``` ### 🤖 **Automated Action Type Generation** To complement the bulk registration feature, this PR also introduces an automated script that generates TypeScript action types from controller methods, eliminating the need to manually create and maintain action type definitions. #### **New Script: `generate-method-action-types.ts`** ```bash # Auto-generate action types for all controllers yarn ts-node scripts/generate-method-action-types.ts ``` **What it does:** - **Discovers controllers**: Automatically finds all controllers with `MESSENGER_EXPOSED_METHODS` constants - **Extracts metadata**: Parses JSDoc comments and method signatures using TypeScript AST - **Generates types**: Creates properly formatted action type definitions **Generated Output Example:** ```typescript /** * Returns the Infura network client with the given ID. * * @param networkClientId - A network client ID. * @returns The network client. * @throws If a network client does not exist with the given ID. */ export type NetworkControllerGetNetworkClientByIdAction = { type: `NetworkController:getNetworkClientById`; handler: NetworkController['getNetworkClientById']; }; ``` #### **Development Workflow** 1. Add/modify methods in a controller 2. Update the `MESSENGER_EXPOSED_METHODS` constant 3. Run `yarn ts-node scripts/generate-method-action-types.ts` or `yarn run generate-method-action-types` 4. Commit the generated/updated action type files **Output Files:** ```typescript packages/network-controller/src/network-controller-method-action-types.ts packages/address-book-controller/src/addressbook-controller-method-action-types.ts ``` ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> Fixes: #4582 ## Changelog <!-- THIS SECTION IS NO LONGER NEEDED. The process for updating changelogs has changed. Please consult the "Updating changelogs" section of the Contributing doc for more. --> ## 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 communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 6f7c339 commit 36c3f4b

File tree

8 files changed

+1004
-15
lines changed

8 files changed

+1004
-15
lines changed

eslint-warning-thresholds.json

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,6 @@
133133
"packages/base-controller/src/BaseControllerV2.ts": {
134134
"jsdoc/check-tag-names": 2
135135
},
136-
"packages/base-controller/src/Messenger.test.ts": {
137-
"import-x/namespace": 33
138-
},
139-
"packages/base-controller/src/RestrictedMessenger.test.ts": {
140-
"import-x/namespace": 31
141-
},
142136
"packages/build-utils/src/transforms/remove-fenced-code.test.ts": {
143137
"import-x/order": 1
144138
},

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,12 @@
2121
"changelog:update": "yarn workspaces foreach --all --no-private --parallel --interlaced --verbose run changelog:update",
2222
"changelog:validate": "yarn workspaces foreach --all --no-private --parallel --interlaced --verbose run changelog:validate",
2323
"create-package": "ts-node scripts/create-package",
24-
"lint": "yarn lint:eslint && echo && yarn lint:misc --check && yarn constraints && yarn lint:dependencies && yarn lint:teams",
24+
"generate-method-action-types": "ts-node scripts/generate-method-action-types.ts",
25+
"lint": "yarn lint:eslint && echo && yarn lint:misc --check && yarn constraints && yarn lint:dependencies && yarn lint:teams && yarn generate-method-action-types --check",
2526
"lint:dependencies": "depcheck && yarn dedupe --check",
2627
"lint:dependencies:fix": "depcheck && yarn dedupe",
2728
"lint:eslint": "yarn build:only-clean && yarn ts-node ./scripts/run-eslint.ts --cache",
28-
"lint:fix": "yarn lint:eslint --fix && echo && yarn lint:misc --write && yarn constraints --fix && yarn lint:dependencies:fix",
29+
"lint:fix": "yarn lint:eslint --fix && echo && yarn lint:misc --write && yarn constraints --fix && yarn lint:dependencies:fix && yarn generate-method-action-types --fix",
2930
"lint:misc": "prettier --no-error-on-unmatched-pattern '**/*.json' '**/*.md' '**/*.yml' '!.yarnrc.yml' '!merged-packages/**' --ignore-path .gitignore",
3031
"lint:teams": "ts-node scripts/lint-teams-json.ts",
3132
"prepack": "./scripts/prepack.sh",

packages/base-controller/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Add `registerMethodActionHandlers` method to `Messenger`, and `RestrictedMessenger` for simplified bulk action handler registration ([#5927](https://github.com/MetaMask/core/pull/5927))
13+
- Allows registering action handlers that map to methods on a messenger client at once by passing an array of method names
14+
- Automatically binds action handlers to the given messenger client
15+
1016
### Changed
1117

1218
- Bump `@metamask/utils` from `^11.2.0` to `^11.4.2` ([#6054](https://github.com/MetaMask/core/pull/6054))

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

Lines changed: 169 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Patch } from 'immer';
2-
import * as sinon from 'sinon';
2+
import sinon from 'sinon';
33

44
import { Messenger } from './Messenger';
55

@@ -556,4 +556,172 @@ describe('Messenger', () => {
556556

557557
expect(handler.callCount).toBe(0);
558558
});
559+
560+
describe('registerMethodActionHandlers', () => {
561+
it('should register action handlers for specified methods on the given messenger client', () => {
562+
type TestActions =
563+
| { type: 'TestService:getType'; handler: () => string }
564+
| {
565+
type: 'TestService:getCount';
566+
handler: () => number;
567+
};
568+
569+
const messenger = new Messenger<TestActions, never>();
570+
571+
class TestService {
572+
name = 'TestService';
573+
574+
getType() {
575+
return 'api';
576+
}
577+
578+
getCount() {
579+
return 42;
580+
}
581+
}
582+
583+
const service = new TestService();
584+
const methodNames = ['getType', 'getCount'] as const;
585+
586+
messenger.registerMethodActionHandlers(service, methodNames);
587+
588+
const state = messenger.call('TestService:getType');
589+
expect(state).toBe('api');
590+
591+
const count = messenger.call('TestService:getCount');
592+
expect(count).toBe(42);
593+
});
594+
595+
it('should bind action handlers to the given messenger client', () => {
596+
type TestAction = {
597+
type: 'TestService:getPrivateValue';
598+
handler: () => string;
599+
};
600+
const messenger = new Messenger<TestAction, never>();
601+
602+
class TestService {
603+
name = 'TestService';
604+
605+
privateValue = 'secret';
606+
607+
getPrivateValue() {
608+
return this.privateValue;
609+
}
610+
}
611+
612+
const service = new TestService();
613+
messenger.registerMethodActionHandlers(service, ['getPrivateValue']);
614+
615+
const result = messenger.call('TestService:getPrivateValue');
616+
expect(result).toBe('secret');
617+
});
618+
619+
it('should handle async methods', async () => {
620+
type TestAction = {
621+
type: 'TestService:fetchData';
622+
handler: (id: string) => Promise<string>;
623+
};
624+
const messenger = new Messenger<TestAction, never>();
625+
626+
class TestService {
627+
name = 'TestService';
628+
629+
async fetchData(id: string) {
630+
return `data-${id}`;
631+
}
632+
}
633+
634+
const service = new TestService();
635+
messenger.registerMethodActionHandlers(service, ['fetchData']);
636+
637+
const result = await messenger.call('TestService:fetchData', '123');
638+
expect(result).toBe('data-123');
639+
});
640+
641+
it('should not throw when given an empty methodNames array', () => {
642+
type TestAction = { type: 'TestController:test'; handler: () => void };
643+
const messenger = new Messenger<TestAction, never>();
644+
645+
class TestController {
646+
name = 'TestController';
647+
}
648+
649+
const controller = new TestController();
650+
const methodNames: readonly string[] = [];
651+
652+
expect(() => {
653+
messenger.registerMethodActionHandlers(
654+
controller,
655+
methodNames as never[],
656+
);
657+
}).not.toThrow();
658+
});
659+
660+
it('should skip non-function properties', () => {
661+
type TestAction = {
662+
type: 'TestController:getValue';
663+
handler: () => string;
664+
};
665+
const messenger = new Messenger<TestAction, never>();
666+
667+
class TestController {
668+
name = 'TestController';
669+
670+
readonly nonFunction = 'not a function';
671+
672+
getValue() {
673+
return 'test';
674+
}
675+
}
676+
677+
const controller = new TestController();
678+
messenger.registerMethodActionHandlers(controller, ['getValue']);
679+
680+
// getValue should be registered
681+
expect(messenger.call('TestController:getValue')).toBe('test');
682+
683+
// nonFunction should not be registered
684+
expect(() => {
685+
// @ts-expect-error - This is a test
686+
messenger.call('TestController:nonFunction');
687+
}).toThrow(
688+
'A handler for TestController:nonFunction has not been registered',
689+
);
690+
});
691+
692+
it('should work with class inheritance', () => {
693+
type TestActions =
694+
| { type: 'ChildController:baseMethod'; handler: () => string }
695+
| { type: 'ChildController:childMethod'; handler: () => string };
696+
697+
const messenger = new Messenger<TestActions, never>();
698+
699+
class BaseController {
700+
name = 'BaseController';
701+
702+
baseMethod() {
703+
return 'base method';
704+
}
705+
}
706+
707+
class ChildController extends BaseController {
708+
name = 'ChildController';
709+
710+
childMethod() {
711+
return 'child method';
712+
}
713+
}
714+
715+
const controller = new ChildController();
716+
messenger.registerMethodActionHandlers(controller, [
717+
'baseMethod',
718+
'childMethod',
719+
]);
720+
721+
expect(messenger.call('ChildController:baseMethod')).toBe('base method');
722+
expect(messenger.call('ChildController:childMethod')).toBe(
723+
'child method',
724+
);
725+
});
726+
});
559727
});

packages/base-controller/src/Messenger.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export class Messenger<
154154
*
155155
* This will make the registered function available to call via the `call` method.
156156
*
157-
* @param actionType - The action type. This is a unqiue identifier for this action.
157+
* @param actionType - The action type. This is a unique identifier for this action.
158158
* @param handler - The action handler. This function gets called when the `call` method is
159159
* invoked with the given action type.
160160
* @throws Will throw when a handler has been registered for this action type already.
@@ -172,12 +172,32 @@ export class Messenger<
172172
this.#actions.set(actionType, handler);
173173
}
174174

175+
/**
176+
* Registers action handlers for a list of methods on a messenger client
177+
*
178+
* @param messengerClient - The object that is expected to make use of the messenger.
179+
* @param methodNames - The names of the methods on the messenger client to register as action
180+
* handlers
181+
*/
182+
registerMethodActionHandlers<
183+
MessengerClient extends { name: string },
184+
MethodNames extends keyof MessengerClient & string,
185+
>(messengerClient: MessengerClient, methodNames: readonly MethodNames[]) {
186+
for (const methodName of methodNames) {
187+
const method = messengerClient[methodName];
188+
if (typeof method === 'function') {
189+
const actionType = `${messengerClient.name}:${methodName}` as const;
190+
this.registerActionHandler(actionType, method.bind(messengerClient));
191+
}
192+
}
193+
}
194+
175195
/**
176196
* Unregister an action handler.
177197
*
178198
* This will prevent this action from being called.
179199
*
180-
* @param actionType - The action type. This is a unqiue identifier for this action.
200+
* @param actionType - The action type. This is a unique identifier for this action.
181201
* @template ActionType - A type union of Action type strings.
182202
*/
183203
unregisterActionHandler<ActionType extends Action['type']>(
@@ -201,7 +221,7 @@ export class Messenger<
201221
* This function will call the action handler corresponding to the given action type, passing
202222
* along any parameters given.
203223
*
204-
* @param actionType - The action type. This is a unqiue identifier for this action.
224+
* @param actionType - The action type. This is a unique identifier for this action.
205225
* @param params - The action parameters. These must match the type of the parameters of the
206226
* registered action handler.
207227
* @throws Will throw when no handler has been registered for the given type.

0 commit comments

Comments
 (0)