events: EventEmitter prototypal inheritance has side effects

[bnoordhuis]

$ node
> events
{ usingDomains: false,
  EventEmitter: { [Function: EventEmitter] listenerCount: [Function] } }
> function F() { events.EventEmitter.call(this) }
undefined
> F.prototype = new events.EventEmitter;
{ domain: null,
  _events: {},
  _maxListeners: 10 }
> var a = new F, b = new F
undefined
> a.on('x', function no() {})
{ domain: null,
  _events: { x: [Function: no] },
  _maxListeners: 10 }
> b._events
{ x: [Function: no] }

It's because of commit 45a13d9 introduced in v0.8.14. The commit log is light on details and doesn't reference a bug number so I can only guess what it's trying to fix. At any rate, the current behavior seems pretty broken to me.

[bnoordhuis]

As a test case:

var assert = require('assert');
var events = require('events');

function F() {
  events.EventEmitter.call(this);
}

F.prototype = new events.EventEmitter;

var a = new F;
var b = new F;
a.on('x', function no() {});

assert.equal(events.EventEmitter.listenerCount(b, 'x'), 0);

[TooTallNate]

Well what you're doing isn't proper prototypal inheritance, per say. You'd wanna use util.inherits() for that.

That said, this is indeed a common practice that I've seen throughout the node community, so perhaps we shouldn't break that.

I'm not exactly sure what the commit is fixing either, so hopefully @isaacs could shed some light on that.

[piscisaureus]

Judging by the test, it looks like the intention was so make subclasses that have an EventEmitter with listeners as their prototype inherit those listeners. I don't recall any discussion on this though, which is surprising b/c EventEmitters were already considered frozen at the time.

And indeed Ben describes a very obvious bug. Either we should just copy over listeners from the parent in the constructor, or just disable this behaviour altogether.

[simonkcleung]

The "domain"," _events" and " _maxListeners" are supposed to be the "own" properties of an instance.
So, to make these properties prototypal inheritable, they must be defined in the F.prototype, as follows:

Object.defineProperty(F.prototype,"_events",{
        get:function(){
            Object.defineProperty(this,"_events",{value:{}});
            return this._events;
        }
    });

Object.defineProperty(F.prototype,"_maxListeners",{
        get:function(){
            Object.defineProperty(this,"_maxListeners",{value:defaultMaxListeners});
            return this._maxListeners;
        },
                set:function(val){
                        Object.defineProperty(this,"_maxListeners",{value:val});
                }
    });