Bug 993342 - [marionette-js-logger] Support multiple listeners by emitting events in addition to backwards-compatible handleMessage method #7
Conversation
| assert.deepEqual(msg, sentMessage); | ||
| gotFirstListener = true; | ||
| }); | ||
| subject.on('message', function second(msg) { |
There was a problem hiding this comment.
Is it an explicit part of the EventEmitter#emit contract that listeners will be called in the order that they were registered? Perhaps a more intuitive test would poll/observe gotHandleMessage and gotFirstListener? Or, if what you're trying to do is ensure that multiple listeners will be notified, perhaps you could set a gotSecondListener flag here that also gets polled/observed?
There was a problem hiding this comment.
Yes.
http://nodejs.org/api/events.html#events_emitter_emit_event_arg1_arg2 says: Execute each of the listeners in order with the supplied arguments.
http://nodejs.org/api/events.html#events_emitter_addlistener_event_listener (same as on):
Adds a listener to the end of the listeners array for the specified event.
And the implementation explicitly uses a list and maintains the ordering (once it upgrades from a single listener). Given the existing docs and that this is the historical implementation, it seems like there is a 0% chance of this changing.
There was a problem hiding this comment.
And to clarify I really just cared that all the listeners got notified and that was the easiest way to accomplish the goal without using Promises and knowing how the event emitter is implemented. Having said that, I think it is desirable that in this use case (like all EventEmitter-looking use cases) that the ordering of calls is consistent and predictable. If it were otherwise it seems like it would be primed to increase the number of intermittent failures.
…tting events in addition to backwards-compatible handleMessage method
Bug 993342 - [marionette-js-logger] Support multiple listeners by emitting events in addition to backwards-compatible handleMessage method. r=gaye
...