Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

EventEmitter.listeners() should not auto-create this._events #3803

Closed
bnoordhuis opened this issue Jul 31, 2012 · 6 comments
Closed

EventEmitter.listeners() should not auto-create this._events #3803

bnoordhuis opened this issue Jul 31, 2012 · 6 comments
Labels

Comments

@bnoordhuis
Copy link
Member

This test should pass but doesn't because .listeners() has side effects.

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

var e = new EventEmitter;

e.listeners('foo');
assert(typeof e._events == 'undefined');

e.on('foo', assert.fail);
e.listeners('bar');
assert(typeof e._events['bar'] == 'undefined');
@TooTallNate
Copy link

What should listeners() return in that case then?

@bnoordhuis
Copy link
Member Author

An empty array. It does a .slice() now in master so that's okay but it shouldn't create the _events property.

@isaacs
Copy link

isaacs commented Aug 4, 2012

Agreed. First one to review #3825 gets a virtual high five.

@isaacs isaacs closed this as completed in 50c7d80 Aug 13, 2012
@bnoordhuis
Copy link
Member Author

Fixed in 50c7d80.

@ivanstoyanov
Copy link

Hi, it would be fantastic if you include this in the stable branch too. It is quite a memory leak with my kind of usage.

@TooTallNate
Copy link

@ivanstoyanov Not gonna happen, this is a breaking API change.

tricknotes added a commit to tricknotes/node that referenced this issue Dec 10, 2012
This object initialization has been unnecessary from 12cf730.
rel nodejs#3803
bnoordhuis pushed a commit that referenced this issue Dec 10, 2012
This object initialization has been unnecessary since 12cf730.

Ref #3803.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants