-
Notifications
You must be signed in to change notification settings - Fork 518
Implement wildcard listeners #3335
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds support for wildcard ('*') event listeners in QueueEvents and covers this new feature with tests.
- Introduces a
wildcardListenerscollection and updatesemit,on,off,once, andcloseto handle'*'listeners. - Adds tests in
tests/test_events.tsto verify wildcard listener behavior for built-in and custom events.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_events.ts | Added two tests to verify wildcard listener invocation and data. |
| src/classes/queue-events.ts | Implemented wildcard listeners support and corresponding overloads. |
Comments suppressed due to low confidence (1)
src/classes/queue-events.ts:338
- [nitpick] Add a JSDoc comment above the wildcard
on('*', ...)overload to explain how and when wildcard listeners are invoked, and their order relative to normal listeners.
if (event === '*') {
| */ | ||
| export class QueueEvents extends QueueBase { | ||
| private running = false; | ||
| private wildcardListeners: Array<(eventName: string, ...args: any[]) => void> = []; |
Copilot
AI
Jul 9, 2025
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.
[nitpick] Consider using a Set for wildcardListeners instead of an Array to prevent duplicate listeners and simplify removal logic.
| private wildcardListeners: Array<(eventName: string, ...args: any[]) => void> = []; | |
| private wildcardListeners: Set<(eventName: string, ...args: any[]) => void> = new Set(); |
| // Emit to normal listeners | ||
| const emitted = super.emit(event, ...args); | ||
| // Emit to wildcard listeners, except for '*' | ||
| if (event !== '*' && this.wildcardListeners.length > 0) { |
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.
why do we need to discriminate the * event 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.
You are right, I think it should be just
if (this.wildcardListeners.length > 0) { ... }
Co-authored-by: Copilot <[email protected]>
|
@manast what do you think about such design in general? I tend to think that it is not the best choice and instead it should be something like example below without magical queueEvents.onAll(handler)In my case I ended up with a direct Redis consumer group listener to get full control of it. |
Why
#3334
Ability to listen on any event:
I'm not very familiar with the code base, hopefully, this is the right direction.