chore (client): refactoring subscribe methods of the event notifier

[kevin-dp]

This PR refactors the various subscribe methods provided by the event notifier. For every listener that was being subscribed, the event notifier would generate a random key, store the listener under that key, and return that key. To unsubscribe, the user had to pass that key to the corresponding unsubscribe method which would lookup the listener and then unsubscribe it from the underlying event emitter. Storing the listener under a key is not needed and is a bit redundant as the underlying event emitter already stores them.

This PR modified the event notifier such that it does not keep track of registered callbacks but instead returns an unsubscribe function from the subscribe methods. The unsubscribe function closes over the callback that needs to be unregistered from the underlying event emitter.

[icehaunter]

I love it

[samwillis]

I love the concept here too, however we need to consider this in relation to a multi-process/thread notifier.

The subscription keys are ugly, but it is the style of API needed for a cross process notifier (at least at a protocol level)

[kevin-dp]

I love the concept here too, however we need to consider this in relation to a multi-process/thread notifier.

The subscription keys are ugly, but it is the style of API needed for a cross process notifier (at least at a protocol level)

Could you elaborate a bit more on the problem?
With multi-process notifier, the problem is that the listener does not run on the process that registered it but rather on the notifier's process? I believe this will always be a problem with this kind of notifier API (where a callback is registered) and solving it requires changing the notifier to send a message to the subscribed processes.

I believe this is out of scope for this PR and we can implement a multi-process notifier when we move toward that architecture. Do you agree?

[samwillis]

The multi-process notifier will have a server + client component, the client receives a notification of event with a subscription key and running the callback locally. It will also use that key to unsubscribe. Hiding that behind a closure would be nice DX, as you have here! My only thought was that some of the existing machinery around the keys could be a good starting point for an initial multi process notifier.

However, you're probably right that it's out of scope, ignore me as you see fit.