DX-1211: RealtimeChannel interface docstrings#2243
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
m-hulbert
left a comment
There was a problem hiding this comment.
A few more comments on this one outside of language. There are some things I think make the descriptions more complicated rather than simpler and a few things to tidy up in terms of naming.
| readonly name: string; | ||
| /** | ||
| * An {@link ErrorInfo} object describing the last error which occurred on the channel, if any. | ||
| * An {@link ErrorInfo} object describing the last error which occurred on the channel, if any. This is `null` until an error occurs, for example a transition to `failed` or `suspended`, so guard against `null` despite the declared type; inspect it to diagnose a failed or suspended channel. |
There was a problem hiding this comment.
| * An {@link ErrorInfo} object describing the last error which occurred on the channel, if any. This is `null` until an error occurs, for example a transition to `failed` or `suspended`, so guard against `null` despite the declared type; inspect it to diagnose a failed or suspended channel. | |
| * An {@link ErrorInfo} object describing the last error which occurred on the channel, if any. This is `null` until an error occurs, for example a transition in channel state to `failed` or `suspended`. Guard against `null` despite the declared type and inspect it to diagnose a channel in the failed or suspended state. |
| readonly state: ChannelState; | ||
| /** | ||
| * Optional [channel parameters](https://ably.com/docs/realtime/channels/channel-parameters/overview) that configure the behavior of the channel. | ||
| * Optional [channel parameters](https://ably.com/docs/realtime/channels/channel-parameters/overview) that configure the behavior of the channel. After the channel attaches this reflects the parameters the server confirmed on the most recent attach, which may differ from those requested via {@link ChannelOptions.params}, and is `undefined` before the channel attaches for the first time, so guard against `undefined` despite the declared type. |
There was a problem hiding this comment.
| * Optional [channel parameters](https://ably.com/docs/realtime/channels/channel-parameters/overview) that configure the behavior of the channel. After the channel attaches this reflects the parameters the server confirmed on the most recent attach, which may differ from those requested via {@link ChannelOptions.params}, and is `undefined` before the channel attaches for the first time, so guard against `undefined` despite the declared type. | |
| * Optional channel parameters that configure the behavior of the channel. After the channel attaches this reflects the parameters the server confirmed on the most recent attach, which may differ from those requested via {@link ChannelOptions.params}. It is `undefined` before the channel attaches for the first time, so guard against `undefined` despite the declared type. |
Appreciate this existed before, but it's odd that we link out to the docs on this and the TypeDoc reference and then the @see link is to some different docs again. I think drop the initial link to docs.
| params: ChannelParams; | ||
| /** | ||
| * An array of {@link ResolvedChannelMode} objects. | ||
| * An array of {@link ResolvedChannelMode} objects reflecting the modes the server granted on the most recent attach, which may differ from the modes requested via {@link ChannelOptions.modes}. The value is `undefined` before the channel attaches for the first time, and is also `undefined` (rather than an empty array) when the server grants no modes, so callers should guard against `undefined` despite the declared type. |
There was a problem hiding this comment.
which may differ from the modes requested via {@link ChannelOptions.modes} how and why?
I'd also remove the brackets and long sentences as per other suggestions.
| modes: ResolvedChannelMode[]; | ||
| /** | ||
| * Deregisters the given listener for the specified event name. This removes an earlier event-specific subscription. | ||
| * Deregisters the given listener for the specified event name, removing an earlier event-specific subscription. This only removes the local listener and does not detach the channel, so messages may continue to arrive for any other registered listeners. |
There was a problem hiding this comment.
I think if we're saying other listeners may still be receiving messages then we should also state that messages will still be streamed to the client if they have permissions, unless they detach maybe?
There was a problem hiding this comment.
Yes I think we need to be very clear here. It does not detach the channel and any registered listeners may receive messages. But the key part is not detaching the channel - messages will still be sent by the server and will be billed accordingly
| unsubscribe(event: string, listener: messageCallback<InboundMessage>): void; | ||
| /** | ||
| * Deregisters the given listener from all event names in the array. | ||
| * Deregisters the given listener from all event names in the array. This only removes the local listeners and does not detach the channel. |
There was a problem hiding this comment.
Is there a reason to only include the additional info on 1 overload?
| history(params: RealtimeHistoryParams | null, callback: StandardCallback<PaginatedResult<InboundMessage>>): never; | ||
| /** | ||
| * Sets the {@link ChannelOptions} for the channel. | ||
| * Sets the {@link ChannelOptions} for the channel. If the channel is already attached or attaching and the new {@link ChannelOptions.modes} or {@link ChannelOptions.params} differ from the current ones, this triggers a live re-attach to apply them, and the returned promise resolves only once the server confirms the new options, rejecting with an {@link ErrorInfo} if the channel detaches or fails in the meantime. Otherwise the new options are stored for the next attach. The promise also rejects with an {@link ErrorInfo} when the supplied options are invalid. |
There was a problem hiding this comment.
This is a very long sentence and it isn't clear what a "live re-attach" is.
| setOptions(options: ChannelOptions): Promise<void>; | ||
| /** | ||
| * Registers a listener for messages with a given event name on this channel. The caller supplies a listener function, which is called each time one or more matching messages arrives on the channel. | ||
| * Registers a listener for messages with a given event name on this channel. The caller supplies a listener function, which is called each time one or more matching messages arrives on the channel. Implicitly attaches the channel unless {@link ChannelOptions.attachOnSubscribe} is `false`. Requires the `subscribe` mode (granted by default unless {@link ChannelOptions.modes} excludes it); if the channel attaches without it the server never delivers messages, so the listener silently never fires and a warning is logged rather than the call rejecting (or it rejects with a hinted {@link ErrorInfo} when {@link ClientOptions.strictMode} is enabled). |
There was a problem hiding this comment.
Mmm, this is a lot of extra info getting into modes here - do you think that's really required?
| subscribe(event: string, listener: messageCallback<InboundMessage>, callback: ErrorCallback): never; | ||
| /** | ||
| * Publishes a single message to the channel with the given event name and payload. When publish is called with this client library, it won't attempt to implicitly attach to the channel, so long as [transient publishing](https://ably.com/docs/realtime/channels#transient-publish) is available in the library. Otherwise, the client will implicitly attach. | ||
| * Publishes a single message to the channel with the given event name and payload. Publishing does not attach the channel, so unlike {@link RealtimeChannel.subscribe | `subscribe()`}, {@link RealtimePresence.enter | `enter()`}, and {@link RealtimePresence.get | `get()`} a publish-only client need not attach or subscribe first. |
There was a problem hiding this comment.
I don't think we should be saying unlike X, Y or Z as someone is just looking at this method. If you think we really need to state something, I'd go more along the lines of "the client does not need to attach to the channel before publishing to it", but even that seems redundant to a certain extent.
| whenState(targetState: ChannelState): Promise<ChannelStateChange | null>; | ||
| /** | ||
| * Retrieves the latest version of a specific message by its serial identifier. | ||
| * Retrieves the latest version of a specific message by its serial identifier. Retrievable messages require message interactions (mutable messages) to be enabled for the channel by a channel rule. The supplied serial or {@link Message} must carry a populated `serial`, which is present on a {@link Message} received from a subscribe callback but not on a freshly constructed one; if the serial is missing the promise rejects with an {@link ErrorInfo}. |
There was a problem hiding this comment.
We don't call things "mutable messages", "message interactions" nor "channel rules".
'Retrievable messages' also reads strangely - as does what the subsequent sentence is trying to say. I think you're saying serial only exists if you've received a message via subscribe(), not if you've constructed one which is arguable more confusing to read than what it's trying to solve IMO.
| getMessage(serialOrMessage: string | Message): Promise<Message>; | ||
| /** | ||
| * Publishes an update to an existing message with patch semantics. Non-null `name`, `data`, and `extras` fields in the provided message will replace the corresponding fields in the existing message, while null fields will be left unchanged. | ||
| * Publishes an update to an existing message with patch semantics. Non-null `name`, `data`, and `extras` fields in the provided message will replace the corresponding fields in the existing message, while null fields will be left unchanged. The namespace must have message interactions (updates) enabled by a channel rule, and the supplied `message` must carry the `serial` it was given when published (pass the {@link Message} received in a subscribe callback, not a freshly constructed object); otherwise the promise rejects with an {@link ErrorInfo}. |
There was a problem hiding this comment.
As above (haven't been commenting when a previous comment applies to every overload) but also should Message and Serial be capitalised?
There was a problem hiding this comment.
not serial, no. serial is the field on Message (lowercase in the API)
| * ```ts | ||
| * const result = await channel.history({ limit: 25 }); | ||
| * ``` | ||
| * @see https://ably.com/docs/pub-sub/api/javascript/realtime/realtime-channel#history |
There was a problem hiding this comment.
A general question - do we have a plan for keeping this in sync with the docs to avoid 404s?
There was a problem hiding this comment.
not directly, the pr description highlights that some of these point at ably/docs PR #3400 which is yet to be merged.
I'll create a ticket to track how we can avoid 404s going forwards, likely some CI step
| * {@label WITH_MESSAGE_FILTER} | ||
| * | ||
| * Registers a listener for messages on this channel that match the supplied filter. | ||
| * Registers a listener for messages on this channel that match the supplied filter. Implicitly attaches the channel unless {@link ChannelOptions.attachOnSubscribe} is `false`. Requires the `subscribe` mode (granted by default unless {@link ChannelOptions.modes} excludes it); if the channel attaches without it the server never delivers messages, so the listener silently never fires and a warning is logged rather than the call rejecting (or it rejects with a hinted {@link ErrorInfo} when {@link ClientOptions.strictMode} is enabled). |
There was a problem hiding this comment.
Granted by default IFF the key/token has the necessary capability
There was a problem hiding this comment.
fair! I've moved towards keeping things as simple as possible and just mentioned that subscribe is required
| publish(name: string, data: any, callback: ErrorCallback): never; | ||
| /** | ||
| * If the channel is already in the given state, returns a promise which immediately resolves to `null`. Else, calls {@link EventEmitter.once | `once()`} to return a promise which resolves the next time the channel transitions to the given state. | ||
| * If the channel is already in the given state, returns a promise which immediately resolves to `null`. Else, calls {@link EventEmitter.once | `once()`} to return a promise which resolves with the {@link ChannelStateChange} the next time the channel transitions to the given state. |
There was a problem hiding this comment.
Calling EventEmitter.once is an internal detail, does the consumer need to know how it works?
| whenState(targetState: ChannelState): Promise<ChannelStateChange | null>; | ||
| /** | ||
| * Retrieves the latest version of a specific message by its serial identifier. | ||
| * Retrieves the latest version of a specific message by its serial identifier. Retrievable messages require message interactions (mutable messages) to be enabled for the channel by a channel rule. The supplied serial or {@link Message} must carry a populated `serial`, which is present on a {@link Message} received from a subscribe callback but not on a freshly constructed one; if the serial is missing the promise rejects with an {@link ErrorInfo}. |
There was a problem hiding this comment.
Mutable messages != message interactions - the latter is an unrelated, deprecated feature
| deleteMessage(message: Message, operation?: MessageOperation, options?: PublishOptions): Promise<UpdateDeleteResult>; | ||
| /** | ||
| * Appends data to an existing message. The supplied `data` field is appended to the previous message's data, while all other fields (`name`, `extras`) replace the previous values if provided. | ||
| * Appends data to an existing message. The supplied `data` field is appended to the previous message's data, while all other fields (`name`, `extras`) replace the previous values if provided. Requires message interactions (mutable messages) to be enabled for the channel by a channel rule, and the supplied {@link Message} must carry the `serial` it received from a subscribe callback, otherwise the call rejects with an {@link ErrorInfo}. |
There was a problem hiding this comment.
Ditto here and other places mentioning "message interactions" - its unrelated to this
Apply reviewer suggestions and corrections from m-hulbert and AndyTWF:
- errorReason, params, push, history: apply the reviewers' suggested
wording; drop the inline docs link on params.
- push: state that neither the default nor modular build bundles the
Push plugin and to import it from `ably/push`; fix naming
("push notification", backticked `Push`).
- Remove deprecated/incorrect terms across the message-mutation methods:
"message interactions" and "channel rule" -> "a rule"; "durable
storage" -> "storage". State the populated-serial requirement plainly.
- attach/detach: drop internal EventEmitter references, describe the
emitted state change as prose; add implicit-attach triggers (presence
update()/get(), LiveObjects get()) and the detaching state; note that
detaching stops server delivery while local listeners remain; link
release()/get().
- subscribe: collapse the mode-gate narration to a one-line headline.
- publish: drop the sibling-contrast sentence; unify the verb mood.
- setOptions: remove the "live re-attach" coinage.
- unsubscribe: state the no-detach consequence (server keeps sending,
billed) consistently across all overloads.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c81b62a to
75ca7f8
Compare
Apply reviewer suggestions and corrections from m-hulbert and AndyTWF:
- errorReason, params, push, history: apply the reviewers' suggested
wording; drop the inline docs link on params.
- push: state that neither the default nor modular build bundles the
Push plugin and to import it from `ably/push`; fix naming
("push notification", backticked `Push`).
- Remove deprecated/incorrect terms across the message-mutation methods:
"message interactions" and "channel rule" -> "a rule"; "durable
storage" -> "storage". State the populated-serial requirement plainly.
- attach/detach: drop internal EventEmitter references, describe the
emitted state change as prose; add implicit-attach triggers (presence
update()/get(), LiveObjects get()) and the detaching state; note that
detaching stops server delivery while local listeners remain; link
release()/get().
- subscribe: collapse the mode-gate narration to a one-line headline.
- publish: drop the sibling-contrast sentence; unify the verb mood.
- setOptions: remove the "live re-attach" coinage.
- unsubscribe: state the no-detach consequence (server keeps sending,
billed) consistently across all overloads.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bec860c to
ae2344a
Compare
|
Thanks both - @AndyTWF @m-hulbert I've heavily refactored the prompt/workflow this was generated from, and introduced a new review step which covers some language and content style guidelines with your feedback from this PR in mind. Latest commit shows the output of this which I've also checked through. Might still be some stragglers but leaning into this approach so it's easier to apply to all the other docstrings too |
2a86111 to
7906161
Compare
m-hulbert
left a comment
There was a problem hiding this comment.
These are looking a lot better - added a few comments but they're mostly about long sentences and replacing some semi-colons with fullstops to make things easier to read.
There was a problem hiding this comment.
A few things on this one:
- Where are the documented silent-failure paths?
- We shouldn't be including a version in the string, as these will only be visible to anyone on version 2.x+
- I don't think we should have an example in this instance - we just state it returns the legacy values.
- I'd then move the info about the default to just before the final sentence.
| * `authorize()` cannot change the API key, so passing an `authOptions.key` that differs from the one the client was constructed with is rejected with an {@link ErrorInfo}. | ||
| * | ||
| * Any {@link TokenParams} and {@link AuthOptions} passed in are stored as the new defaults for all subsequent token requests and entirely replace, rather than merge with, the current saved values. | ||
| * Instructs the library to get a new token immediately. On a realtime client it re-authenticates the live connection, or initiates a connection if not connected, and the returned promise resolves only once the new token has taken effect on a `connected` connection, rejecting with an {@link ErrorInfo} if re-authentication fails or the connection cannot be (re)established. The client must be able to issue tokens, so a `key`, `authUrl`, or `authCallback` must be configured in {@link ClientOptions}, otherwise the call rejects with an {@link ErrorInfo}; `authorize()` cannot change the API key, so passing an `authOptions.key` that differs from the one the client was constructed with also rejects. Any {@link TokenParams} and {@link AuthOptions} passed in are stored as the new defaults for all subsequent token requests and entirely replace, rather than merge with, the current saved values. |
There was a problem hiding this comment.
These are some really long sentences - can we split them up?
| * An API `key` value must be available locally to sign the request, supplied either in the client's {@link ClientOptions} or as `key` in the `authOptions` argument. Without a `key` the call rejects with an {@link ErrorInfo}, since a token-authenticated client cannot construct token requests itself and must instead obtain the {@link TokenRequest} from the key owner. | ||
| * | ||
| * Both {@link TokenParams} and {@link AuthOptions} are optional. When omitted or `null`, the client's stored defaults are used, as specified at instantiation or later updated by an `authorize()` request. Any values passed in replace, rather than merge with, those defaults. | ||
| * Creates and signs an Ably {@link TokenRequest} based on the specified (or if none specified, the client library stored) {@link TokenParams} and {@link AuthOptions}. Use this to implement an Ably Token request callback for use by other clients. An API `key` value must be available locally to sign the request, supplied either in the client's {@link ClientOptions} or as `key` in the `authOptions` argument; without one the call rejects with an {@link ErrorInfo}, since a token-authenticated client cannot construct token requests itself and must instead obtain the {@link TokenRequest} from the key owner. Both {@link TokenParams} and {@link AuthOptions} are optional; when omitted or `null`, the client library's stored defaults (those set at construction or by a later `authorize` call) are used, and any values passed in replace, rather than merge with, those defaults. |
There was a problem hiding this comment.
As above - I'd make the semi-colon a fullstop to split up the long sentences and make it easier to parse.
| * Calls the `requestToken` REST API endpoint to obtain an Ably Token according to the specified {@link TokenParams} and {@link AuthOptions}. Both {@link TokenParams} and {@link AuthOptions} are optional. When omitted or `null`, the client's stored defaults are used, as specified at instantiation or later updated by an `authorize()` request. Any values passed in replace, rather than merge with, those defaults. | ||
| * | ||
| * The client must have a way to obtain a token, so the resolved {@link AuthOptions} must include one of `authCallback`, `authUrl`, or `key`. A client given only a literal token or `tokenDetails` with no renewal mechanism cannot request a new token and the call rejects with an {@link ErrorInfo}. | ||
| * Calls the `requestToken` REST API endpoint to obtain an Ably Token according to the specified {@link TokenParams} and {@link AuthOptions}. Both are optional; when omitted or `null`, the client's stored defaults are used, as specified in the {@link ClientOptions} at instantiation or later updated by an `authorize` request, and any values passed in are used instead of, rather than merged with, those defaults. The client must have a usable way to obtain a token, so the resolved {@link AuthOptions} must include one of `authCallback`, `authUrl`, or `key`; a client given only a literal token or `tokenDetails` with no renewal mechanism cannot request a new token and the call rejects with an {@link ErrorInfo}. |
There was a problem hiding this comment.
Two big sentences - sensing a theme 🙂
| * The client must be authenticated with an API key (basic auth), not a token; a | ||
| * token-authenticated client cannot revoke tokens and the call rejects with an {@link ErrorInfo}. | ||
| * Only tokens issued by an API key that had revocable tokens enabled before the token was issued | ||
| * can be revoked. |
There was a problem hiding this comment.
Some odd formatting here and similar comment to before in that we should be explicit in that the client revoking the tokens needs to be auth'd with an API key. Also semi-colon should be a fullstop again.
There was a problem hiding this comment.
I think this is mixing part of what went wrong with how to fix it.
There was a problem hiding this comment.
since the key you passed has no colon-separated secret - I'd argue this should be in the message itself rather than the hint.
| code: 93001, | ||
| statusCode: 400, | ||
| hint: 'Include "annotation_subscribe" in the channel modes: realtime.channels.get(name, { modes: ["annotation_subscribe", ...] }), or call channel.setOptions({ modes: [...] }) on an existing channel (this triggers a reattach). If the attach is then rejected, the channel namespace does not have "Message annotations, updates, appends, and deletes" enabled (`ably apps rules list` shows which namespaces do). If the attach succeeds but annotations still are not delivered, your API key lacks the annotation-subscribe capability and the server silently dropped the mode (`ably auth keys list` shows your key\'s capabilities).', | ||
| hint: 'Include "annotation_subscribe" in the channel modes: realtime.channels.get(name, { modes: ["annotation_subscribe", ...] }), or call channel.setOptions({ modes: [...] }) on an existing channel (this triggers a reattach). If the attach is then rejected, the channel namespace does not have "Message annotations, updates, and deletes" enabled (`ably apps rules list` shows which namespaces do). If the attach succeeds but annotations still are not delivered, your API key lacks the annotation-subscribe capability and the server silently dropped the mode (`ably auth keys list` shows your key\'s capabilities).', |
There was a problem hiding this comment.
Would the part from "If the attach is then rejected..." not result in different errors?
7906161 to
963b791
Compare
ae2344a to
1d45ea3
Compare
963b791 to
c54b363
Compare
61c9a99 to
20f12c5
Compare
c54b363 to
b368d48
Compare
20f12c5 to
d71092a
Compare
… side-effects, failure modes) Surface call-site prerequisites and failure modes across the RealtimeChannel public surface per docstringRules.md (terse folded prose + @see companion): - subscribe(): silent missing-`subscribe`-mode trap + strictMode aside (DX-1211 core) - params/errorReason: guard against undefined/null despite the declared type - history(): persistence channel-rule prerequisite; fix "presence members" @param bug - message ops (get/update/delete/append/getMessageVersions): async-reject framing and message-interactions channel-rule prerequisite - push: universal missing-plugin throw kept; presence/annotations left as-is (bundled by the default build) TypeDoc (treatWarningsAsErrors), prettier, and eslint all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply reviewer suggestions and corrections from m-hulbert and AndyTWF:
- errorReason, params, push, history: apply the reviewers' suggested
wording; drop the inline docs link on params.
- push: state that neither the default nor modular build bundles the
Push plugin and to import it from `ably/push`; fix naming
("push notification", backticked `Push`).
- Remove deprecated/incorrect terms across the message-mutation methods:
"message interactions" and "channel rule" -> "a rule"; "durable
storage" -> "storage". State the populated-serial requirement plainly.
- attach/detach: drop internal EventEmitter references, describe the
emitted state change as prose; add implicit-attach triggers (presence
update()/get(), LiveObjects get()) and the detaching state; note that
detaching stops server delivery while local listeners remain; link
release()/get().
- subscribe: collapse the mode-gate narration to a one-line headline.
- publish: drop the sibling-contrast sentence; unify the verb mood.
- setOptions: remove the "live re-attach" coinage.
- unsubscribe: state the no-detach consequence (server keeps sending,
billed) consistently across all overloads.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Channel docstrings
From the 2026-07-02 docstrings-pr-review run (per-member review, adversarial
verify, reviewer-lens, and cross-unit consistency barrier), each fix traced
against the implementation:
- unsubscribe (all 7 overloads): drop the banned billing consequence for the
capability-qualified streaming form; drop restating participials; state
that the filter overload matches by object identity and that an unknown
filter removes nothing and no error is raised; drop the signature-obvious
example on the event overload
- subscribe (all 4 overloads): name the silent failure ("and no error is
raised"); document the second null resolution (attachOnSubscribe: false, no
attach attempted); drop the restated listener sentence on two overloads
- attach: receiver-qualify and link every implicit-attach trigger (bare
"subscribe()" was ambiguous); replace undefined "connection is unusable"
with the traced connection states (suspended, closing, closed, failed);
name the state-change listener surface (on()/once()), also on detach
- detach: cover presence/annotation/object traffic, not just messages;
sanctioned lifecycle-state constructions
- setOptions: only changed modes/params wait for the next attach; other
options, such as a cipher, take effect immediately (traced: channelOptions
and decoding context update synchronously)
- params: drop the causeless may-differ clause; split the undefined guard
into its own paragraph. modes: split the 32-word sentence, state that no
modes granted yields undefined, not an empty array
- push: drop bundling/import mechanism (belongs at the plugin docs); split
purpose from prerequisite; end the failure sentence at the throw
- whenState: a state never reached means the promise never settles and no
error is raised
- deleteMessage: restore the fact dropped from main that a delete keeps the
previous data unless explicitly overwritten; appendMessage: example now
shows the spread shape (passing the received message verbatim would re-send
its existing data)
- mutable-message members: link the rule prerequisite to
https://ably.com/docs/channels#rules (anchor verified live); REST Channel
siblings get the same serial-prerequisite and rule-gate paragraphs,
canonical patch-semantics sentence, and mirrored examples (they share the
RestChannelMixin code paths byte-for-byte; @see mirroring deferred, no REST
reference page exists in ably/docs #3400)
- history: persistence caveats split into their own paragraph
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… apply the suggested push docstring Moves the storage concept inline onto the persistence-rule mention per the single-link convention, and restores the Push plugin bundling sentence via the reviewer's suggested rewrite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d71092a to
c064311
Compare
b368d48 to
6a9f6ba
Compare
m-hulbert
left a comment
There was a problem hiding this comment.
A few minor comments left, but mostly LGTM now.
| modes: ResolvedChannelMode[]; | ||
| /** | ||
| * Deregisters the given listener for the specified event name. This removes an earlier event-specific subscription. | ||
| * Deregisters the given listener for the specified event name. This only removes the local listeners and does not detach the channel. The channel stays attached, so the server keeps streaming its messages to the client while the client's capability permits. |
There was a problem hiding this comment.
| * Deregisters the given listener for the specified event name. This only removes the local listeners and does not detach the channel. The channel stays attached, so the server keeps streaming its messages to the client while the client's capability permits. | |
| * Deregisters the given listener for the specified event name. This only removes local listeners and does not detach the channel. If the channel is not detached, the server continues to stream messages to the client, if the client's capabilities permit it to receive them. |
| unsubscribe(events: Array<string>): void; | ||
| /** | ||
| * Deregisters all listeners to messages on this channel that match the supplied filter. | ||
| * Deregisters listeners registered with the supplied {@link MessageFilter}. The filter is compared by object identity, so it must be the same object previously passed to {@link RealtimeChannel.subscribe | `subscribe()`}. A filter object not previously passed to `subscribe()` removes nothing and no error is raised. This only removes the local listeners and does not detach the channel. The channel stays attached, so the server keeps streaming its messages to the client while the client's capability permits. |
| subscribe(filter: MessageFilter, listener?: messageCallback<InboundMessage>): Promise<ChannelStateChange | null>; | ||
| /** | ||
| * Registers a listener for messages on this channel. The caller supplies a listener function, which is called each time one or more messages arrives on the channel. | ||
| * Registers a listener for messages on this channel. Implicitly attaches the channel unless {@link ChannelOptions.attachOnSubscribe} is `false`. Without the `subscribe` mode the listener never fires and no error is raised. |
There was a problem hiding this comment.
Is Without the subscribe mode the listener never fires and no error is raised truly necessary or does this actually make reading the description more complex than it needs to be?
Stacked on #2242 (Auth docstrings, base
DX-1211/auth-docstrings); review/merge that first. Rewrites the JSDoc on theRealtimeChannelpublic surface inably.d.tsperdocstringRules.md: terse folded prose that surfaces call-site prerequisites, side-effects, and failure modes (the DX-1211 silent-failure class), with an@seecompanion link to the canonical reference.Scope
ably.d.tsonly (+135/−29). 19 members rewritten, 2 deliberately left as-is.Highlights
subscribe()(DX-1211 core): documents the silent missing-subscribe-mode trap — the listener never fires, the SDK warn-logs rather than rejecting (or rejects with a hintedErrorInfounderstrictMode). On all four active overloads.params/errorReason: declared non-optional but actuallyundefined/nulluntil first attach/error — now carry a guard-against-undefined/null caveat (parallel tomodes).history(): non-code persistence channel-rule prerequisite; fixes a@paramcopy-paste bug ("presence members" → "messages").getMessage/updateMessage/deleteMessage/appendMessage/getMessageVersions): async reject framing (not synchronous throw) + message-interactions channel-rule prerequisite.push: universal missing-plugin throw kept at the call site;presence/annotationsleft unchanged (bundled by the default build, so the throw is modular-only).Validation
npm run docs(TypeDoctreatWarningsAsErrors),prettier --check, andeslintall pass. A clean TypeDoc build confirms every{@link}resolves and nothing became undocumented.Reviewer notes
@seeanchors follow the kebab-case convention (matching the Auth PR) but are unverified — ably/docs #3400 isn't live yet and@seeis not TypeDoc-validated.@seefor the five message-interaction members was deliberately omitted pending a confirmed slug.🤖 Generated with Claude Code