Skip to content

Commit 233e333

Browse files
sam-githubbnoordhuis
authored andcommitted
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: #687 Reviewed-By: Ben Noordhuis <[email protected]>
1 parent d75fecf commit 233e333

4 files changed

+68
-6
lines changed

doc/api/events.markdown

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,18 @@ Return the number of listeners for a given event.
143143
* `event` {String} The event name
144144
* `listener` {Function} The event handler function
145145

146-
This event is emitted any time a listener is added. When this event is triggered,
147-
the listener may not yet have been added to the array of listeners for the `event`.
146+
This event is emitted *before* a listener is added. When this event is
147+
triggered, the listener has not been added to the array of listeners for the
148+
`event`. Any listeners added to the event `name` in the newListener event
149+
callback will be added *before* the listener that is in the process of being
150+
added.
148151

149152

150153
### Event: 'removeListener'
151154

152155
* `event` {String} The event name
153156
* `listener` {Function} The event handler function
154157

155-
This event is emitted any time someone removes a listener. When this event is triggered,
156-
the listener may not yet have been removed from the array of listeners for the `event`.
158+
This event is emitted *after* a listener is removed. When this event is
159+
triggered, the listener has been removed from the array of listeners for the
160+
`event`.

test/parallel/test-event-emitter-add-listeners.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ var times_hello_emited = 0;
1212
assert.equal(e.addListener, e.on);
1313

1414
e.on('newListener', function(event, listener) {
15+
if (event === 'newListener')
16+
return; // Don't track our adding of newListener listeners.
1517
console.log('newListener: ' + event);
1618
events_new_listener_emited.push(event);
1719
listeners_new_listener_emited.push(listener);
@@ -23,6 +25,11 @@ function hello(a, b) {
2325
assert.equal('a', a);
2426
assert.equal('b', b);
2527
}
28+
e.once('newListener', function(name, listener) {
29+
assert.equal(name, 'hello');
30+
assert.equal(listener, hello);
31+
assert.deepEqual(this.listeners('hello'), []);
32+
});
2633
e.on('hello', hello);
2734

2835
var foo = function() {};
@@ -44,4 +51,17 @@ process.on('exit', function() {
4451
assert.equal(1, times_hello_emited);
4552
});
4653

47-
54+
var listen1 = function listen1() {};
55+
var listen2 = function listen2() {};
56+
var e1 = new events.EventEmitter;
57+
e1.once('newListener', function() {
58+
assert.deepEqual(e1.listeners('hello'), []);
59+
e1.once('newListener', function() {
60+
assert.deepEqual(e1.listeners('hello'), []);
61+
});
62+
e1.on('hello', listen2);
63+
});
64+
e1.on('hello', listen1);
65+
// The order of listeners on an event is not always the order in which the
66+
// listeners were added.
67+
assert.deepEqual(e1.listeners('hello'), [listen2, listen1]);

test/parallel/test-event-emitter-remove-all-listeners.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,15 @@ e3.on('removeListener', listener);
5757
// there exists a removeListener listener, but there exists
5858
// no listeners for the provided event type
5959
assert.doesNotThrow(e3.removeAllListeners.bind(e3, 'foo'));
60+
61+
var e4 = new events.EventEmitter();
62+
var expectLength = 2;
63+
e4.on('removeListener', function(name, listener) {
64+
assert.equal(expectLength--, this.listeners('baz').length);
65+
});
66+
e4.on('baz', function(){});
67+
e4.on('baz', function(){});
68+
e4.on('baz', function(){});
69+
assert.equal(e4.listeners('baz').length, expectLength+1);
70+
e4.removeAllListeners('baz');
71+
assert.equal(e4.listeners('baz').length, 0);

test/parallel/test-event-emitter-remove-listeners.js

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,20 @@ assert.deepEqual([listener1], e2.listeners('hello'));
4545
var e3 = new events.EventEmitter();
4646
e3.on('hello', listener1);
4747
e3.on('hello', listener2);
48-
e3.on('removeListener', common.mustCall(function(name, cb) {
48+
e3.once('removeListener', common.mustCall(function(name, cb) {
4949
assert.equal(name, 'hello');
5050
assert.equal(cb, listener1);
51+
assert.deepEqual([listener2], e3.listeners('hello'));
5152
}));
5253
e3.removeListener('hello', listener1);
5354
assert.deepEqual([listener2], e3.listeners('hello'));
55+
e3.once('removeListener', common.mustCall(function(name, cb) {
56+
assert.equal(name, 'hello');
57+
assert.equal(cb, listener2);
58+
assert.deepEqual([], e3.listeners('hello'));
59+
}));
60+
e3.removeListener('hello', listener2);
61+
assert.deepEqual([], e3.listeners('hello'));
5462

5563
var e4 = new events.EventEmitter();
5664
e4.on('removeListener', common.mustCall(function(name, cb) {
@@ -61,3 +69,21 @@ e4.on('removeListener', common.mustCall(function(name, cb) {
6169
e4.on('quux', remove1);
6270
e4.on('quux', remove2);
6371
e4.removeListener('quux', remove1);
72+
73+
var e5 = new events.EventEmitter();
74+
e5.on('hello', listener1);
75+
e5.on('hello', listener2);
76+
e5.once('removeListener', common.mustCall(function(name, cb) {
77+
assert.equal(name, 'hello');
78+
assert.equal(cb, listener1);
79+
assert.deepEqual([listener2], e5.listeners('hello'));
80+
e5.once('removeListener', common.mustCall(function(name, cb) {
81+
assert.equal(name, 'hello');
82+
assert.equal(cb, listener2);
83+
assert.deepEqual([], e5.listeners('hello'));
84+
}));
85+
e5.removeListener('hello', listener2);
86+
assert.deepEqual([], e5.listeners('hello'));
87+
}));
88+
e5.removeListener('hello', listener1);
89+
assert.deepEqual([], e5.listeners('hello'));

0 commit comments

Comments
 (0)