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

[Gudahtt]

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

• I've updated the test suite for new or updated code as appropriate
• I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
• I've highlighted breaking changes using the "BREAKING" category above as appropriate

[Gudahtt]

The base controller has been updated in #3702

See that PR for a demonstration of how this would work

[mcmire]

Left two comments, but overall this makes sense.

[mcmire]

It seems that we are only extending the API of the messenger for a very specific use case. Do we expect consumers to call this on its own, or is this really something that we will only call internally? If the former, what are your thoughts on tying this more closely to that use case by adding a fourth argument to subscribe that would allow consumers to specify an initial payload getter when subscribing (and would end up calling this method)? If the latter, then what do you think about adding a note in the docs here — that if you want to use this for a particular event, make sure to call it ahead of any code that registers subscriptions.

[Gudahtt]

We could make it a fourth parameter to subscribe, but that'd put the responsibility on each subscriber to know how to get the initial state, and it'd make using selectors a lot more verbose (and would require an additional action to get the initial state).

Whereas with the registerInitialEventPayload solution, the publisher of the event is also responsible for registering this "get payload" function. This seems ideal because the publisher is the one who can be most confident what the state will be at any given time, and it keeps the responsibility in a single place.

Do we expect consumers to call this on its own, or is this really something that we will only call internally?

This is something that would be called in controller constructors, similar to the action registrations. Before any publishing happens.

It doesn't really matter whether this registration occurs before or after subscriptions. It just has to happen before publishing. Just like the action handlers need to be registered before calling.

I'm not sure if it'll have a use beyond the state event. Maybe if we have a controller that is exposing third-party state, it might be relevant (e.g. something that is proxying on-chain or remote API state).

[mcmire]

Looks good!

[Gudahtt]

I'm going to wait until #3713 is merged and released before merging this, to keep the releases more focused.