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

listenerCount() and Emittery.listenerAdded event not working as expected #64

Open
AuHau opened this issue Sep 30, 2020 · 6 comments
Open

Comments

@AuHau
Copy link

AuHau commented Sep 30, 2020

If I have the following snippet

emitter.on(Emittery.listenerAdded, () => {
  console.log('Count: ', emitter.listenerCount('helloEvent'))
})

emitter.on('helloEvent', () => {})
emitter.on('helloEvent', () => {})

Then I get printed:

Count: 2
Count: 2

While I would expect:

Count: 1
Count: 2
@sindresorhus
Copy link
Owner

// @Richienb

@Richienb
Copy link
Contributor

Richienb commented Sep 30, 2020

@sindresorhus The problem involves the asynchronous nature of promises. They are only executed during idle time which is why sindresorhus/make-synchronous#3 is impossible (since synchronously sleeping is not counted as idle). However, before idle time is reached, all of the synchronous emitter.on calls are made before these promises:

emittery/index.js

Lines 256 to 268 in 1afed6a

await resolvedPromise;
await Promise.all([
...staticListeners.map(async listener => {
if (listeners.has(listener)) {
return listener(eventData);
}
}),
...staticAnyListeners.map(async listener => {
if (anyListeners.has(listener)) {
return listener(eventName, eventData);
}
})
]);
get a chance to start calling the callback defined in emitter.on(Emitter.listenerAdded). As expected, these proof-of-concept changes within .emit() fix the problem at hand:

-               await resolvedPromise;
-               await Promise.all([
-                       ...staticListeners.map(async listener => {
-                               if (listeners.has(listener)) {
-                                       return listener(eventData);
-                               }
-                       }),
-                       ...staticAnyListeners.map(async listener => {
-                               if (anyListeners.has(listener)) {
-                                       return listener(eventName, eventData);
-                               }
-                       })
-               ]);
+               staticListeners.map(listener => {
+                       if (listeners.has(listener)) {
+                               return listener(eventData);
+                       }
+               });
+               staticAnyListeners.map(listener => {
+                       if (anyListeners.has(listener)) {
+                               return listener(eventName, eventData);
+                       }
+               });

This means that the scope of this problem actually extends to any code using .emit():

const Emittery = require('.');

const emitter = new Emittery();

emitter.on('unicorn', () => {
	console.count(`Event received`);
});

emitter.emit('unicorn');
console.count('Event sent');

emitter.emit('unicorn');
console.count('Event sent');

The above code outputs:

Event sent: 1
Event sent: 2
Event received: 1
Event received: 2

Instead of:

Event received: 1
Event sent: 1
Event received: 2
Event sent: 2

@Richienb
Copy link
Contributor

Richienb commented Oct 3, 2020

After some additional investigation, it appears that the only way to fix this problem is to wait for .emit to resolve by awaiting it here:

this.emit(listenerAdded, {eventName, listener});

and here:

this.emit(listenerRemoved, {eventName, listener});

However, this will require us to make .on() and .off() return promises.

@sindresorhus
Copy link
Owner

However, this will require us to make .on() and .off() return promises.

Is there really no other way?

@Richienb
Copy link
Contributor

🤷‍♂️

@Richienb
Copy link
Contributor

Is there really no other way?

I guess the only other way is to make .emit a synchronous function.

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

No branches or pull requests

3 participants