Node: events: adding multiple listeners at once

Created on 30 Sep 2017  Â·  15Comments  Â·  Source: nodejs/node

I often find myself in need of registering to multiple events using the same handler. .on currently only supports on event per call, so this most concise way to write it is

````js
const errorHandler = (err) => {
// handle error
}

server.on('error', errorHandler)
server.on('clientError', errorHandler)
server.on('tlsClientError', errorHandler)
````

Ideally, I think .on and possibly .once should provide a way to add multiple listeners in one call, like for example using jQuery-style space-separated event names:

js server.on('error clientError tlsClientError', (err) => { // handle error })

Above would be a breaking change for event names containing spaces, so an alternative could be to take the event names from an array:

js server.on(['error', 'clientError', 'tlsClientError'], (err) => { // handle error })

events feature request

Most helpful comment

I've often thought about making improvements to this area but there are some challenges to keep in mind. I ran into some of these when introducing the prependOn and prependOnce methods and primarily deal with backwards compatibility and compatibility with things like the standalone readable-streams module. If changes are made, I strongly suggest adding new methods and not touching the existing methods.

Getting back to the original question, the approach I've considered is more along the lines of....

emitter.addListeners({
  error() { /* ... */ },
  clientError() { /* ... */ },
   tlsClientError() { /* ... */ }
});

emitter.addOnceListeners({
  error() { /* ... */ },
  clientError() { /* ... */ },
   tlsClientError() { /* ... */ }
});

function fn() {}

emitter.addListeners({
  error: fn,
  clientError: fn,
  tlsClientError: fn
});

All 15 comments

Just thinking about the polymorphism that comes with allowing arrays – would emitter.onAnyOf(array, fn) (or any other new method name) work for you just as well?

I've often thought about making improvements to this area but there are some challenges to keep in mind. I ran into some of these when introducing the prependOn and prependOnce methods and primarily deal with backwards compatibility and compatibility with things like the standalone readable-streams module. If changes are made, I strongly suggest adding new methods and not touching the existing methods.

Getting back to the original question, the approach I've considered is more along the lines of....

emitter.addListeners({
  error() { /* ... */ },
  clientError() { /* ... */ },
   tlsClientError() { /* ... */ }
});

emitter.addOnceListeners({
  error() { /* ... */ },
  clientError() { /* ... */ },
   tlsClientError() { /* ... */ }
});

function fn() {}

emitter.addListeners({
  error: fn,
  clientError: fn,
  tlsClientError: fn
});

Just thinking about the polymorphism that comes with allowing arrays – would emitter.onAnyOf(array, fn) (or any other new method name) work for you just as well?

I'm not really happy with introducing additional methods. What issues are foreseeing when adding array support?

Currently, when registering a listener with an array, a implicit toString conversion is ran, so adding a ['a','b'] listener results in a event named 'a,b' being actually registered, which is likely not what the user wanted. Still I'd say a change to array handling should be semver-major.

@silverwind There could be performance issues due to the differing types being passed to the same function in addition to the added argument type checking.

If this should get implemented, I would also prefer to see it as a separate function.

To be honest, I think that this is a common issue to everyone. If we can agree on the relevance of the issue, maybe I could consider trying to work on this.

By the way, I took at look at the code. It does not seem impossible to change, but this is an opinion of a beginner. Do you think that this is a beginner-level issue?

The key issues I see with allowing the polymorphic signature are Performance and Discoverability... that is...

  1. Performance (as @mscdex points out). The additional type checking does not come free.
  2. Discoverability... that is, one would have to know exactly which versions of Node.js the polymorphic signature has been ported to in order to reliably use it (vs. just checking for the existence of the new method)

There is another approach to this that I've been considering... the ability to register a generic event sink for any event emitter... e.g.

function mySink(event, ...args) {
  switch (event) {
    case 'foo': console.log('foo'); break;
    case 'bar': console.log('bar'); break;
  }
}

const ee = new EventEmitter();
ee.addEventSink(mySink);
ee.emit('foo');
ee.emit('bar');

I think I'm convinced that the downsides of polymorphism are outweighting the benefits. So for the question on what to implement, I'd propose:

  • emitter.onMultiple(events, handler), where events is an array and handler recieves ...args
  • emitter.onAny(handler) where handler receives event, ..args (@jasnell's sink idea)

I'm not sure of the value of similar methods for .once, so I think it's better left out for now.

@Giovarco I guess this shouldn't be too hard to implement.

If we went with the handler approach, there'd definitely be no need for a once counterpart.

@silverwind Ok then tomorrow I'll give it a shot. emitter.onMultiple(events, handler) use is ok, but I didn't quite understand the use of emitter.onAny(handler).

If you don't concern the performance, you may use ES6 Proxy to achieve what you want in this way:

const EE = require("events");

function EP(target){
    return new Proxy(target, proxyHandler);
}
const proxyHandler = {
    set(target, name, handler){
    target.addListener(name, handler);
    }
};

let e = new EE();

let p = EP(e);

p.error1 = p.error2 = p.error3 = function(){
    console.log("This is an error");    
}

e.emit("error1");
e.emit("error2");
e.emit("error3");

To be honest, the event modules is a little bit murky for me. I did not find a "node API for developers" that explain the inner workings of the module. Anyway, this is how I tried to add the feature:

events.js

EventEmitter.prototype.onMultiple = function onMultiple(eventList, handler) {

  for(let i = 0; i < eventList.length; i++) {
    this.on(eventList[i], handler);
  }

};

This is my test:

test-event-emitter-onMultiple.js

'use strict';
const common = require('../common');

// This test ensures that it is possible to add a listener to multiple events at once

const assert = require('assert');
const EventEmitter = require('../../lib/events');
const myEE = new EventEmitter();

async function main() {

    // Input
    const inputArray = ["a", "b", Symbol('symbol')];
    const emptyFunction = function() {};

    // Test
    await myEE.onMultiple(inputArray, emptyFunction);
    const outputArray = await myEE.eventNames();

    // Assert
    await assert.deepStrictEqual(inputArray, outputArray, "The listener did not get binded to multiple events");
}

main()

I suspect that I'm far from a good implementation, but it works at least. Please don't be too strict with me ^_^"

Anyone have examples other than the one @silverwind posted? Going over my own code, the places where I add the same listener for different events can be counted on one hand.

Anyone have examples

For core modules, it's probably only useful for error handling. One more example:

process.on('uncaughtException', handler)
process.on('unhandledRejection', handler)

Other than that, I could've used in on modules that fire many different events like chokidar.

@bnoordhuis I you're working on implementing this feature, please read this https://github.com/nodejs/node/pull/16169 . I'm already working on it, so you can propose changes and we don't get duplicate code.

Closing this for lack of support. Also see https://github.com/nodejs/node/pull/16169#issuecomment-346425028.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

addaleax picture addaleax  Â·  146Comments

benjamingr picture benjamingr  Â·  135Comments

nicolo-ribaudo picture nicolo-ribaudo  Â·  147Comments

silverwind picture silverwind  Â·  113Comments

AkashaThorne picture AkashaThorne  Â·  207Comments