Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

events: add eventNames() method #5617

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions doc/api/events.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,20 @@ they were registered, passing the supplied arguments to each.

Returns `true` if event had listeners, `false` otherwise.

### emitter.eventNames()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be Names? Why not just events?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed above. Two reasons:

  1. This is less likely to cause conflicts with modules extending EventEmitter.

  2. This is more reasonable name because it correctly describes what this method actually does — it returns the list of event names for the registered event listeners. _events, for an example, is an object that stores all listeners (grouped by event name).

    It would be easier to understand what the code that uses this method does without navigating to the docs this way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChALkeR I am really sorry. I straight away started with the code. It makes sense. Thanks for explaining.


Returns an array listing the events for which the emitter has registered
listeners.

```js
const EventEmitter = require('events');
const myEE = new EventEmitter();
myEE.on('foo', () => {});
myEE.on('bar', () => {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe throw in a symbol example too?

console.log(myErr.eventNames());
// Prints ['foo', 'bar']
```

### emitter.getMaxListeners()

Returns the current max listener value for the `EventEmitter` which is either
Expand Down
8 changes: 8 additions & 0 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,14 @@ function listenerCount(type) {
return 0;
}

EventEmitter.prototype.eventNames = function() {
const events = this._events;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: this can go inside the if block now.

if (events) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to instead check this._eventsCount > 0 in case of an empty _events object?

return Object.getOwnPropertyNames(events).concat(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't Object.keys() sufficient here since we aren't creating non-enumerable properties? It should be much faster than Object.getOwnPropertyNames() too.

Object.getOwnPropertySymbols(events));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we check if the symbol property is enumerable events.propertyIsEnumerable(symbol) and add it to the array only if it is?

} else return [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the return [] on the next line would be more readable IMHO

};

// About 1.5x faster than the two-arg version of Array#splice().
function spliceOne(list, index) {
for (var i = index, k = i + 1, n = list.length; k < n; i += 1, k += 1)
Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-events-list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

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

const EE = new EventEmitter();
const m = () => {};
EE.on('foo', () => {});
assert.deepStrictEqual(['foo'], EE.eventNames());
EE.on('bar', m);
assert.deepStrictEqual(['foo', 'bar'], EE.eventNames());
EE.removeListener('bar', m);
assert.deepStrictEqual(['foo'], EE.eventNames());
const s = Symbol('s');
EE.on(s, m);
assert.deepStrictEqual(['foo', s], EE.eventNames());
EE.removeListener(s, m);
assert.deepStrictEqual(['foo'], EE.eventNames());