fix(base-controller): Fix selectors initial value comparison (#3697)

## Explanation

The "selectors" feature of the `ControllerMessenger` did not work
correctly for the first event published after subscribing, because the
original value of the payload (at time of subscribing) was not available
for comparison.

To address this, a `registerInitialEventPayload` method has been added
to the `ControllerMessenger` and `RestrictedControllerMessenger`. This
allows registering a function that returns the "event payload state" at
a given time, which is used internally by the `subscribe` method to
correctly initialize the payload cache when a subscriber with a selector
is added. This ensures that the selector works correctly even for the
initial state change, in that it will return the correct previous value,
and not fire at all if the selected piece of state is unchanged.

In a later PR, `BaseControllerV2` can be updated to register an event
payload for the `stateChange` event. This will fix this flaw for the
`stateChange` event of each controller.

## References

Fixes #3649

## Changelog

### `@metamask/base-controller`

- Added: Add `registerInitialEventPayload` to `ControllerMessenger` and
`RestrictedControllerMessenger`
- This allows registering an event payload function for an event, which
has the benefit of ensuring the "subscription selector" feature works
correctly the first time the event is fired after subscribing.
- Fixed: Fix `subscribe` method selector support on first publish
- An event with a registered initial event payload function will work
better with selectors, in that it will correctly compare with the
initial selected state and return the previous value the first time it's
published. Without this, the initial published event will always return
`undefined` as the previous value.

## 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

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

@@ -252,73 +252,193 @@ describe('ControllerMessenger', () => {
     expect(handler2.callCount).toBe(1);
   });
 
-  it('should publish event with selector to subscriber', () => {
-    type MessageEvent = {
-      type: 'complexMessage';
-      payload: [Record<string, unknown>];
-    };
-    const controllerMessenger = new ControllerMessenger<never, MessageEvent>();
-
-    const handler = sinon.stub();
-    const selector = sinon.fake((obj: Record<string, unknown>) => obj.prop1);
-    controllerMessenger.subscribe('complexMessage', handler, selector);
-    controllerMessenger.publish('complexMessage', { prop1: 'a', prop2: 'b' });
+  describe('on first state change with an initial payload function registered', () => {
+    it('should publish event if selected payload differs', () => {
+      const state = {
+        propA: 1,
+        propB: 1,
+      };
+      type MessageEvent = {
+        type: 'complexMessage';
+        payload: [typeof state];
+      };
+      const controllerMessenger = new ControllerMessenger<
+        never,
+        MessageEvent
+      >();
+      controllerMessenger.registerInitialEventPayload({
+        eventType: 'complexMessage',
+        getPayload: () => [state],
+      });
+      const handler = sinon.stub();
+      const selector = sinon.fake((obj: Record<string, unknown>) => obj.propA);
+      controllerMessenger.subscribe('complexMessage', handler, selector);
+
+      state.propA += 1;
+      controllerMessenger.publish('complexMessage', state);
+
+      expect(handler.getCall(0)?.args).toStrictEqual([2, 1]);
+      expect(handler.callCount).toBe(1);
+    });
 
-    expect(handler.calledWithExactly('a', undefined)).toBe(true);
-    expect(handler.callCount).toBe(1);
-    expect(selector.calledWithExactly({ prop1: 'a', prop2: 'b' })).toBe(true);
-    expect(selector.callCount).toBe(1);
+    it('should not publish event if selected payload is the same', () => {
+      const state = {
+        propA: 1,
+        propB: 1,
+      };
+      type MessageEvent = {
+        type: 'complexMessage';
+        payload: [typeof state];
+      };
+      const controllerMessenger = new ControllerMessenger<
+        never,
+        MessageEvent
+      >();
+      controllerMessenger.registerInitialEventPayload({
+        eventType: 'complexMessage',
+        getPayload: () => [state],
+      });
+      const handler = sinon.stub();
+      const selector = sinon.fake((obj: Record<string, unknown>) => obj.propA);
+      controllerMessenger.subscribe('complexMessage', handler, selector);
+
+      controllerMessenger.publish('complexMessage', state);
+
+      expect(handler.callCount).toBe(0);
+    });
   });
 
-  it('should call selector event handler with previous selector return value', () => {
-    type MessageEvent = {
-      type: 'complexMessage';
-      payload: [Record<string, unknown>];
-    };
-    const controllerMessenger = new ControllerMessenger<never, MessageEvent>();
-
-    const handler = sinon.stub();
-    const selector = sinon.fake((obj: Record<string, unknown>) => obj.prop1);
-    controllerMessenger.subscribe('complexMessage', handler, selector);
-    controllerMessenger.publish('complexMessage', { prop1: 'a', prop2: 'b' });
-    controllerMessenger.publish('complexMessage', { prop1: 'z', prop2: 'b' });
+  describe('on first state change without an initial payload function registered', () => {
+    it('should publish event if selected payload differs', () => {
+      const state = {
+        propA: 1,
+        propB: 1,
+      };
+      type MessageEvent = {
+        type: 'complexMessage';
+        payload: [typeof state];
+      };
+      const controllerMessenger = new ControllerMessenger<
+        never,
+        MessageEvent
+      >();
+      const handler = sinon.stub();
+      const selector = sinon.fake((obj: Record<string, unknown>) => obj.propA);
+      controllerMessenger.subscribe('complexMessage', handler, selector);
+
+      state.propA += 1;
+      controllerMessenger.publish('complexMessage', state);
+
+      expect(handler.getCall(0)?.args).toStrictEqual([2, undefined]);
+      expect(handler.callCount).toBe(1);
+    });
 
-    expect(handler.getCall(0).calledWithExactly('a', undefined)).toBe(true);
-    expect(handler.getCall(1).calledWithExactly('z', 'a')).toBe(true);
-    expect(handler.callCount).toBe(2);
-    expect(
-      selector.getCall(0).calledWithExactly({ prop1: 'a', prop2: 'b' }),
-    ).toBe(true);
+    it('should publish event even when selected payload does not change', () => {
+      const state = {
+        propA: 1,
+        propB: 1,
+      };
+      type MessageEvent = {
+        type: 'complexMessage';
+        payload: [typeof state];
+      };
+      const controllerMessenger = new ControllerMessenger<
+        never,
+        MessageEvent
+      >();
+      const handler = sinon.stub();
+      const selector = sinon.fake((obj: Record<string, unknown>) => obj.propA);
+      controllerMessenger.subscribe('complexMessage', handler, selector);
+
+      controllerMessenger.publish('complexMessage', state);
+
+      expect(handler.getCall(0)?.args).toStrictEqual([1, undefined]);
+      expect(handler.callCount).toBe(1);
+    });
 
-    expect(
-      selector.getCall(1).calledWithExactly({ prop1: 'z', prop2: 'b' }),
-    ).toBe(true);
-    expect(selector.callCount).toBe(2);
+    it('should not publish if selector returns undefined', () => {
+      const state = {
+        propA: undefined,
+        propB: 1,
+      };
+      type MessageEvent = {
+        type: 'complexMessage';
+        payload: [typeof state];
+      };
+      const controllerMessenger = new ControllerMessenger<
+        never,
+        MessageEvent
+      >();
+      const handler = sinon.stub();
+      const selector = sinon.fake((obj: Record<string, unknown>) => obj.propA);
+      controllerMessenger.subscribe('complexMessage', handler, selector);
+
+      controllerMessenger.publish('complexMessage', state);
+
+      expect(handler.callCount).toBe(0);
+    });
   });
 
-  it('should not publish event with selector if selector return value is unchanged', () => {
-    type MessageEvent = {
-      type: 'complexMessage';
-      payload: [Record<string, unknown>];
-    };
-    const controllerMessenger = new ControllerMessenger<never, MessageEvent>();
-
-    const handler = sinon.stub();
-    const selector = sinon.fake((obj: Record<string, unknown>) => obj.prop1);
-    controllerMessenger.subscribe('complexMessage', handler, selector);
-    controllerMessenger.publish('complexMessage', { prop1: 'a', prop2: 'b' });
-    controllerMessenger.publish('complexMessage', { prop1: 'a', prop3: 'c' });
+  describe('on later state change', () => {
+    it('should call selector event handler with previous selector return value', () => {
+      type MessageEvent = {
+        type: 'complexMessage';
+        payload: [Record<string, unknown>];
+      };
+      const controllerMessenger = new ControllerMessenger<
+        never,
+        MessageEvent
+      >();
+
+      const handler = sinon.stub();
+      const selector = sinon.fake((obj: Record<string, unknown>) => obj.prop1);
+      controllerMessenger.subscribe('complexMessage', handler, selector);
+      controllerMessenger.publish('complexMessage', { prop1: 'a', prop2: 'b' });
+      controllerMessenger.publish('complexMessage', { prop1: 'z', prop2: 'b' });
+
+      expect(handler.getCall(0).calledWithExactly('a', undefined)).toBe(true);
+      expect(handler.getCall(1).calledWithExactly('z', 'a')).toBe(true);
+      expect(handler.callCount).toBe(2);
+    });
 
-    expect(handler.calledWithExactly('a', undefined)).toBe(true);
-    expect(handler.callCount).toBe(1);
-    expect(
-      selector.getCall(0).calledWithExactly({ prop1: 'a', prop2: 'b' }),
-    ).toBe(true);
+    it('should publish event with selector to subscriber', () => {
+      type MessageEvent = {
+        type: 'complexMessage';
+        payload: [Record<string, unknown>];
+      };
+      const controllerMessenger = new ControllerMessenger<
+        never,
+        MessageEvent
+      >();
+
+      const handler = sinon.stub();
+      const selector = sinon.fake((obj: Record<string, unknown>) => obj.prop1);
+      controllerMessenger.subscribe('complexMessage', handler, selector);
+      controllerMessenger.publish('complexMessage', { prop1: 'a', prop2: 'b' });
+
+      expect(handler.calledWithExactly('a', undefined)).toBe(true);
+      expect(handler.callCount).toBe(1);
+    });
 
-    expect(
-      selector.getCall(1).calledWithExactly({ prop1: 'a', prop3: 'c' }),
-    ).toBe(true);
-    expect(selector.callCount).toBe(2);
+    it('should not publish event with selector if selector return value is unchanged', () => {
+      type MessageEvent = {
+        type: 'complexMessage';
+        payload: [Record<string, unknown>];
+      };
+      const controllerMessenger = new ControllerMessenger<
+        never,
+        MessageEvent
+      >();
+
+      const handler = sinon.stub();
+      const selector = sinon.fake((obj: Record<string, unknown>) => obj.prop1);
+      controllerMessenger.subscribe('complexMessage', handler, selector);
+      controllerMessenger.publish('complexMessage', { prop1: 'a', prop2: 'b' });
+      controllerMessenger.publish('complexMessage', { prop1: 'a', prop3: 'c' });
+
+      expect(handler.calledWithExactly('a', undefined)).toBe(true);
+      expect(handler.callCount).toBe(1);
+    });
   });
 
   it('should publish event to many subscribers with the same selector', () => {

packages/base-controller/src/ControllerMessenger.ts

@@ -129,6 +129,16 @@ export class ControllerMessenger<
 
   readonly #events = new Map<Event['type'], EventSubscriptionMap<Event>>();
 
+  /**
+   * A map of functions for getting the initial event payload.
+   *
+   * Used only for events that represent state changes.
+   */
+  readonly #initialEventPayloadGetters = new Map<
+    Event['type'],
+    () => ExtractEventPayload<Event, Event['type']>
+  >();
+
   /**
    * A cache of selector return values for their respective handlers.
    */
@@ -210,6 +220,27 @@ export class ControllerMessenger<
     return handler(...params);
   }
 
+  /**
+   * Register a function for getting the initial payload for an event.
+   *
+   * This is used for events that represent a state change, where the payload is the state.
+   * Registering a function for getting the payload allows event selectors to have a point of
+   * comparison the first time state changes.
+   *
+   * @param args - The arguments to this function
+   * @param args.eventType - The event type to register a payload for.
+   * @param args.getPayload - A function for retrieving the event payload.
+   */
+  registerInitialEventPayload<EventType extends Event['type']>({
+    eventType,
+    getPayload,
+  }: {
+    eventType: EventType;
+    getPayload: () => ExtractEventPayload<Event, EventType>;
+  }) {
+    this.#initialEventPayloadGetters.set(eventType, getPayload);
+  }
+
   /**
    * Publish an event.
    *
@@ -310,6 +341,14 @@ export class ControllerMessenger<
     }
 
     subscribers.set(handler, selector);
+
+    if (selector) {
+      const getPayload = this.#initialEventPayloadGetters.get(eventType);
+      if (getPayload) {
+        const initialValue = selector(...getPayload());
+        this.#eventPayloadCache.set(handler, initialValue);
+      }
+    }
   }
 
   /**