Skip to content

Commit 94d55d8

Browse files
feat: refactor notification controller initialisation (#5504)
## Explanation Separates out the controller constructor and initialisation. This makes it easier to integrate with the Modular Initialisation approach in extension and mobile. **BREAKING** it would now require the controller to explicitly call init after being constructed to be correctly setup. E.g. ``` const notificationServicesController = ... // This needs to be done notificationServicesController.init() ``` ## 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 --> ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/notification-services-controller` - **REMOVED**: BREAKING - refactored out the `NotificationServicesController` constructor initialisation methods. You must explicitly call the `.init()` method to initialise controller - **ADDED**: `init` method to initialise `NotificationServicesController` ## 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 - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 8bf4c2a commit 94d55d8

File tree

2 files changed

+32
-22
lines changed

2 files changed

+32
-22
lines changed

packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -69,26 +69,6 @@ const mockErrorLog = () =>
6969
const mockWarnLog = () => jest.spyOn(log, 'warn').mockImplementation(jest.fn());
7070

7171
describe('metamask-notifications - constructor()', () => {
72-
const arrangeMocks = () => {
73-
const messengerMocks = mockNotificationMessenger();
74-
jest
75-
.spyOn(ControllerUtils, 'toChecksumHexAddress')
76-
.mockImplementation((x) => x);
77-
78-
return messengerMocks;
79-
};
80-
81-
const actPublishKeyringStateChange = async (
82-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
83-
messenger: any,
84-
) => {
85-
messenger.publish(
86-
'KeyringController:stateChange',
87-
{} as KeyringControllerState,
88-
[],
89-
);
90-
};
91-
9272
it('initializes state & override state', () => {
9373
const controller1 = new NotificationServicesController({
9474
messenger: mockNotificationMessenger().messenger,
@@ -108,6 +88,28 @@ describe('metamask-notifications - constructor()', () => {
10888
expect(controller2.state.isFeatureAnnouncementsEnabled).toBe(true);
10989
expect(controller2.state.isNotificationServicesEnabled).toBe(true);
11090
});
91+
});
92+
93+
describe('metamask-notifications - init()', () => {
94+
const arrangeMocks = () => {
95+
const messengerMocks = mockNotificationMessenger();
96+
jest
97+
.spyOn(ControllerUtils, 'toChecksumHexAddress')
98+
.mockImplementation((x) => x);
99+
100+
return messengerMocks;
101+
};
102+
103+
const actPublishKeyringStateChange = async (
104+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
105+
messenger: any,
106+
) => {
107+
messenger.publish(
108+
'KeyringController:stateChange',
109+
{} as KeyringControllerState,
110+
[],
111+
);
112+
};
111113

112114
it('keyring Change Event but feature not enabled will not add or remove triggers', async () => {
113115
const { messenger, globalMessenger, mockWithKeyring } = arrangeMocks();
@@ -118,6 +120,7 @@ describe('metamask-notifications - constructor()', () => {
118120
messenger,
119121
env: { featureAnnouncements: featureAnnouncementsEnv },
120122
});
123+
controller.init();
121124

122125
const mockUpdate = jest
123126
.spyOn(controller, 'updateOnChainTriggersByAccount')
@@ -146,6 +149,7 @@ describe('metamask-notifications - constructor()', () => {
146149
subscriptionAccountsSeen: [ADDRESS_1],
147150
},
148151
});
152+
controller.init();
149153

150154
const mockUpdate = jest
151155
.spyOn(controller, 'updateOnChainTriggersByAccount')
@@ -199,12 +203,14 @@ describe('metamask-notifications - constructor()', () => {
199203
modifications?.(mocks);
200204

201205
// Act
202-
new NotificationServicesController({
206+
const controller = new NotificationServicesController({
203207
messenger: mocks.messenger,
204208
env: { featureAnnouncements: featureAnnouncementsEnv },
205209
state: { isNotificationServicesEnabled: true },
206210
});
207211

212+
controller.init();
213+
208214
return mocks;
209215
};
210216

@@ -287,7 +293,7 @@ describe('metamask-notifications - constructor()', () => {
287293
modifications?.(mocks);
288294

289295
// Act
290-
new NotificationServicesController({
296+
const controller = new NotificationServicesController({
291297
messenger: mocks.messenger,
292298
env: {
293299
featureAnnouncements: featureAnnouncementsEnv,
@@ -296,6 +302,8 @@ describe('metamask-notifications - constructor()', () => {
296302
state: { isNotificationServicesEnabled: true },
297303
});
298304

305+
controller.init();
306+
299307
return mocks;
300308
};
301309

packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,9 @@ export default class NotificationServicesController extends BaseController<
545545
this.#featureAnnouncementEnv = env.featureAnnouncements;
546546
this.#registerMessageHandlers();
547547
this.#clearLoadingStates();
548+
}
548549

550+
init() {
549551
this.#keyringController.setupLockedStateSubscriptions(async () => {
550552
await this.#accounts.initialize();
551553
await this.#pushNotifications.initializePushNotifications();

0 commit comments

Comments
 (0)