Skip to content

Commit

Permalink
events: remove indeterminancy from event ordering
Browse files Browse the repository at this point in the history
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: #687
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
sam-github authored and bnoordhuis committed Feb 2, 2015
1 parent d75fecf commit 233e333
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 6 deletions.
12 changes: 8 additions & 4 deletions doc/api/events.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,18 @@ 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'

* `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`.
22 changes: 21 additions & 1 deletion test/parallel/test-event-emitter-add-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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() {};
Expand All @@ -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]);
12 changes: 12 additions & 0 deletions test/parallel/test-event-emitter-remove-all-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
28 changes: 27 additions & 1 deletion test/parallel/test-event-emitter-remove-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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'));

0 comments on commit 233e333

Please sign in to comment.