Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix #3425: removeAllListeners should delete array #3431

wants to merge 1 commit into
Jump to file or symbol
Failed to load files and symbols.
+11 −13
@@ -228,15 +228,8 @@ EventEmitter.prototype.removeAllListeners = function(type) {
return this;
- var events = this._events && this._events[type];
- if (!events) return this;
- if (isArray(events)) {
- events.splice(0);
- } else {
- this._events[type] = null;
- }
+ // does not use listeners(), so no side effect of creating _events[type]

reid Jun 13, 2012

@TooTallNate The reverted behavior explicitly tries to avoid calling listeners() to avoid creating _events[type]. Is that still needed?. Regarding your proposal in joyent#3425 (comment)


TooTallNate Jun 13, 2012

I personally think that's overkill. I don't consider removeAllListeners() to be a hot code path enough to justify that, but others would need to weigh in on that as well.

+ if (type && this._events && this._events[type]) this._events[type] = null;
return this;
@@ -39,10 +39,15 @@ e1.removeAllListeners('baz');
assert.deepEqual(e1.listeners('foo'), [listener]);
assert.deepEqual(e1.listeners('bar'), []);
assert.deepEqual(e1.listeners('baz'), []);
-// identity check, the array should not change
-assert.equal(e1.listeners('foo'), fooListeners);
-assert.equal(e1.listeners('bar'), barListeners);
-assert.equal(e1.listeners('baz'), bazListeners);
+// after calling removeAllListeners,
+// the old listeners array should stay unchanged
+assert.deepEqual(fooListeners, [listener]);
+assert.deepEqual(barListeners, [listener]);
+assert.deepEqual(bazListeners, [listener, listener]);
+// after calling removeAllListeners,
+// new listeners arrays are different from the old
+assert.notEqual(e1.listeners('bar'), barListeners);
+assert.notEqual(e1.listeners('baz'), bazListeners);
var e2 = new events.EventEmitter();
e2.on('foo', listener);