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

emitMetaEvent assumes synchronous operation #118

Open
twilker opened this issue Jul 26, 2024 · 0 comments
Open

emitMetaEvent assumes synchronous operation #118

twilker opened this issue Jul 26, 2024 · 0 comments

Comments

@twilker
Copy link

twilker commented Jul 26, 2024

emittery/index.js

Lines 167 to 176 in 69193ca

function emitMetaEvent(emitter, eventName, eventData) {
if (isMetaEvent(eventName)) {
try {
canEmitMetaEvents = true;
emitter.emit(eventName, eventData);
} finally {
canEmitMetaEvents = false;
}
}
}

The above code executes the emit method without any await. The way this library is currently implemented, this is no problem. But for my project I derived a class from emittery where I made an override of emit to yield the control back to the event loop. I did this to make every emit truely async and prevent any event from being executed synchronously. This might be a good feature for emittery too, but that is beside the point here. Here is my implementation:

    override async emit<Name extends DatalessEventNames<EventData>>(eventName: Name): Promise<void>;
    override async emit<Name extends keyof EventData>(eventName: Name, eventData: EventData[Name]): Promise<void>;
    override async emit(eventName: unknown, eventData?: unknown): Promise<void> {
        log.info('emit', eventName);
    }

This lead to a lot of errors from this line, because the canEmitMetaEvents flag is already reset again:

throw new TypeError('`eventName` cannot be meta event `listenerAdded` or `listenerRemoved`');

This issue could be solved by using the suggested code in this comment: #97 (comment)

Yup. Or we can have a special function that emits the meta events on a given emitter instance. Something like this.

async function emitMetaEvent(emitter, event, data) {
  metaEventsAllowed = true
  await emitter.emit(event, data)
  metaEventsAllowed = false
}

The only benefit of this function over the callback approach is, we will not end up creating too many anonymous functions.

if (!isMetaEvent(eventName)) {
  emitMetaEvent(this, listenerAdded, {eventName, listener})
}
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

No branches or pull requests

1 participant