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

Migrate to use a different event emitter base #90

Open
jeswr opened this issue Sep 20, 2022 · 2 comments
Open

Migrate to use a different event emitter base #90

jeswr opened this issue Sep 20, 2022 · 2 comments
Milestone

Comments

@jeswr
Copy link
Collaborator

jeswr commented Sep 20, 2022

While we are doing a semver upgrade we may also want to migrate to use https://www.npmjs.com/package/eventemitter3 which appears to be a much less bloated; and more optimised version over the EventEmitter compared to what I've seen in the NodeJS core.

@RubenVerborgh
Copy link
Owner

I'm interested, although:

  • We do not throw an error when you emit an error event and nobody is listening.
  • The newListener and removeListener events have been removed as they are useful only in some uncommon use-cases.

But we can get around these limitations.

Just a note that AsyncIterator might be bundled together with other Node.js code, which would use the native EventEmitter, so it's not certain that we would remove bloat. But happy to give it a go.

@jacoscaz
Copy link
Collaborator

Just FYI and for when the time comes, I can easily run a few tests by forcing the build of quadstore-comunica to use eventemitter3 instead of events.

@RubenVerborgh RubenVerborgh added this to the v4-alpha milestone Sep 21, 2022
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

3 participants