emitter.listeners('ev') should return a copy, not the actual array

[isaacs]

It makes a lot of headaches in many edge cases to expose a reference to the listeners array.

It would be better if emitter.listeners('ev') returned a copy of the array, not the actual thing. Then we could change it, without worrying about consistency guarantees or breaking userland code.

[piscisaureus]

+1 +1 +1 +1

[isaacs]

Foretold in the docs on e72addc.

[adammw]

032fc42 changed readline.js to be compatible with this change, however doesn't copying the array introduce an extra overhead for when only the length of the array is required?

Perhaps a separate listenerCount function or property could be introduced that directly gets the length of the underlying array without having to copy the array first for these sort of uses.

[dynajoe]

I think the listenerCount idea might be a bit misleading. Considering that listenerCount may not be consistent with the length of a copied array. The overhead of making a shallow copy of the array seems minimal to me vs the potential for inconsistent data.

[adammw]

@joeandaverde: Slightly confused... how would there be inconsistent data? Your probably correct as I don't know too much about Node internals, but shouldn't array.length === array.slice(0).length?

[dynajoe]

You are correct indeed.

Consider this:

listeners = emitter.listeners('foo')
emitter.removeAllListeners('foo')

if you're iterating over the listeners array with the count retrieved from emitter.listenerCount then the size MAY not correspond with the size of listeners.length.

There's just more source for confusion when accessing the size of the array separately from the actual array.

[adammw]

My .listenerCount proposal was not supposed to be used if you already had listeners from the emitter.listeners(): there's no point then and you are correct that listeners.lengthshould be used when iterating over the array and definitely not .listenerCount.
However in the case where a library wants to remove itself or do some other wok when no one is listening to it, such as the readline module, the entire array is not actually needed, just the count of listeners to know if there are any listeners. As such, in these situations the overhead of a copy just to get a length seems like it could be avoided with a property/function to directly access the length.

[bnoordhuis]

Fixed in 20e12e4.

[mscdex]

I agree with adammw. There are times when having this overhead is unnecessary.

Another example would be if you just want to check if a particular listener currently exists for some event. You shouldn't be forced to obtain a whole new copy of the listeners array just to determine this.

If this commit is here to stay, would pull requests be accepted to incorporate into EventEmitter some additional functions that operate directly on the original array to reduce overhead (e.g. emitter.hasListener('event', listener))?

Or heck, how about a function that does return the original reference to the array (e.g. emitter.listenersRef('event') or emitter.listeners('event', true)) instead?