feat: listeners api #2093
feat: listeners api #2093
Conversation
WalkthroughAdds listener retrieval APIs across realtime entities. RealtimeChannel and RealtimePresence expose listeners(event?: string | string[]): Function[]. EventEmitter.listeners now accepts undefined, string, or string[] and aggregates listeners for arrays. Type declarations in ably.d.ts updated to reflect these public methods. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant RC as RealtimeChannel/Presence
participant EE as EventEmitter (subscriptions)
C->>RC: listeners(event?: string|string[])
alt single event or undefined
RC->>EE: listeners(event?)
EE-->>RC: Function[] | null
RC-->>C: Function[] (empty if null)
else multiple events (string[])
RC->>EE: listeners(events[])
note over EE: Iterate events, collect listeners
EE-->>RC: Combined Function[] | null
RC-->>C: Function[] (empty if null)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/common/lib/client/realtimepresence.ts (1)
467-471: LGTMThin wrapper over the internal emitter; null coalescing to [] matches the new API docs. Consider returning a shallow copy to avoid exposing internal arrays, but better fixed centrally in EventEmitter (see my comment there).
ably.d.ts (1)
2043-2063: Tighten typings: use PresenceAction and callback types.These overloads should use PresenceAction (and arrays thereof) and return messageCallback[], not Function[]. This improves developer ergonomics and aligns with existing presence types.
Apply:
- listeners(): Function[]; + listeners(): messageCallback<PresenceMessage>[]; - listeners(event: string): Function[]; + listeners(event: PresenceAction): messageCallback<PresenceMessage>[]; - listeners(events: string[]): Function[]; + listeners(events: PresenceAction[]): messageCallback<PresenceMessage>[];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
ably.d.ts(2 hunks)src/common/lib/client/realtimechannel.ts(1 hunks)src/common/lib/client/realtimepresence.ts(1 hunks)src/common/lib/util/eventemitter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/common/lib/util/eventemitter.ts (2)
src/common/lib/client/realtimechannel.ts (1)
listeners(467-470)src/common/lib/client/realtimepresence.ts (1)
listeners(468-471)
🔇 Additional comments (1)
src/common/lib/client/realtimechannel.ts (1)
467-471: Breaking change: override of EventEmitter.listeners on RealtimeChannel
RealtimeChannel extends EventEmitter, which already provides listeners(event?: ChannelEvent): channelEventCallback[] | null. Overriding it here to return subscription listeners:
- changes semantics from channel-state listeners to message subscription listeners
- collides with inherited signature, causing typing/runtime ambiguity for existing code that inspects channel-state listeners
Recommend one of:
- rename to messageListeners(event?: string|string[]) and preserve listeners() for EventEmitter semantics
- or add stateListeners(event?: ChannelEvent|ChannelEvent[]) that calls super.listeners(), and document that listeners() returns subscription listeners
If you keep listeners() as is, update ably.d.ts to avoid overload conflicts and clarify semantics. No internal uses of channel.listeners(...) were found—please verify downstream consumers/tests for potential breakage.
| /** | ||
| * Returns all listeners currently registered on this channel. | ||
| * | ||
| * @returns An array of listener functions for all events. Returns an empty array if no listeners are found. | ||
| */ | ||
| listeners(): Function[]; | ||
| /** | ||
| * Returns the listeners for a specified event name on this channel. | ||
| * | ||
| * @param event - The event name to retrieve the listeners for. | ||
| * @returns An array of listener functions for the specified event. Returns an empty array if no listeners are found. | ||
| */ | ||
| listeners(event: string): Function[]; | ||
| /** | ||
| * Returns the listeners for multiple specified event names on this channel. | ||
| * | ||
| * @param events - An array of event names to retrieve the listeners for. | ||
| * @returns An array of listener functions for all the specified events combined. Returns an empty array if no listeners are found. | ||
| */ | ||
| listeners(events: string[]): Function[]; |
There was a problem hiding this comment.
🧩 Analysis chain
Type/behavior conflict with inherited EventEmitter.listeners.
RealtimeChannel extends EventEmitter<…> which already declares listeners(eventName?: EventType): CallbackType[] | null. Adding channel.listeners overloads returning Function[] conflicts:
- Overload ambiguity for listeners() and listeners(string), with incompatible return types (Function[] vs CallbackType[] | null).
- Runtime implementation returns message subscription listeners, not channel state listeners expected by EventEmitter.
Options:
- Rename to messageListeners(…) (preferred), or
- Keep name but add stateListeners(…) for channel state, and relax these overloads to Function[] | null to avoid type conflict, and
- Update EventEmitter.listeners in this d.ts to accept arrays: listeners(eventName?: EventType | EventType[]): CallbackType[] | null.
As a minimal typing fix if you keep the name:
- listeners(): Function[];
+ listeners(): Function[] | null;
- listeners(event: string): Function[];
+ listeners(event: string): Function[] | null;
- listeners(events: string[]): Function[];
+ listeners(events: string[]): Function[] | null;Additionally, please add:
- listeners(eventName?: EventType): CallbackType[] | null;
+ listeners(eventName?: EventType | EventType[]): CallbackType[] | null;to the EventEmitter interface to reflect the new array support. Clarify in docs that RealtimeChannel.listeners() returns message subscription listeners, not channel state listeners, to prevent confusion.
To surface potential TS breakage from overload conflicts, run:
🏁 Script executed:
#!/bin/bash
# Show where both EventEmitter and RealtimeChannel listeners are referenced in d.ts
rg -n -C2 -P 'interface\s+RealtimeChannel|interface\s+EventEmitter<|listeners\(' ably.d.tsLength of output: 2731
Fix type conflict in RealtimeChannel.listeners overloads
RealtimeChannel.listeners overloads clash with inherited EventEmitter.listeners(eventName?: EventType): CallbackType[] | null due to incompatible return types.
As a minimal typing fix if you keep the name:
- listeners(): Function[];
+ listeners(): Function[] | null;
- listeners(event: string): Function[];
+ listeners(event: string): Function[] | null;
- listeners(events: string[]): Function[];
+ listeners(events: string[]): Function[] | null;Additionally, please update EventEmitter to accept an array of event names:
- listeners(eventName?: EventType): CallbackType[] | null;
+ listeners(eventName?: EventType | EventType[]): CallbackType[] | null;- Rename to
messageListeners(…)(preferred) to avoid overload conflicts. - Clarify docs that
.listeners()returns subscription listeners, not channel-state listeners.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Returns all listeners currently registered on this channel. | |
| * | |
| * @returns An array of listener functions for all events. Returns an empty array if no listeners are found. | |
| */ | |
| listeners(): Function[]; | |
| /** | |
| * Returns the listeners for a specified event name on this channel. | |
| * | |
| * @param event - The event name to retrieve the listeners for. | |
| * @returns An array of listener functions for the specified event. Returns an empty array if no listeners are found. | |
| */ | |
| listeners(event: string): Function[]; | |
| /** | |
| * Returns the listeners for multiple specified event names on this channel. | |
| * | |
| * @param events - An array of event names to retrieve the listeners for. | |
| * @returns An array of listener functions for all the specified events combined. Returns an empty array if no listeners are found. | |
| */ | |
| listeners(events: string[]): Function[]; | |
| /** | |
| * Returns all listeners currently registered on this channel. | |
| * | |
| * @returns An array of listener functions for all events. Returns an empty array if no listeners are found. | |
| */ | |
| listeners(): Function[] | null; | |
| /** | |
| * Returns the listeners for a specified event name on this channel. | |
| * | |
| * @param event - The event name to retrieve the listeners for. | |
| * @returns An array of listener functions for the specified event. Returns an empty array if no listeners are found. | |
| */ | |
| listeners(event: string): Function[] | null; | |
| /** | |
| * Returns the listeners for multiple specified event names on this channel. | |
| * | |
| * @param events - An array of event names to retrieve the listeners for. | |
| * @returns An array of listener functions for all the specified events combined. Returns an empty array if no listeners are found. | |
| */ | |
| listeners(events: string[]): Function[] | null; |
🤖 Prompt for AI Agents
In ably.d.ts around lines 2958-2977, the RealtimeChannel.listeners overloads
conflict with the inherited EventEmitter.listeners signature; rename the method
to messageListeners(...) throughout the file and update its JSDoc to state it
returns subscription (message) listeners not channel-state listeners; update all
call sites and exports accordingly; if you must keep the name instead, change
the overloads to a single signature compatible with EventEmitter (e.g.
listeners(event?: string | string[]): Function[] | null) and adjust the
EventEmitter type to accept an array of event names (event?: string | string[])
so return types align.
| * @param event (optional) the name of the event, array of event names, or none for 'any' | ||
| * @return array of events, or null if none | ||
| */ | ||
| listeners(event: string) { | ||
| listeners(event?: string | string[]) { | ||
| if (Array.isArray(event)) { | ||
| const allListeners: Function[] = []; | ||
| event.forEach((eventName) => { | ||
| const listeners = this.listeners(eventName); | ||
| if (listeners) { | ||
| allListeners.push(...listeners); | ||
| } | ||
| }); | ||
| return allListeners.length ? allListeners : null; | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: listeners() mutates internal state and breaks once semantics.
In the single-event branch (below), you push eventsOnce[event] into the persistent array returned by this.events[event], which permanently converts once listeners into persistent listeners whenever listeners() is called. This PR introduces more public usages of listeners(), increasing the likelihood of this bug surfacing.
Fix by returning copies and never mutating internal arrays. Also consider de‑duping when aggregating arrays.
Apply:
- listeners(event?: string | string[]) {
- if (Array.isArray(event)) {
- const allListeners: Function[] = [];
- event.forEach((eventName) => {
- const listeners = this.listeners(eventName);
- if (listeners) {
- allListeners.push(...listeners);
- }
- });
- return allListeners.length ? allListeners : null;
- }
-
- if (event) {
- const listeners = this.events[event] || [];
- if (this.eventsOnce[event]) Array.prototype.push.apply(listeners, this.eventsOnce[event]);
- return listeners.length ? listeners : null;
- }
- return this.any.length ? this.any : null;
- }
+ listeners(event?: string | string[]) {
+ if (Array.isArray(event)) {
+ const set = new Set<Function>();
+ event.forEach((eventName) => {
+ const ls = this.listeners(eventName);
+ if (ls) ls.forEach((l) => set.add(l));
+ });
+ return set.size ? Array.from(set) : null;
+ }
+
+ if (event) {
+ const persistent = this.events[event] || [];
+ const once = this.eventsOnce[event] || [];
+ const combinedLen = persistent.length + once.length;
+ return combinedLen ? persistent.slice().concat(once) : null;
+ }
+ return this.any.length ? this.any.slice() : null;
+ }Note: The doc comment above still states “excludes once events”, which is inaccurate; update accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * @param event (optional) the name of the event, array of event names, or none for 'any' | |
| * @return array of events, or null if none | |
| */ | |
| listeners(event: string) { | |
| listeners(event?: string | string[]) { | |
| if (Array.isArray(event)) { | |
| const allListeners: Function[] = []; | |
| event.forEach((eventName) => { | |
| const listeners = this.listeners(eventName); | |
| if (listeners) { | |
| allListeners.push(...listeners); | |
| } | |
| }); | |
| return allListeners.length ? allListeners : null; | |
| } | |
| listeners(event?: string | string[]) { | |
| if (Array.isArray(event)) { | |
| const set = new Set<Function>(); | |
| event.forEach((eventName) => { | |
| const ls = this.listeners(eventName); | |
| if (ls) ls.forEach((l) => set.add(l)); | |
| }); | |
| return set.size ? Array.from(set) : null; | |
| } | |
| if (event) { | |
| const persistent = this.events[event] || []; | |
| const once = this.eventsOnce[event] || []; | |
| const combinedLen = persistent.length + once.length; | |
| return combinedLen ? persistent.slice().concat(once) : null; | |
| } | |
| return this.any.length ? this.any.slice() : null; | |
| } |
🤖 Prompt for AI Agents
In src/common/lib/util/eventemitter.ts around lines 181-195, listeners()
currently mutates internal arrays by pushing once-listeners into the persistent
this.events[event] array and thus converts once listeners into permanent ones;
change it to never mutate internal state: when returning listeners for a single
event, return a shallow copy (e.g., concatenate copies of persistent and once
arrays) instead of pushing into stored arrays; when handling an array of events,
aggregate copies from each event and de-duplicate the combined list (e.g., via a
Set) before returning; ensure the function still returns null when the resulting
list is empty and update the doc comment above to accurately state whether once
listeners are included.
|
Some comments before review: Also should discuss if there is a need to have the same for LiveObjects, can discuss it with Mike and can add it as part of the new location based API changes. |
adds
channel.listenersandpresence.listenersmethods to get an array of listeners subscribed to the channel/presence.Summary by CodeRabbit