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

add connection event listener support similar to node-redis #475

Merged
merged 6 commits into from
Dec 8, 2024

Conversation

ardabeyazoglu
Copy link
Contributor

@ardabeyazoglu ardabeyazoglu commented Dec 7, 2024

Tried to imitate node-redis event listener to make migrations easier. Having an event handler is always useful for logging, debugging and tracing as well.

connect: fires when (re)connected sucessfully
ready: fires when (re)connected and authenticated sucessfully, and ready to send commands
reconnecting: fires only before reconnection with a delay
close: fires after connection is closed for whatever reason
end: fires after connection is closed and terminated, no reconnection is planned
error: fires when an unexpected/uncontrolled error occurs

Copy link
Member

@uki00a uki00a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I'm in favor of this idea. I have only one suggestion: how about managing listeners based on EventTarget? It includes support for AbortSignal, plus it works with Deno as well as Node.js, etc.

The following is an example of strictly defining the type of events to be fired based on EventTarget in Deno:

@uki00a uki00a added the enhancement New feature or request label Dec 7, 2024
@ardabeyazoglu
Copy link
Contributor Author

ardabeyazoglu commented Dec 8, 2024

It took way more time than i expected to implement a proper, fully typed event-target based solution due to typescript inconvenience with events. Honestly, i can't spend any more time for this but i think it is done. I am updating the pull request soon.

Copy link
Member

@uki00a uki00a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you! 👍

@uki00a uki00a merged commit 31e34e2 into denodrivers:master Dec 8, 2024
4 checks passed
uki00a added a commit that referenced this pull request Dec 8, 2024
Follows-up to #475. Type definitions have been improved so that type errors are caused for unsupported events.
@uki00a
Copy link
Member

uki00a commented Dec 8, 2024

Released in v0.37.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants