Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

Events doc fixes #8883

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions doc/api/events.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ added and `'removeListener'` when a listener is removed.
### emitter.addListener(event, listener)
### emitter.on(event, listener)

Adds a listener to the end of the listeners array for the specified event.
Adds a listener to the end of the listeners array for the specified `event`.
No checks are made to see if the `listener` has already been added. Multiple
calls passing the same combination of `event` and `listener` will result
in the `listener` being added multiple times.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this its obvious that when you add a listener it adds a listener. Always. But, it is clear.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but what wasn't obvious (per the issue I was addressing) is that a single listener can be added multiple times.


server.on('connection', function (stream) {
console.log('someone connected!');
Expand Down Expand Up @@ -65,6 +68,11 @@ Remove a listener from the listener array for the specified event.
// ...
server.removeListener('connection', callback);

`removeListener` will remove, at most, one instance of a listener from the
listener array. If any single listener has been added multiple times to the
listener array for the specified `event`, then `removeListener` must be called
multiple times to remove each instance.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is less obvious.


Returns emitter, so calls can be chained.

### emitter.removeAllListeners([event])
Expand Down Expand Up @@ -121,8 +129,9 @@ Return the number of listeners for a given event.
* `event` {String} The event name
* `listener` {Function} The event handler function

This event is emitted any time someone adds a new listener. It is unspecified
if `listener` is in the list returned by `emitter.listeners(event)`.
This event is emitted any time someone adds a listener, even if the added
listener was previously already added to the listeners array for the given
`event`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is borderline misleading... it seems to me to almost suggest that .on() doesn't really add listeners if they are already in the array, something which isn't true (as you emphasized above).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps...

This event is emitted any time someone adds a listener. When the event is triggered, the listener may not yet have been added to the list returned by emitter.listeners(event).
If any single listener happens to be added multiple times (see emitter.addListener), the newListener event will trigger once for each time it is added.

Truthfully, tho, the best solution would be to simply prevent listeners from being added multiple times.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. event api is frozen
  2. its a feature
  3. if you don't like it... you can always do a wrapper, call listeners(event), see if your listener is in the array, and then don't add it if you don't want to.

So, you can get the no-multiples behaviour from the current API, whereas if it didn't allow multiples, you couldn't get the current behaviour.



### Event: 'removeListener'
Expand Down