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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typed async events #394

Merged
merged 5 commits into from
Nov 23, 2020
Merged

Typed async events #394

merged 5 commits into from
Nov 23, 2020

Conversation

mmkal
Copy link
Contributor

@mmkal mmkal commented Nov 23, 2020

This switches from nodejs's EventEmitter to emittery for a few reasons:

  • type-safe events. This will prevent people doing things like .on('migration', ...) instead of .on('migrating', ...) etc. It also types the event payload - especially important now since the payload is changing slightly
  • async events. this.emit(...) now returns a promise, which we can await. Which means in a follow-up, we can add beforeAll and afterAll events, which will enable use cases like locking the migration table. That wouldn't be possible with sync events.
  • apparently, less chance of memory leaks 馃し

The API is smaller than EventEmitter which makes this a breaking change by itself. emittery only supports single payloads too, meaning we can no longer emit('migrating', m.name, m) - but that was pretty redundant anyway, so it makes sense to update to MigrationParams as part of the major (v3) release.

@mmkal mmkal merged commit 6fce00e into master Nov 23, 2020
@mmkal
Copy link
Contributor Author

mmkal commented Nov 23, 2020

@papb keeping the merge train going, but again feel free to comment! 馃殏

@mmkal mmkal deleted the typed-events branch November 23, 2020 19:55
@papb
Copy link
Member

papb commented Dec 3, 2020

Great!

@papb
Copy link
Member

papb commented Dec 3, 2020

Why did you decide to move from ^ to ~ in our dependencies?

@mmkal
Copy link
Contributor Author

mmkal commented Dec 3, 2020

I think tilde is the default for renovate, although I could be wrong because they call out that in most cases you won't get fixes if you use it: https://docs.renovatebot.com/dependency-pinning/#tilde-vs-caret. Happy to re-visit.

@papb
Copy link
Member

papb commented Dec 3, 2020

Yeah, I believe caret is better.

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.

None yet

2 participants