-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(NODE-3132): Add TypedEventEmitter #2785
Conversation
76b5075
to
e636d1e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make sure there are tests covering at least the breaking changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍 for the refactor removing relayEvents
Using a mapped type of event names to function type can now provide annotations and completion for event argument types. Breaking Changes: - Topology open event emits one argument which is the topology itself - srvPollingHandler on topology now emits TopologyDescriptionChange event (erroneously was serverDescriptionChanged)
985cb9e
to
187e1d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also add an actual ticket for fixing how the srv polling event handler is attached and reference it in the TODO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good
Not sure why but EVG failed to run here. |
Per discussion, let's create 2 tickets to fix the bugs uncovered by typescript and keep the changes in this PR non-breaking |
@dariakp Looks like the revert passes if you wanted to take another look here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Using a mapped type of event names to function type can now provide
annotations and completion for event argument types.
Breaking Changes:Topology open event emits one argument which is the topology itselfsrvPollingHandler on topology now emits TopologyDescriptionChange event (erroneously was serverDescriptionChanged)