cabal-club / cabal-client

interface for writing peer-to-peer chat clients
GNU General Public License v3.0
95 stars 14 forks source link

improve granularity of listeners #23

Closed cblgh closed 4 years ago

cblgh commented 4 years ago

khubo and nick, and myself while starting to patch cabal-irc, have felt a need to have more granular listeners than on("update")

as an example: this method, from cabal-desktop, would be split up into several listeners

i haven't had a sit down to think properly about which ones yet but maybe:

it could also be interesting to include a delta, the change in state that occurred in the current update, that can optionally be used (that way client implementations don't necessarily need to be in a reactive framework like react or vue. introducing a delta could however cause weird errors stemming from out of order updates.

@khubo @nikolaiwarner chime in on your recent impressions and thoughts when developing cabal-desktop so excellently these past 2-3 weeks. which specific cases were aching for more granular listeners?

khubo commented 4 years ago

"messages" tops the list from cabal-desktop perspective. Since whenever a new message comes we have to update new message count, show a notification badge near the cabal etc. We are doing that for all sort of updates now. Also, peers gets connected/disconnected frequently, which also triggers too many updates.

nikolaiwarner commented 4 years ago

@cblgh those are all great @khubo yeah an event for peer changes would be a real good one

also how about:

@cblgh should we make individual issues for each one so we can split up the effort?

cblgh commented 4 years ago

@nikolaiwarner if you want to, otherwise i figure just implementing them all in one go makes more sense (at least as regards cabal-client) :~

nikolaiwarner commented 4 years ago

cblgh commented 4 years ago

shower thought: each invocation of this._emitUpdate() can be changed to this.emitUpdate("<type>", {<payload>})

and redefining the emitUpdate function as

  _emitUpdate(type, payload) {
    if (!this.disableGlobalUpdate) this.emit('update', this)
    this.emit(type, payload)
  }

i still think there's value in having the single update emitter, as it simplifies anyone's first client.