RFC: EventController proposal

[ashishpuliyel]

Updated Wed 9 Feb
I've updated the explanation, and simplified the API.
Updated Wed 10 Feb
Better use of generics? Possibly?

What is this?

It's a pattern, and a standard for how we can communicate between modules.
It's a simplified event-emitter/pub-sub with the addition that in addition to hooking into the next invocation as normal, you can also pull the last/latest value from it. This is useful particularly in the case of the CMP where many consumers want to know if consent has already been set, and if so what it is, and if not, a callback or promise for when it will be.

I've written this to support both callbacks or promises depending on what you need, since different parts of our code use both styles. (More about this below, but this isn't important, it's just something I added for fun. If the consensus is that this isn't a good idea we don't have to keep this 'feature', and can simplify it down to consistently just use callbacks or return promises).

I've not done this here, but the point is partially that by having a place where we define standard interface for these events, we can define the type of events and event data as narrowly as we like here.

The obvious use is for CMP consent change, but we could use it for changes of comment pages, or updates to the liveblog.

• on method receives a callback that is called on every future event
• next If a callback is passed it's executed once, on the next event
  If no callback is passed it returns a promise that resolves on the next event.
• after If event has not previously been dispatched, this behaves the same as next.
  If event has previously occurred it resolves/calls back immediately with the last value.
• dispatch Used by the originator to actually publish the events

How is it used?

Example 1
boot.js currently does this:

    let recordedConsentTime = false;
    onConsentChange(() => {
        if (!recordedConsentTime) {
            recordedConsentTime = true;
            cmp.willShowPrivacyMessage().then(willShow => {
    ...

The let recordedConsentTime = false stuff is here just because onConsentChange happens every time consent changes, but we only care the first time.

If we were to adopt this proposal it could be updated to look like this:

    EventController.getPublisher('consentChange').after(()=> {
        cmp.willShowPrivacyMessage().then(willShow => {
    ...

Example 2
We have a file called inital-consent-state in frontend. This proposal would let us replace it with this:

let consentState = await EventController.getPublisher('consentChange').after()

• If consent is already set it resolves immediately with the latest value. If it's not yet set, it will resolve when it is.
• As in the previous example I'm using the after method, but here as I'm not passing it a callback so it returns a promise I can await. Both after and next will return a promise if not passed a callback. The third subscriber event: on only accepts a callback because it will perform the action for all future events (and a promise can only resolve once).

Is this just observables but less flexible?

Yeah. But's like 800bytes gzipped, and does what we need? Maybe?

Is this just eventEmitter (aka mediator), but without off /remove listener events?

Basically? Frontend formally used eventEmitter for similar reasons. The problems it was there to solve haven't entirely gone away in DCR, so it makes sense to bring back some version of it. True this doesn't support off/remove listener, and we could add those if they're useful. We could also use the native addeventlistener API instead of keeping them in an array, under the hood. But I think those are implementation details I'm not that fussed about at this point.

If we recognise that this is a use-case we have in our code base and this is a generalised pattern that could address it, I think the important bit / useful part here is the controller. It becomes the place we document / define the types of events and what kind of data they're expecting. Providing a clearer contract in this interface layer between modules that publisher events and those that listen for them.

type CallbackFunction = (value: unknown) => void;

class EventPublisher<EventDataType> {
    private _subscribers:CallbackFunction[] = [];
    private _onNext:CallbackFunction[] = [];
    private _promises: ((arg: EventDataType) => void)[] = [];

    declare private _lastValue:EventDataType;
    private _hasUpdated = false;

    /**
     * The callback passed to this method is called on _every future_ event.
     */
    on(callback:CallbackFunction):void {
        this._subscribers.push(callback);
    }

    /**
     * If a callback is passed it's executed once, on the _next_ event
     * If no callback is passed it returns a promise that resolves on the _next_ event.
     */
    next(callback: CallbackFunction): void;
    next(): Promise<EventDataType>;
    next(callback?: CallbackFunction): Promise<EventDataType>|void {
        if (callback) {
            this._onNext.push(callback);
        } else {
            return new Promise((resolve)=> {
                this._promises.push(resolve);
                return resolve;
            })
        }
    }
    /**
     * If event has not previously been dispatched, this behaves the same as next().
     *      If a callback is passed it's executed once, on the _next_ event
     *      If no callback is passed it returns a promise that resolves on the _next_ event.
     * If event has previously occoured:
     *      If a callback is passed it's called immediately with the last value
     *      If no callback is passed a resolved promise is returned with the last value.
     */
    after(callback: CallbackFunction): void;
    after(): Promise<EventDataType>;   
    after(callback?:CallbackFunction): Promise<EventDataType>|void {
        if (this._hasUpdated) {
            if (callback) {
                callback(this._lastValue);
            } else {
                return Promise.resolve(this._lastValue);
            }
        } else {
            if (callback) {
                this.next(callback);
            } else {
                return this.next();
            }
        }
    }

    /**
     * This is the function that's called with data which is sent to subscribers.
     */
    dispatch(eventData:EventDataType) {
        this._hasUpdated = true;
        this._lastValue = Object.freeze(Object.assign({}, eventData)); 
        this._subscribers.forEach((cb=> {
            cb(this.lastValue);
        }));

        this._onNext.forEach(cb=> {
            cb(this.lastValue);
        });
        this._onNext.length = 0;

        this._promises.forEach(resolve=> {
            resolve(this.lastValue);
        });
        this._promises.length = 0;
    }

    /**
     * Syncronously check if this has updated at least once.
     */  
    get hasUpdated () {
        return this._hasUpdated;
    }
    get lastValue() {
        return this._lastValue;
    }

}

/* 
I've Defined EventController as a class that exports a singleton. But it 
doesn't need to do that. EventPublisher as a class is a bit more useful, 
since we could make it abstract and then have ConsentChangePublisher, 
CommentsPagePublisher that implement it with typed event data.
*/

const EVENT_NAMES = ['consentChange', 'commentsPageChange', 'liveBlogUpdate'] as const;
type EventName = typeof EVENT_NAMES[number];
class EventController {

    eventPublishers: { [K in EventName]: EventPublisher<unknown>; };
    
    constructor() {
        this.eventPublishers = {
            'consentChange' : new EventPublisher(),
            'commentsPageChange' : new EventPublisher(),
            'liveBlogUpdate' : new EventPublisher()
        };
    }

    static init():EventController {
        return window.guardian.pageevents ||= new EventController(); 

    }

    static getPublisher(eventName: EventName): EventPublisher<unknown>{
        return this.init().eventPublishers[eventName];
    }
}

[mxdvl]

I quite like this, especially the size and the typing of possible messages/events.

Is there any way we could make this even simpler? I would be tempted to keep an array of events that have happened, and infer first, next and last from it. I’d be tempted to simplify the api with only a on method that takes options. I’m wondering how far we can get with EventTarget and maybe keeping a list of past events for people who want to look at what has already happened.

How would we handle implementation of event controllers not overriding each other on the guardian.pageevents object? Maybe we could implement a prefix that’s defined centrally. This way you can be safe in knowing there won’t be a clash.

export const CONTROLLERS = ['consent-management-platform', 'comments'] as const;
export type Controllers = typeof CONTROLLERS[number];

[ashishpuliyel]

How would we handle implementation of event controllers not overriding each other

There is supposed to be only one eventController. so init(), saves itself to window.guardian.pageevents but only saves itself if it's not defined otherwise it returns the instance that's there. This makes it safe to use across modules from different bundles.

Is there any way we could make this even simpler?

on(type) might well be cleaner and I'd be happy with that. But I'm keen to keep a separation for the promise version (which can only be next or afterFirst and happen once), vs the callback (future which can happen on all future events). And given the use of similar patterns in the codebase I think there's a desire for both promises and callbacks in different places.

[mxdvl]

• on → all future events
• next→ promise for the next event
• last → promise for the last (or first?) oh my this is hard isn’t it 😆

[arelra]

I like the route of a global simple class that doesn't use observables - unless we want composability over streams but that could be done by the consumer.

I agree that the naming is really important here and think there could be some improvement.

Naming

Could we reuse terminology that is already widespread for example:

• for the producer: publish, dispatch, emit
• for the consumer: subscribe, listen

My preference would be for publish, subscribe but thats just imo.

Also should your examples use the opposite producer/consumer terminology?

e.g.

    EventController.publisher('consentChange').onAfterFirst(()=> {
        cmp.willShowPrivacyMessage().then(willShow => {
    ...

becomes:

    EventController.subscribe('consentChange').onAfterFirst(()=> {
        cmp.willShowPrivacyMessage().then(willShow => {
    ...

Functionality

I'm not sure we need the lastValue, afterFirst?

It would be good to see example use cases?

[ashishpuliyel]

@mxdvl / @arelra I've simplified this somewhat. It's now just on, next, after for the subscribers; and dispatch for the publisher.

[mxdvl]

I like this more! Nice one. I think you'd want to use generics to specify the type of what is passed to the callbacks and returned by the promises, but otherwise 👌

The type of resolve of a Promise<T> where <T extends unknown> is (arg: T) => void.

[ashishpuliyel]

How about now. I'm passing in the type to the class. Because each event Publisher should return a single type of eventData, adn that's the only 'variable', and I use that type throughout, both in the promises and in the callbacks.

[mxdvl]

You could also pass that as a type to the CallbackFunction:

type CallbackFunction<T extends unknown> = (value: T) => void;

And that type also matches the resolve–which when you think of it makes a lot of sense!

[oliverlloyd]

This is super interesting!

One thing: Is there a way to expose a more reacty version of this api? Most client side code in DCR is built using react and trying to integrate imperative listeners or promises into react code is famously hard and we don't want developers to have to repeatedly solve these problems each time they want to use this api.

We could create a custom hook to do this in DCR but perhaps it's better (for other teams) if this was already defined and exported from commercial core?

The AB test framework is an example for exporting both vanilla js and react versions of an api.

What's wrong with listeners in react?

The problem is react is declarative. It expects your code be able to be rerun (re-rendered) with no side effects. But listeners are imperative and should only be set once, so you need to manage this with guards. This is especially problematic when the listeners are tied to dom elements that have been rendered by react (you then need to start using refs) but it's also just generally confusing and makes it hard to reason about state.

What's wrong with promises?

As above, react is declarative and has its own built in methods for handling state changes. When you combine an async function that returns on its own timescale outside of react's lifecycle it can be hard to reason about changes. This can be ameliorated with async/await but in the words of Dan Abramov: "Async/await in components is a bugfest"

https://twitter.com/dan_abramov/status/1246432321579950081