Page MenuHomePhabricator

How should we deal with events in MobileFrontend's JS?
Closed, DeclinedPublic

Description

This came up while talking with Stephen about https://gerrit.wikimedia.org/r/#/c/416884/

The problem:
In various places we do things like this

var overlay = new SearchOverlay();
overlay.on( 'search-start', function ( searchData ) {});

If the "search-start" event name ever changes, we break our contract with clients.
Ideally clients would be able to access the name of the event without worrying about whether it changes.

An easy solution for this is to attach the event to the class itself.

e.g.

var overlay = new SearchOverlay();
overlay.on( SearchOverlay.START_SEARCH_EVENT, function ( searchData ) {});

While this is fine for events on the overlay itself, given our current inheritance model this becomes problematic with inherited events.

Consider that the SearchOverlay extends Overlay and the Overlay class has a hide event.

var overlay = new SearchOverlay();
overlay.on( SearchOverlay.START_SEARCH_EVENT, function ( searchData ) {});
overlay.on( Overlay.HIDE_EVENT, function () {});

Now the client has to know about events inherited from parent classes.

Stephen's suggestion to this problem is to make these events inherited by attaching these events to the prototype.

eg.

var overlay = new SearchOverlay();
overlay.on( SearchOverlay.prototype.START_SEARCH_EVENT, function ( searchData ) {});
overlay.on( SearchOverlay.prototype.HIDE_EVENT, function () {});

The problem with this approach is now there are many ways to access the constant:

Overlay.prototype.EXIT_EVENT
new Overlay().EXIT_EVENT
SearchOverlay.prototype.EXIT_EVENT
new SearchOverlay().EXIT_EVENT

While this solves the immediate problem, it is not clear how we want to do this going forward.

Jon's thoughts

Long term it feels like we'll probably want to abandon inheritance altogether and pass properties rather than rely on events.
e.g. the above code might become:

var overlay = new Overlay( {
  onHide: function () {},
  header: new SearchInput( { onSearch: function () {}  })
} );

The advantage of this is that we tie the "events" to the components that use them. We gain more power in how we can assemble our widgets (a SearchInput can now be used outside a SearchOverlay) and it's clear which component uses which method.

What about an events.js like Popups' actionTypes.js? The upside is that there are no composition concerns and coupling is loose like the event bus pattern, but the downside is that only a subset of events are applicable for a given component, convention is needed to avoid conflict, and clients still need to be aware of what events are emitted in throughout a hierarchy.

Event Timeline

+Jan

There's related confusion on which event buses we should use for which purpose too. For example:

  • mw.hook
  • mw.track
  • jQuery.on
  • OOUI EventEmitter
  • DOM events

And also View.js mixes in OO.EventEmitter, so technically all our components use that :P

Jdlrobson triaged this task as Medium priority.Aug 2 2019, 4:57 PM

Plan is to port this code to Vue.js