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

Investigate replacing lib/EventEmitter.js with mitt #4908

Closed
timneutkens opened this issue Aug 7, 2018 · 7 comments · Fixed by #5987
Closed

Investigate replacing lib/EventEmitter.js with mitt #4908

timneutkens opened this issue Aug 7, 2018 · 7 comments · Fixed by #5987

Comments

@timneutkens
Copy link
Member

https://github.com/developit/mitt

Mitt is 277 Bytes

EventEmitter.js is around 1.07KB

They have the same functionality from what I've seen.

@timneutkens
Copy link
Member Author

@DullReferenceException could you check this out maybe?

@HaNdTriX
Copy link
Contributor

HaNdTriX commented Aug 8, 2018

Mitt is awesome but in my opinion "not very node like".

Feature node mitt
chaining (register func return self) yes no
supports class inheritance yes yes
wildcards no yes
emit returns boolean yes no
on/off (after node version 10) yes
addEventListener/removeEventListener yes no

@timneutkens
Copy link
Member Author

@HaNdTriX it doesn't really matter when all we do is provide the .on / .off externally through router.events.

@HaNdTriX
Copy link
Contributor

HaNdTriX commented Aug 8, 2018

No not at all. I am actually fine with using mitt. I just wanted to point out the differences.

The issue that was fixed by @DullReferenceException here b492e67 will be reintroduced though. (chaining & emit returns boolean)

@DullReferenceException
Copy link

Probably an ok compromise. If the EventEmitter changes don't make it out of canary, nobody will know what they're missing out on. Maybe the mitt maintainers would be ok with some minor contributions (I just really like chaining but acknowledge it's just a convenience).

@lucleray
Copy link
Member

lucleray commented Dec 1, 2018

With the current version, event-emitter.js is now ~400 bytes gzipped vs mitt is 200 bytes gzipped. Is it still worth it to use mitt ?

@timneutkens
Copy link
Member Author

@lucleray yes it would still be worth replacing it imo.

timneutkens pushed a commit that referenced this issue Jan 4, 2019
This PR aims at replacing next-server/lib/event-emitter.js by mitt.

Fix #4908

event-emitter.js is ~400 bytes gzipped vs mitt is 200 bytes
@lock lock bot locked as resolved and limited conversation to collaborators Jan 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants