events: remove indeterminancy from event ordering

The order of the `newListener` and `removeListener` events with respect
to the actual adding and removing from the underlying listeners array
should be deterministic. There is no compelling reason for leaving it
indeterminate. Changing the ordering is likely to result in breaking
code that was unwittingly relying on the current behaviour, and the
indeterminancy makes it impossible to use these events to determine when
the first or last listener is added for an event.

PR-URL: https://github.com/iojs/io.js/pull/687
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

diff --git a/doc/api/events.markdown b/doc/api/events.markdown
index 3492619da77e958e941c760fab839fe13439431c..ffee7d04f393bd106c89cfe63810463d5772b5af 100644 (file)
--- a/doc/api/events.markdown
+++ b/doc/api/events.markdown
@@ -143,8 +143,11 @@ 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 a listener is added. When this event is triggered,
-the listener may not yet have been added to the array of listeners for the `event`.
+This event is emitted *before* a listener is added. When this event is
+triggered, the listener has not been added to the array of listeners for the
+`event`. Any listeners added to the event `name` in the newListener event
+callback will be added *before* the listener that is in the process of being
+added.
 
 
 ### Event: 'removeListener'
@@ -152,5 +155,6 @@ the listener may not yet have been added to the array of listeners for the `even
 * `event` {String} The event name
 * `listener` {Function} The event handler function
 
-This event is emitted any time someone removes a listener.  When this event is triggered,
-the listener may not yet have been removed from the array of listeners for the `event`.
+This event is emitted *after* a listener is removed.  When this event is
+triggered, the listener has been removed from the array of listeners for the
+`event`.

diff --git a/test/parallel/test-event-emitter-add-listeners.js b/test/parallel/test-event-emitter-add-listeners.js
index 0a1c148b8f29f81e3fe15310cd280d2dc322df72..7cc302e0b816958f03cad02187719fc3856d696f 100644 (file)
--- a/test/parallel/test-event-emitter-add-listeners.js
+++ b/test/parallel/test-event-emitter-add-listeners.js
@@ -12,6 +12,8 @@ var times_hello_emited = 0;
 assert.equal(e.addListener, e.on);
 
 e.on('newListener', function(event, listener) {
+  if (event === 'newListener')
+    return; // Don't track our adding of newListener listeners.
   console.log('newListener: ' + event);
   events_new_listener_emited.push(event);
   listeners_new_listener_emited.push(listener);
@@ -23,6 +25,11 @@ function hello(a, b) {
   assert.equal('a', a);
   assert.equal('b', b);
 }
+e.once('newListener', function(name, listener) {
+  assert.equal(name, 'hello');
+  assert.equal(listener, hello);
+  assert.deepEqual(this.listeners('hello'), []);
+});
 e.on('hello', hello);
 
 var foo = function() {};
@@ -44,4 +51,17 @@ process.on('exit', function() {
   assert.equal(1, times_hello_emited);
 });
 
-
+var listen1 = function listen1() {};
+var listen2 = function listen2() {};
+var e1 = new events.EventEmitter;
+e1.once('newListener', function() {
+  assert.deepEqual(e1.listeners('hello'), []);
+  e1.once('newListener', function() {
+    assert.deepEqual(e1.listeners('hello'), []);
+  });
+  e1.on('hello', listen2);
+});
+e1.on('hello', listen1);
+// The order of listeners on an event is not always the order in which the
+// listeners were added.
+assert.deepEqual(e1.listeners('hello'), [listen2, listen1]);

diff --git a/test/parallel/test-event-emitter-remove-all-listeners.js b/test/parallel/test-event-emitter-remove-all-listeners.js
index 1c359ce5c262b48130fa5f7467b61f66741b65ab..8c6d68ec5fa2ee35a89d101cd62b8e1b365c755b 100644 (file)
--- a/test/parallel/test-event-emitter-remove-all-listeners.js
+++ b/test/parallel/test-event-emitter-remove-all-listeners.js
@@ -57,3 +57,15 @@ e3.on('removeListener', listener);
 // there exists a removeListener listener, but there exists
 // no listeners for the provided event type
 assert.doesNotThrow(e3.removeAllListeners.bind(e3, 'foo'));
+
+var e4 = new events.EventEmitter();
+var expectLength = 2;
+e4.on('removeListener', function(name, listener) {
+  assert.equal(expectLength--, this.listeners('baz').length);
+});
+e4.on('baz', function(){});
+e4.on('baz', function(){});
+e4.on('baz', function(){});
+assert.equal(e4.listeners('baz').length, expectLength+1);
+e4.removeAllListeners('baz');
+assert.equal(e4.listeners('baz').length, 0);

diff --git a/test/parallel/test-event-emitter-remove-listeners.js b/test/parallel/test-event-emitter-remove-listeners.js
index fd699662cae052c1568ea70616308d88fa46f0aa..5d8acafc063d094774e58fb593d8494b83eb0206 100644 (file)
--- a/test/parallel/test-event-emitter-remove-listeners.js
+++ b/test/parallel/test-event-emitter-remove-listeners.js
@@ -45,12 +45,20 @@ assert.deepEqual([listener1], e2.listeners('hello'));
 var e3 = new events.EventEmitter();
 e3.on('hello', listener1);
 e3.on('hello', listener2);
-e3.on('removeListener', common.mustCall(function(name, cb) {
+e3.once('removeListener', common.mustCall(function(name, cb) {
   assert.equal(name, 'hello');
   assert.equal(cb, listener1);
+  assert.deepEqual([listener2], e3.listeners('hello'));
 }));
 e3.removeListener('hello', listener1);
 assert.deepEqual([listener2], e3.listeners('hello'));
+e3.once('removeListener', common.mustCall(function(name, cb) {
+  assert.equal(name, 'hello');
+  assert.equal(cb, listener2);
+  assert.deepEqual([], e3.listeners('hello'));
+}));
+e3.removeListener('hello', listener2);
+assert.deepEqual([], e3.listeners('hello'));
 
 var e4 = new events.EventEmitter();
 e4.on('removeListener', common.mustCall(function(name, cb) {
@@ -61,3 +69,21 @@ e4.on('removeListener', common.mustCall(function(name, cb) {
 e4.on('quux', remove1);
 e4.on('quux', remove2);
 e4.removeListener('quux', remove1);
+
+var e5 = new events.EventEmitter();
+e5.on('hello', listener1);
+e5.on('hello', listener2);
+e5.once('removeListener', common.mustCall(function(name, cb) {
+  assert.equal(name, 'hello');
+  assert.equal(cb, listener1);
+  assert.deepEqual([listener2], e5.listeners('hello'));
+  e5.once('removeListener', common.mustCall(function(name, cb) {
+    assert.equal(name, 'hello');
+    assert.equal(cb, listener2);
+    assert.deepEqual([], e5.listeners('hello'));
+  }));
+  e5.removeListener('hello', listener2);
+  assert.deepEqual([], e5.listeners('hello'));
+}));
+e5.removeListener('hello', listener1);
+assert.deepEqual([], e5.listeners('hello'));