Remove !events and use appropriate patterns

[molily]

result = undefined
@publishEvent '!someEvent', 'someArg', (_result) ->
  result = _result
doSomethingWith(result)

This pattern is used all over Chaplin. I admit I introduced and supported it. I was wrong. It’s a horrible misuse of Publish/Subscribe.

Pub/Sub isn’t about sending requests and receiving a response in a decoupled way. It’s about broadcasting events. An event should notify the listeners that something has happened, not trigger some behavior. You don’t expect a response from publishing an event, even less a synchronous response. Also a basic rule of event-based communication is that an emitter doesn’t have to know if anyone is listening.

A Backbone.Events based Pub/Sub is not the right pattern for this behavior. We should find a pattern that fits better. At the moment I’m thinking about the Command and Request/Response patterns like they are implemented in Backbone.Wreqr: https://github.com/marionettejs/backbone.wreqr#commands-and-request--response

This could look like:

routed = @request 'router:route', '/foo'

If we have async requests (I don’t think there are any in Chaplin core), the response value should be a Promise.

[paulmillr]

+1, but I don’t think this should have public API. awesome for internal chaplin stuff.

[mehcode]

Something like:

# Register a handler
@receive 'router:route', -> 
  # ...

# Send a message / make a request 
@send 'router:route', '/'

I like the parallel. We could have these methods be defined on the EventBroker object. However this is a pretty fundamental change.

@paulmillr We'd need to have this have a user-visible API to promote the pattern and allow users to send route requests, etc.

[molily]

See also http://appendto.com/blog/2013/04/request-response-pattern-in-postal-js/

[paulmillr]

What is the expected API for this [method names]?

How about setRequestHandler and sendRequest? send looks very similar to publishEvent.

[mehcode]

initialize: ->
  # Declaring 
  Chaplin.commander.setHandler 'router:reverse', (name) -> # ...
  @setRequestHandler 'router:reverse', (name) -> # ...

  # Using
  Chaplin.commander.request 'reverse', 'log'
  @sendRequest 'reverse', 'log' 

We could also reuse Chaplin.mediator for this.

@sendRequest seems a bit verbose though... I see your point about send. How about what @molily suggested, just @request.

[paulmillr]

Actually, after some review, i’m kinda “meh” on this issue.

1. We can move internal chaplin stuff that is accessed via !events to helpers. Then we can remove them from docs and advice ordinary users to use just helpers.
2. Then we won’t need to implement any additional entities in the system and introduce one more “events-like” bus of communication.
3. Which will allow us to not place additional two methods on all EventBroker mixers.

[molily]

Backbone.Events is indeed a good backend for callback/handler lists and it’s not nice to implement yet another event bus, I agree. But I think the implementation of Request/Response can be small and this pattern is useful for decoupled communication in general.

Even if we hide the pattern I mentioned from ordinary users it’s still a bad coding practice. It shouldn’t be in the Chaplin code. People reading the Chaplin code or people who would like to overwrite Chaplin internals will still stumble upon this.

[YAndrew91]

@molily, +1 to all what you've said. I really hate this ! requests, from the very moment I started using Chaplin (nearly 6 months ago). In addition to above, for example, in chaplin-auth (or facebook example) you place a current user right on the mediator, so every external module can rewrite or change it... Isn't it similar to a window object? =) IMO, we should access such data only using requests, so an accessor will able to do only some predefined type of work.

[paulmillr]

BTW We use this kind of technique (get variables / values) for a very small set of events. I was thinking about that: Rename almost all !events to events without bang. It makes perfect sense for router events.

[knuton]

@paulmillr Can you explain in how far '!router:route' and friends are legitimate events? As I understand it, they don't inform about some kind of event that occurred. Rather, they really are requests for specific actions from a specific source.

I just spent some time browsing the web for discussions about Chaplin, and this discussion of bang-events is one of the things I found. They seem to be source for some confusion, and I can see why.

The bang-events are sometimes, but not always, making use of implementation details of the event system that cause a synchronous execution of the callback, allowing for code as in @molily's example at the top of this issue. This is obviously not nice.

While it may be true that in Chaplin's core this pattern is not used much, I assume that programmers working with the framework may run into situations where they want to make requests to other modules. The '!logout' "event" discussed in the StackOverflow thread is just one obvious example. I think it might be a good idea to give them a hand by including a proper solution for this kind of situation.

[paulmillr]

We don’t wait for callback in router:route since master. In the past, callback received routed param, now it will throw an error if something will go wrong.

I suggest to use bang events only for stuff that requires callback and can be replaced with request-response. And which is listened only by one listener all the time. router:reverse, for example.

[paulmillr]

How about this? Super-simple, just what we need.

• mediator.handlers = {}
• resp: mediator.handlers.changeURL = @changeURLHandler
• req: mediator.handlers.changeURL()

I had evaluated everything, we need it only for !router:reverse and !composer events.

Everything else look like ordinary events with subscribers and they are cool.

[paulmillr]

gh-662