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

EventEmitter#eventNames() and Symbol support #7

Merged
merged 5 commits into from
Nov 17, 2016
Merged

EventEmitter#eventNames() and Symbol support #7

merged 5 commits into from
Nov 17, 2016

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Mar 18, 2016

This addresses #6 and add support for symbols.

While I was writing this, I found an issue (see inline comments) that should be fixed, so don't merge it yet.

for (event in this.ee._events) {
if (has.call(this.ee._events, event)) args.push(event);
for (event in ee._events) {
if (has.call(ee._events, event)) args.push(event);
Copy link
Member Author

Choose a reason for hiding this comment

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

It is possible that we are in a browser that doesn't have Object.create support and this is an instance of EventEmitter3 < 1.2.0.
In this case we fill the array with prefixed event names, and this will obviously fail below when we get the listeners via ee.listeners(name) as our name will be prefixed again.
The ideal solution would be to detect if this is an instance of EventEmitter3 and in that case see if it is prefixed and slice the event name accordingly.

The problem here is that in order to do this cleanly we need to add a new method to EventEmitter3 and publish it as 1.1.2. Another option would be something like this:

var EE3;
try {
  EE3 = require('eventemitter3');
} catch(e) {};

and check if ee is an instance of EE3 but this seems fragile.

Do you have better ideas?

@lpinca
Copy link
Member Author

lpinca commented Mar 18, 2016

It seems that there aren't modules for the browser that depend on ultron (I'm not sure about truth and fittings), so we can also assume that we are safe if we end in that branch as new installation of eventemitter3 will use eventNames and we only end there if we are using node.

@lpinca
Copy link
Member Author

lpinca commented Mar 22, 2016

@3rd-Eden I would go like this. People affected by the bug described here, if any, can upgrade eventemitter3 and ultron to the latest versions to have it fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants