Skip to content

Commit

Permalink
fix(base-controller): Fix stateChange subscriptions with selectors
Browse files Browse the repository at this point in the history
Subscribers to the `stateChange` event of any `BaseControllerV2`-based
controllers will now correctly handle the initial state change event.

Previously the initial state change would always result in this event
firing, even for subscriptions with selectors where the selected value
has not changed. Additionally, the `previousValue` returned was always
set to `undefined` the first time.

`BaseControllerV2` has been updated to correctly compare with the
previous value even for the first state change. The returned
`previousValue` is also now guaranteed to be correct even for the
initial state change.

Fixes #3701
  • Loading branch information
Gudahtt committed Jan 9, 2024
1 parent 580f07e commit 0374150
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 0 deletions.
58 changes: 58 additions & 0 deletions packages/base-controller/src/BaseControllerV2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,64 @@ describe('BaseController', () => {
]);
});

it('should notify a subscriber with a selector of state changes', () => {
const controllerMessenger = new ControllerMessenger<
never,
CountControllerEvent
>();
const controller = new CountController({
messenger: getCountMessenger(controllerMessenger),
name: 'CountController',
state: { count: 0 },
metadata: countControllerStateMetadata,
});
const listener = sinon.stub();
controllerMessenger.subscribe(
'CountController:stateChange',
listener,
({ count }) => {
// Selector rounds down to nearest multiple of 10
return Math.floor(count / 10);
},
);

controller.update(() => {
return { count: 10 };
});

expect(listener.callCount).toBe(1);
expect(listener.firstCall.args).toStrictEqual([1, 0]);
});

it('should not inform a subscriber of state changes if the selected value is unchanged', () => {
const controllerMessenger = new ControllerMessenger<
never,
CountControllerEvent
>();
const controller = new CountController({
messenger: getCountMessenger(controllerMessenger),
name: 'CountController',
state: { count: 0 },
metadata: countControllerStateMetadata,
});
const listener = sinon.stub();
controllerMessenger.subscribe(
'CountController:stateChange',
listener,
({ count }) => {
// Selector rounds down to nearest multiple of 10
return Math.floor(count / 10);
},
);

controller.update(() => {
// Note that this rounds down to zero, so the selected value is still zero
return { count: 1 };
});

expect(listener.callCount).toBe(0);
});

it('should inform a subscriber of each state change once even after multiple subscriptions', () => {
const controllerMessenger = new ControllerMessenger<
never,
Expand Down
5 changes: 5 additions & 0 deletions packages/base-controller/src/BaseControllerV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ export class BaseController<
`${name}:getState`,
() => this.state,
);

this.messagingSystem.registerInitialEventPayload({
eventType: `${name}:stateChange`,
getPayload: () => [this.state, []],
});
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/name-controller/src/NameController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const TIME_MOCK = 123;

const MESSENGER_MOCK = {
registerActionHandler: jest.fn(),
registerInitialEventPayload: jest.fn(),
publish: jest.fn(),
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ const requestMock = {
const createMessengerMock = () =>
({
registerActionHandler: jest.fn(),
registerInitialEventPayload: jest.fn(),
publish: jest.fn(),
call: jest.fn(),
// TODO: Replace `any` with type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ function createMessengerMock() {
call: jest.fn(),
publish: jest.fn(),
registerActionHandler: jest.fn(),
registerInitialEventPayload: jest.fn(),
} as unknown as jest.Mocked<UserOperationControllerMessenger>;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ jest.mock('@metamask/controller-utils', () => ({
function createMessengerMock() {
return {
call: jest.fn(),
registerInitialEventPayload: jest.fn(),
} as unknown as jest.Mocked<UserOperationControllerMessenger>;
}

Expand Down

0 comments on commit 0374150

Please sign in to comment.