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

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 workds 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.

Fixes #3649

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 #getEventPayload = 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.#getEventPayload.set(eventType, getPayload);
+  }
+
   /**
    * Publish an event.
    *
@@ -310,6 +341,14 @@ export class ControllerMessenger<
     }
 
     subscribers.set(handler, selector);
+
+    if (selector) {
+      const getPayload = this.#getEventPayload.get(eventType);
+      if (getPayload) {
+        const initialValue = selector(...getPayload());
+        this.#eventPayloadCache.set(handler, initialValue);
+      }
+    }
   }
 
   /**