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

newListener event #8853

Closed
smart--petea opened this issue Dec 10, 2014 · 5 comments
Closed

newListener event #8853

smart--petea opened this issue Dec 10, 2014 · 5 comments

Comments

@smart--petea
Copy link

In http://nodejs.org/api/events.html is written:

Event: 'newListener'#
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).

From this snippet I learnt that if listener is in listeners the newListener event is not fired.

I did

var EE = require('events');
var ee = new EE;

var fnOk = function(){}

ee.on('newListener', function(){
  console.log('NEW LISTENER', arguments);
})

ee.on('ok' , fnOk); //fnOk is in ee.listeners('ok')
ee.on('ok', fnOk);

event newListener is fired twice in spite of fact that listener is just in listeners array.

May be I am wrong.

@skenqbx
Copy link

skenqbx commented Dec 16, 2014

This is either a bug in the EE or in the docs.
There are no checks in addListener() if the listener is already registered for the specific event. Additionally, removeListener() has to be called twice to remove both listeners.

@jasnell
Copy link
Member

jasnell commented Dec 16, 2014

Well, to be fair, the docs say that the behavior is "unspecified" if the listener is already added. It does not actually say that the event won't be fired multiple times. Still, this behavior feels a bit wonky and ought to be looked at.

jasnell added a commit to jasnell/node-joyent that referenced this issue Dec 16, 2014
nodejs#8853 ... specifically,
acknowledge in the docs that EE.addListener(...) does not check to see
if the given listener has already been added, and that newListener will
be called regardless of whether an already existing listener is added
twice.
@sam-github
Copy link

You misread the admittedly poorly worded docs: https://github.com/joyent/node/pull/8883/files#r21945241

@jasnell
Copy link
Member

jasnell commented Dec 17, 2014

Hmm.. hard to misread something so poorly written in the first place ;-). To be certain, the docs say nothing about the listener being added multiple times. I'll admit, it tends towards a "doctor, it hurts when I do this" kind of situation, but the docs are obviously lacking here.

jasnell added a commit to jasnell/node-joyent that referenced this issue Dec 17, 2014
Per nodejs#8853, improvements to
documentation for addListener, removeListener and newListener to
reflect the fact that addListener allows any single listener to be
added multiple times (which is silly, but needs to documented)
jasnell added a commit to jasnell/node-joyent that referenced this issue Dec 19, 2014
Improvements to events.markdown documentation per
nodejs#8853 (same fix as before but
rebased on joyent/v0.10
tjfontaine pushed a commit that referenced this issue Dec 22, 2014
Clarify that adding or removing a listener is not idempotent.

RE: #8853
PR: #8911
PR-URL: #8911
Signed-off-by: Timothy J Fontaine <tjfontaine@gmail.com>
jasnell added a commit to jasnell/node-joyent that referenced this issue Jan 5, 2015
Clarify that adding or removing a listener is not idempotent.

RE: nodejs#8853
PR: nodejs#8911
PR-URL: nodejs#8911
Signed-off-by: Timothy J Fontaine <tjfontaine@gmail.com>
jasnell added a commit to jasnell/node-joyent that referenced this issue Jan 5, 2015
Clarify that adding or removing a listener is not idempotent.

RE: nodejs#8853
PR: nodejs#8911
PR-URL: nodejs#8911
Signed-off-by: Timothy J Fontaine <tjfontaine@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jun 29, 2015

Resolved in the io.js docs. Will pick up the change from there in the converged repo

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants