Skip to content

Commit

Permalink
events: handle inherited properties properly
Browse files Browse the repository at this point in the history
As of now, the events module considers inherited properties as one of the
valid types, by default.

    > process.version
    'v3.0.0'
    > events.EventEmitter.listenerCount(new events.EventEmitter(), 'toString')
    1

This patch makes sure that the inherited properties are considered as
normal types and they will not be counted unless explicitly added.

    > process.version
    'v4.0.0-pre'
    > events.EventEmitter.listenerCount(new events.EventEmitter(), 'toString')
    0
    > const emitter = new events.EventEmitter();
    undefined
    > emitter.on('toString', function() {});
    EventEmitter {
      domain:
       Domain {
         domain: null,
         _events: { error: [Function] },
         _eventsCount: 1,
         _maxListeners: undefined,
         members: [] },
      _events: { toString: [Function] },
      _eventsCount: 1,
      _maxListeners: undefined }
    > events.EventEmitter.listenerCount(emitter, 'toString')
    1
  • Loading branch information
thefourtheye committed Nov 15, 2015
1 parent 84f0964 commit 85b2939
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 14 deletions.
14 changes: 7 additions & 7 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ EventEmitter.init = function() {
}

if (!this._events || this._events === Object.getPrototypeOf(this)._events) {
this._events = {};
this._events = Object.create(null);
this._eventsCount = 0;
}

Expand Down Expand Up @@ -198,7 +198,7 @@ EventEmitter.prototype.addListener = function addListener(type, listener) {

events = this._events;
if (!events) {
events = this._events = {};
events = this._events = Object.create(null);
this._eventsCount = 0;
} else {
// To avoid recursion in the case that type === "newListener"! Before
Expand Down Expand Up @@ -285,7 +285,7 @@ EventEmitter.prototype.removeListener =

if (list === listener || (list.listener && list.listener === listener)) {
if (--this._eventsCount === 0)
this._events = {};
this._events = Object.create(null);
else {
delete events[type];
if (events.removeListener)
Expand All @@ -308,7 +308,7 @@ EventEmitter.prototype.removeListener =
if (list.length === 1) {
list[0] = undefined;
if (--this._eventsCount === 0) {
this._events = {};
this._events = Object.create(null);
return this;
} else {
delete events[type];
Expand All @@ -335,11 +335,11 @@ EventEmitter.prototype.removeAllListeners =
// not listening for removeListener, no need to emit
if (!events.removeListener) {
if (arguments.length === 0) {
this._events = {};
this._events = Object.create(null);
this._eventsCount = 0;
} else if (events[type]) {
if (--this._eventsCount === 0)
this._events = {};
this._events = Object.create(null);
else
delete events[type];
}
Expand All @@ -355,7 +355,7 @@ EventEmitter.prototype.removeAllListeners =
this.removeAllListeners(key);
}
this.removeAllListeners('removeListener');
this._events = {};
this._events = Object.create(null);
this._eventsCount = 0;
return this;
}
Expand Down
28 changes: 22 additions & 6 deletions test/parallel/test-event-emitter-listener-count.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,34 @@
'use strict';

const common = require('../common');
require('../common');
const assert = require('assert');
const EventEmitter = require('events');

const emitter = new EventEmitter();
emitter.on('foo', function() {});
emitter.on('foo', function() {});
emitter.on('baz', function() {});

function noop() {}

emitter.on('foo', noop);
emitter.on('foo', noop);
emitter.on('baz', noop);
// Allow any type
emitter.on(123, function() {});
emitter.on(123, noop);

assert.strictEqual(EventEmitter.listenerCount(emitter, 'foo'), 2);
assert.strictEqual(emitter.listenerCount('foo'), 2);
assert.strictEqual(emitter.listenerCount('bar'), 0);
assert.strictEqual(emitter.listenerCount('baz'), 1);
assert.strictEqual(emitter.listenerCount(123), 1);

// The inherited properties should not be counted towards the actual
// listeners count
assert.strictEqual(EventEmitter.listenerCount(emitter, 'toString'), 0);

// when we add a new listener with the name of an inherited property, it should
// accept it
emitter.on('toString', noop);
assert.strictEqual(EventEmitter.listenerCount(emitter, 'toString'), 1);

// after removing a listener with the name of an inherited property, the count
// should reduce by one
emitter.removeListener('toString', noop);
assert.strictEqual(EventEmitter.listenerCount(emitter, 'toString'), 0);
2 changes: 1 addition & 1 deletion test/parallel/test-event-emitter-listeners-side-effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ assert(fl.length === 1);
assert(fl[0] === assert.fail);

e.listeners('bar');
assert(!e._events.hasOwnProperty('bar'));
assert(!e._events['bar']);

e.on('foo', assert.ok);
fl = e.listeners('foo');
Expand Down

0 comments on commit 85b2939

Please sign in to comment.