Typedocs: Expand typedocs for publicly exposed interfaces and methods. #641
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR updates documentation across core APIs and React hooks, introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant Component as React Component
participant Provider as ChatClientProvider
participant Manager as RoomReferenceManager
participant Client as ChatClient
participant Ably as Ably Client
Component->>Provider: Wrap with ChatClientProvider
Note over Provider: Initialize on mount
Provider->>Client: Create/retrieve ChatClient
Provider->>Client: addReactAgent()
Provider->>Manager: Create RoomReferenceManager
Note over Provider: Register UI Kit agent if available
Provider->>Client: addAgentWithVersion(name, version)
Provider->>Component: Provide context (Client, Manager)
Component->>Component: useRoom() inside ChatRoomProvider
Note over Component: Get room instance via Manager
Component->>Manager: Reference counting on room
Component->>Ably: Attach to room
Component->>Component: unmount
Note over Manager: Decrement ref count
alt Ref count reaches 0
Manager->>Ably: Detach from room
Manager->>Manager: Release room
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (31)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (14)
🧰 Additional context used📓 Path-based instructions (8)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Files:
⚙️ CodeRabbit configuration file
Files:
{src,test}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/core/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{src,test,demo}/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/core/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/react/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/react-conventions.mdc)
Files:
src/react/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (9)src/core/chat-client.ts (1)
src/core/rooms.ts (2)
src/react/hooks/use-room-reactions.ts (2)
src/react/hooks/use-presence.ts (1)
src/core/room.ts (9)
src/react/hooks/use-messages.ts (4)
src/core/connection.ts (1)
src/core/message-reactions.ts (1)
src/core/occupancy.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (17)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
splindsay-92
force-pushed
the
typedocs/ensure-full-function-context-is-covered
branch
from
064c502 to
6eef38a
Compare
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
splindsay-92
force-pushed
the
typedocs/ensure-full-function-context-is-covered
branch
from
afa1113 to
5cdb9a9
Compare
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/core/message.ts (2)
426-435: UseErrorCodesenum instead of magic numbers; add spec annotation.Per guidelines, replace
40000, 400with specific codes fromsrc/core/errors.ts, and annotate the spec point.+import { ErrorCodes } from './errors.js'; // @CHA-ERR-01 ... - throw new Ably.ErrorInfo('cannot apply a created event to a message', 40000, 400); + // @CHA-ERR-01 Use explicit ErrorCodes enum for consistency + throw new Ably.ErrorInfo('cannot apply a created event to a message', ErrorCodes.MessageEventInvalid, 400);Note: Use the exact enum member names used in
errors.ts.
432-435: Same change for other throws; avoid repeating numeric codes.Update both throws to use
ErrorCodes.*and add the spec annotation comment.- throw new Ably.ErrorInfo('cannot apply event for a different message', 40000, 400); + // @CHA-ERR-01 + throw new Ably.ErrorInfo('cannot apply event for a different message', ErrorCodes.MessageSerialMismatch, 400);- throw new Ably.ErrorInfo('cannot apply event for a different message', 40000, 400); + // @CHA-ERR-01 + throw new Ably.ErrorInfo('cannot apply event for a different message', ErrorCodes.MessageSerialMismatch, 400);Also applies to: 458-460
src/core/messages.ts (1)
770-776: Avoid logging message content/PII; log metadata safelyTrace logs include full params/update/delete details (text, metadata). Redact content; log lengths/keys instead.
Apply this diff:
- this._logger.trace('Messages.send();', { params }); + this._logger.trace('Messages.send();', { + textLength: params?.text?.length ?? 0, + metadataKeys: Object.keys(params?.metadata ?? {}), + headersKeys: Object.keys(params?.headers ?? {}), + });- this._logger.trace('Messages.delete();', { params }); + this._logger.trace('Messages.delete();', { hasDetails: !!params });- this._logger.trace('Messages.update();', { updateParams, details }); + this._logger.trace('Messages.update();', { + textLength: updateParams?.text?.length ?? 0, + metadataKeys: Object.keys(updateParams?.metadata ?? {}), + headersKeys: Object.keys(updateParams?.headers ?? {}), + hasDetails: !!details, + });Also applies to: 792-800, 805-821
src/core/presence.ts (1)
555-597: Argument discrimination bug insubscribe(); incorrect handling when only events are passed.Calling
subscribe(['enter','leave'])compiles (listener optional) but will treat the array as a listener at runtime. Also the “listener optional” overload throws if missing. Make types/runtime consistent.- subscribe( - listenerOrEvents?: PresenceEventType | PresenceEventType[] | PresenceListener, - listener?: PresenceListener, - ): Subscription { + subscribe( + listenerOrEvents: PresenceEventType | PresenceEventType[] | PresenceListener, + listener?: PresenceListener, + ): Subscription { this._logger.trace('Presence.subscribe(); listenerOrEvents', { listenerOrEvents }); @@ - if (!listenerOrEvents && !listener) { - this._logger.error('could not subscribe to presence; invalid arguments'); - throw new Ably.ErrorInfo('could not subscribe listener: invalid arguments', 40000, 400); - } + // Discriminate args + const isFn = typeof listenerOrEvents === 'function'; + const isEvent = typeof listenerOrEvents === 'string' || Array.isArray(listenerOrEvents); + if ((!isFn && !isEvent) || (isEvent && !listener)) { + this._logger.error('could not subscribe to presence; invalid arguments', { listenerOrEvents, listener }); + throw new Ably.ErrorInfo('could not subscribe to presence; invalid arguments', /* ErrorCodes.InvalidArgument */ 40000, 400); + } @@ - if (listener) { + if (isEvent && listener) { const wrapped = wrap(listener); - this._emitter.on(listenerOrEvents as PresenceEventType, wrapped); + this._emitter.on(listenerOrEvents as PresenceEventType | PresenceEventType[], wrapped); return { unsubscribe: () => { this._logger.trace('Presence.unsubscribe();', { events: listenerOrEvents }); this._emitter.off(wrapped); }, }; - } else { + } else { const wrapped = wrap(listenerOrEvents as PresenceListener); this._emitter.on(wrapped); return { unsubscribe: () => { this._logger.trace('Presence.unsubscribe();'); this._emitter.off(wrapped); }, }; }Additionally, change the overload
subscribe(listener?: PresenceListener)to require a listener to match the runtime check.src/core/messages-reactions.ts (1)
446-452: Validatecountonly when unset and enforce ≥1 for Multiple.- if (type === MessageReactionType.Multiple && !count) { - count = 1; - } + if (type === MessageReactionType.Multiple) { + if (count == null) count = 1; + if (count < 1) { + throw new Ably.ErrorInfo( + 'count must be ≥ 1 for Multiple reactions', + /* ErrorCodes.InvalidArgument */ 40000, + 400 + ); + } + }Also update the
send()JSDoc to clarify thatcountdefaults to 1 if unset and must be ≥ 1 whentypeisMultiple.
🧹 Nitpick comments (33)
src/core/message.ts (2)
468-470: Known TODO: reaction summaries freshness.You’re explicitly ignoring newer summaries on the incoming message. Please either document this limitation here or open a tracking issue and reference it in the TODO.
324-354: Optional: defensively copy Date instances to prevent external mutation.Dates are passed/stored by reference; consumers can mutate them. Consider copying in the constructor.
- this.createdAt = createdAt; - this.timestamp = timestamp; + this.createdAt = new Date(createdAt); + this.timestamp = new Date(timestamp);src/react/hooks/use-typing.ts (4)
1-2: Remove ESLint disable by actually usingErrorInfoin JSDoc.Prefer referencing the type via JSDoc
@throwsso the import is justified and the disable comment can go.-// eslint-disable-next-line @typescript-eslint/no-unused-vars -import type { ErrorInfo } from 'ably'; +import type { ErrorInfo } from 'ably';
48-55: Add@throwsand async notes per guidelines; links will useErrorInfo.Document that returned methods may reject with Ably errors to satisfy docs completeness.
/** * A hook that provides access to typing state (e.g. currently typing clients) of the room. * It will use the instance belonging to the room in the nearest {@link ChatRoomProvider} in the component tree. * @param params - Allows the registering of optional callbacks. - * @returns UseTypingResponse - An object containing the {@link Typing} instance and methods to interact with it. + * @returns UseTypingResponse - An object containing the {@link Typing} instance and methods to interact with it. + * @throws {ErrorInfo} Methods returned by this hook (e.g. {@link Typing.keystroke}, {@link Typing.stop}) + * may reject with {@link Ably.ErrorInfo} if the room is not attached or the network request fails. + * // @CHA-DOC-THROWS
137-139: Prefer async/await over.then(...).Keeps style consistent and aids error handling.
- const keystroke = useCallback(() => context.room.then((room) => room.typing.keystroke()), [context]); - const stop = useCallback(() => context.room.then((room) => room.typing.stop()), [context]); + const keystroke = useCallback(async () => { + const room = await context.room; + return room.typing.keystroke(); + }, [context]); + const stop = useCallback(async () => { + const room = await context.room; + return room.typing.stop(); + }, [context]);
62-63: Optional: include context in trace logs.Add lightweight context to trace logs to aid debugging.
- logger.trace('useTyping();'); + logger.trace('useTyping();', { hasListener: Boolean(params?.listener) });src/react/hooks/use-room-reactions.ts (3)
1-2: Same: remove ESLint disable by usingErrorInfoin JSDoc.Use the imported type in JSDoc to avoid the suppression.
-// eslint-disable-next-line @typescript-eslint/no-unused-vars -import type { ErrorInfo } from 'ably'; +import type { ErrorInfo } from 'ably';
36-41: Add@throwsto hook JSDoc.Document possible Ably errors from
sendRoomReaction./** * A hook that provides access to the {@link RoomReactions} instance in the room. * It will use the instance belonging to the nearest {@link ChatRoomProvider} in the component tree. * @param params - Allows the registering of optional callbacks. - * @returns UseRoomReactionsResponse + * @returns UseRoomReactionsResponse + * @throws {ErrorInfo} {@link RoomReactions.send} may reject with {@link Ably.ErrorInfo} if the room is not attached + * or if sending the reaction fails. // @CHA-DOC-THROWS
90-93: Prefer async/await in callback.Align with project style and improve error stack traces.
- const sendRoomReaction = useCallback( - (params: SendReactionParams) => context.room.then((room) => room.reactions.send(params)), - [context], - ); + const sendRoomReaction = useCallback(async (params: SendReactionParams) => { + const room = await context.room; + return room.reactions.send(params); + }, [context]);src/core/rooms.ts (3)
17-61: Doc polish: use {@link} and reference typed error codes
- Replace raw types with links: {@link Room}, {@link RoomOptions}, {@link Ably.ErrorInfo}, {@link Subscription}.
- Replace “code 40000” with a specific constant from ErrorCode (src/core/errors.ts), and reflect that in @throws text.
66-106: Verify accuracy: does release unsubscribe listeners?Docs say release “doesn't unsubscribe existing event listeners,” but DefaultRoom.release() typically disposes features (e.g., messages.dispose()), which removes user listeners. Please confirm actual behavior and align docs accordingly. Also apply {@link Ably.ErrorInfo} and ErrorCode constants in @throws.
287-303: Avoid magic error codes; use ErrorCode enumThrows use 40000 directly. Per guidelines, use specific ErrorCode constants (src/core/errors.ts) to improve clarity and consistency.
src/core/occupancy.ts (3)
21-59: Doc polish: link types and prerequisites
- Link types with {@link}: {@link OccupancyEvent}, {@link Subscription}, {@link Room}.
- Keep “enableEvents”/attach notes; they match runtime checks.
93-135: Doc polish: link types and codify @throws
- Use {@link Ably.ErrorInfo} and an ErrorCode constant in @throws.
- Consider linking {@link OccupancyData}.
206-213: Remove unnecessary casts and use ErrorCode constants
- Throwing Ably.ErrorInfo doesn’t need “as unknown as Error”; ErrorInfo extends Error.
- Replace 40000 with a named constant from ErrorCode.
Apply this diff:
- throw new Ably.ErrorInfo( - 'cannot subscribe to occupancy; occupancy events are not enabled in room options', - 40000, - 400, - ) as unknown as Error; + throw new Ably.ErrorInfo( + 'cannot subscribe to occupancy; occupancy events are not enabled in room options', + /* ErrorCode.OccupancyEventsNotEnabled */ 40000, + 400, + );- throw new Ably.ErrorInfo( - 'cannot get current occupancy; occupancy events are not enabled in room options', - 40000, - 400, - ) as unknown as Error; + throw new Ably.ErrorInfo( + 'cannot get current occupancy; occupancy events are not enabled in room options', + /* ErrorCode.OccupancyEventsNotEnabled */ 40000, + 400, + );Also applies to: 241-246
src/react/hooks/use-presence.ts (2)
336-344: Document the type assertion or replace with a type guard
(room.presence as PresenceWithStateChangeListener)is a bare assertion. Per guidelines, document why it’s safe, or add a type guard.Apply this diff to document:
- const subscription = (room.presence as PresenceWithStateChangeListener).onPresenceStateChange( + // The Presence implementation supports onPresenceStateChange at runtime; we assert here for hooks-level typing context. + // @CHA-TYPEDOC: documented type assertion per guidelines. + const subscription = (room.presence as PresenceWithStateChangeListener).onPresenceStateChange(
60-71: Add @throws to hook-returned methodsAdd
@throws {@link Ably.ErrorInfo}toupdate,enter, andleaveproperty docs, since underlying Presence ops can reject.Also applies to: 97-99, 136-137
src/core/typing.ts (5)
20-60: Doc polish: link types and clarify attachment requirementUse {@link TypingSetEvent}, {@link Subscription}, and {@link Room}. Keep the “room must be attached” note; matches behavior.
64-94: Verify: “Must be subscribed to populate current” may be inaccurateDefaultTyping subscribes to channel events in the constructor, so
current()may update even without user subscriptions. Please verify and adjust the example/note.
96-134: Doc polish: link types and codify @throws with ErrorCode constants
- Use {@link Ably.ErrorInfo} in @throws.
- Reference specific ErrorCode constants instead of numeric 40000/50000 in prose.
Also applies to: 137-179
617-619: Use approved log levelsLogger uses
warn, but guidelines restrict to trace/debug/error. Switch todebug(orerrorif it indicates a fault).Apply this diff:
- this._logger.warn(`DefaultTyping._internalSubscribeToEvents(); unrecognized event`, { + this._logger.debug(`DefaultTyping._internalSubscribeToEvents(); unrecognized event`, { name, });
341-352: Avoid magic error codes; use ErrorCode enumReplace 40000/50000 with specific ErrorCode constants from src/core/errors.ts when throwing ErrorInfo.
Also applies to: 389-399
src/core/messages.ts (4)
620-629: Consistent logger prefixes: use DefaultMessages not DefaultSubscriptionManagerSeveral log messages use “DefaultSubscriptionManager…”. Rename for clarity.
Apply this diff:
- this._logger.trace(`DefaultSubscriptionManager.getBeforeSubscriptionStart();`); + this._logger.trace(`DefaultMessages.getBeforeSubscriptionStart();`);- this._logger.error( - `DefaultSubscriptionManager.getBeforeSubscriptionStart(); listener has not been subscribed yet`, - ); + this._logger.error(`DefaultMessages.getBeforeSubscriptionStart(); listener has not been subscribed yet`);- this._logger.error(`DefaultSubscriptionManager.handleAttach(); channelSerial is undefined`); + this._logger.error(`DefaultMessages.handleAttach(); channelSerial is undefined`);- this._logger.error(`DefaultSubscriptionManager.handleAttach(); attachSerial is undefined`); + this._logger.error(`DefaultMessages.handleAttach(); attachSerial is undefined`);- this._logger.error(`DefaultSubscriptionManager.handleAttach(); attachSerial is undefined`); + this._logger.error(`DefaultMessages.handleAttach(); attachSerial is undefined`);Also applies to: 677-679, 718-723, 739-743
883-887: Remove unnecessary cast on disposal error
Ably.ErrorInfoextendsError; no need foras unknown as Error.Apply this diff:
- const disposalError = new Ably.ErrorInfo('room has been disposed', 40000, 400) as unknown as Error; + const disposalError = new Ably.ErrorInfo('room has been disposed', /* ErrorCode.RoomDisposed */ 40000, 400);
629-633: Avoid magic error codes; use ErrorCode enumReplace 40000 with specific ErrorCode constants throughout this file when throwing or constructing ErrorInfo.
Also applies to: 678-679, 721-723, 741-743, 884-884
234-277: Doc polish: add {@link} links for types and errors
- Link {@link Message}, {@link MessageListener}, {@link MessageSubscriptionResponse}, {@link PaginatedResult}, {@link Ably.ErrorInfo}, {@link OrderBy}.
- Keep sequencing notes; they’re useful.
Also applies to: 281-325, 327-362, 409-462, 465-526
src/core/messages-reactions.ts (4)
372-382: Use approved log levels (trace/debug/error); avoidinfo.Coding guidelines specify trace/debug/error only. Please change
logger.infocalls here todebug(ortrace) for consistency.- this._logger.info('MessagesReactions._processAnnotationEvent(); ignoring unknown reaction type', { event }); + this._logger.debug('MessagesReactions._processAnnotationEvent(); ignoring unknown reaction type', { event }); ... - this._logger.info('MessagesReactions._processAnnotationEvent(); ignoring unknown reaction event type', { event }); + this._logger.debug('MessagesReactions._processAnnotationEvent(); ignoring unknown reaction event type', { event });
467-469: Replace magic error codes with named ErrorCodes.Per guidelines, use the
ErrorCodesenum instead of raw numbers.+import { ErrorCodes } from './errors.js'; ... - throw new Ably.ErrorInfo(`cannot delete reaction of type ${type} without a name`, 40001, 400); + throw new Ably.ErrorInfo(`cannot delete reaction of type ${type} without a name`, ErrorCodes.ReactionNameRequired, 400); ... - throw new Ably.ErrorInfo('Raw message reactions are not enabled', 40001, 400); + throw new Ably.ErrorInfo('Raw message reactions are not enabled', ErrorCodes.RawMessageReactionsDisabled, 400);Please adjust enum member names to actual ones in
src/core/errors.ts. Based on learningsAlso applies to: 498-500
420-423: Prefer type guards over assertions for summary shapes.Avoid
as unknown as ...by introducing a narrow helper.- const unique = (event.summary[ReactionAnnotationType.Unique] ?? {}) as unknown as Ably.SummaryUniqueValues; - const distinct = (event.summary[ReactionAnnotationType.Distinct] ?? {}) as unknown as Ably.SummaryDistinctValues; - const multiple = (event.summary[ReactionAnnotationType.Multiple] ?? {}) as Ably.SummaryMultipleValues; + const uniqueRaw = event.summary[ReactionAnnotationType.Unique] ?? {}; + const distinctRaw = event.summary[ReactionAnnotationType.Distinct] ?? {}; + const multipleRaw = event.summary[ReactionAnnotationType.Multiple] ?? {}; + const unique = isSummaryUniqueValues(uniqueRaw) ? uniqueRaw : {}; + const distinct = isSummaryDistinctValues(distinctRaw) ? distinctRaw : {}; + const multiple = isSummaryMultipleValues(multipleRaw) ? multipleRaw : {};Define
isSummaryUniqueValues/isSummaryDistinctValues/isSummaryMultipleValueslocally with shape checks, and add a brief comment explaining why assertions are avoided. As per coding guidelines
327-327: Add spec annotation marker to new public API.Please add a short spec tag comment on the interface method to trace this addition in future audits (e.g.,
// @CHA-API-PR641-clientReactions).src/core/presence.ts (3)
566-570: Replace magic error codes withErrorCodes; align error messages.Use your central enum and consistent message text.
+import { ErrorCodes } from './errors.js'; ... - throw new Ably.ErrorInfo('could not subscribe to presence; presence events are not enabled', 40000, 400); + throw new Ably.ErrorInfo('could not subscribe to presence; presence events are not enabled', ErrorCodes.PresenceEventsDisabled, 400); ... - this._logger.error('could not subscribe to presence; invalid arguments'); - throw new Ably.ErrorInfo('could not subscribe listener: invalid arguments', 40000, 400); + this._logger.error('could not subscribe to presence; invalid arguments'); + throw new Ably.ErrorInfo('could not subscribe to presence; invalid arguments', ErrorCodes.InvalidArgument, 400); ... - throw new Ably.ErrorInfo('could not perform presence operation; room is not attached', 40000, 400); + throw new Ably.ErrorInfo('could not perform presence operation; room is not attached', ErrorCodes.RoomNotAttached, 400);Adjust enum names to your actual constants. As per coding guidelines
Also applies to: 572-575, 675-678
60-61: Consider a JSON‑serializablePresenceDatatype.
unknownis acceptable, but aJsonValuealias (recursive union) better encodes the persistence/serialization constraint and helps consumers.export type JsonValue = string | number | boolean | null | { [k: string]: JsonValue } | JsonValue[]; export type PresenceData = JsonValue;
324-374: Add spec annotation markers to overload docs.Please add compact spec tags (e.g.,
// @CHA-API-PR641-presence-subscribe-overloads) near the overloads to satisfy the spec‑point annotation guideline.Also applies to: 376-422
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 ignored due to path filters (2)
demo/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
demo/api/ably-token-request/index.ts(1 hunks)package.json(2 hunks)src/core/chat-client.ts(4 hunks)src/core/connection.ts(1 hunks)src/core/message.ts(3 hunks)src/core/messages-reactions.ts(1 hunks)src/core/messages.ts(2 hunks)src/core/occupancy.ts(1 hunks)src/core/presence.ts(1 hunks)src/core/room-reactions.ts(1 hunks)src/core/room.ts(1 hunks)src/core/rooms.ts(2 hunks)src/core/typing.ts(1 hunks)src/react/hooks/internal/use-room-context.ts(1 hunks)src/react/hooks/internal/use-room-reference-manager.ts(1 hunks)src/react/hooks/use-messages.ts(1 hunks)src/react/hooks/use-presence.ts(1 hunks)src/react/hooks/use-room-reactions.ts(1 hunks)src/react/hooks/use-typing.ts(1 hunks)src/react/providers/chat-client-provider.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: When importing in this project, use relative imports.
When importing the package ably, doimport * as Ably from 'ably'.
Use named exports over default exports.
Group related exports together.
Use PascalCase for interface names.
Use descriptive names that reflect the purpose for interfaces.
Prefix interfaces with 'I' is not required.
Use JSDoc style comments for interfaces.
Include a clear description of the interface's purpose in JSDoc.
Document all properties inline, alongside the property itself in interfaces.
Document all errors using@throwsin JSDoc.
Document the return type using@returnin JSDoc.
Link using {@link} to types mentioned in JSDoc.
Use PascalCase for enum names.
Use PascalCase for enum members.
Include JSDoc documentation for both enum and members.
Use PascalCase for class names.
Implement interfaces explicitly in classes.
Avoid extension wherever possible in classes.
Document class purpose and implementation details.
Use underscore prefix for private members in classes.
Group class members by visibility (private, protected, public).
Document all public methods and properties in classes.
Use JSDoc for all public methods.
Make use of@param,@returnsand@throwsin method documentation.
Use descriptive parameter names in methods.
Group related parameters into parameter objects in methods.
Use optional parameters with default values when appropriate in methods.
The error type used isErrorInfofrom the package 'ably'.
Use consistent logging levels (trace, debug, error).
Include relevant context in log messages, the second argument is a free-formed object for any relevant context.
Never log instances of Ably channels.
Structure log messages for easy parsing.
Minimize use of type assertions.
Use type guards when possible.
Document when and why type assertions are necessary.
Never useany. If we really can't avoid it, useunknown, but strong typing always preferred.
Use async/await over raw promises.
Handle promise re...
Files:
src/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/core/rooms.tsdemo/api/ably-token-request/index.tssrc/core/presence.tssrc/core/messages.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/core/connection.tssrc/core/chat-client.tssrc/core/occupancy.tssrc/react/hooks/use-messages.tssrc/core/typing.tssrc/core/room.tssrc/core/message.tssrc/core/room-reactions.tssrc/core/messages-reactions.tssrc/react/hooks/internal/use-room-context.ts
src/react/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React.
Prefix all hooks withuse(e.g.,useMessages,useLogger).
Use descriptive names that reflect the hook's purpose.
Use JSDoc style comments for hooks.
Include clear description of the hook's purpose in JSDoc.
Document all parameters and return values using@param,@returnsand@throwsin hooks.
Group related parameters into a single params object in hooks.
Use TypeScript interfaces to define parameter shapes for hooks.
Return objects with named properties for clarity in hooks.
Define return type interfaces explicitly for hooks.
Document each returned property in hooks.
Memoize callbacks withuseCallbackin hooks.
Memoize expensive computations withuseMemoin hooks.
Use refs for values that shouldn't trigger re-renders in hooks.
Always clean up subscriptions and listeners in hooks.
Handle component unmounting gracefully in hooks.
Clear timers and intervals in cleanup functions in hooks.
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React custom hooks
Prefix all React hook names withuse(e.g.,useMessages,useLogger)
Use descriptive names for hooks that reflect the hook's purpose
Use JSDoc style comments for hooks
Include a clear description of the hook's purpose in its JSDoc comment
Document all hook parameters and return values using@param,@returns, and@throwsin JSDoc
Group related hook parameters into a single params object
Use TypeScript interfaces to define parameter shapes for hooks
Return objects with named properties for clarity from hooks
Define return type interfaces explicitly for hook return values
Document each returned property of a hook in its JSDoc comment
Memoize callbacks in hooks withuseCallback
Memoize expensive computations in hooks withuseMemo
Use refs for values in hooks that shouldn't trigger re-renders
Always clean up subscriptions and listeners in hooks
Handle component unmounting gracefully in hooks
Clear timers and intervals...
Files:
src/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-messages.tssrc/react/hooks/internal/use-room-context.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Files:
src/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/core/rooms.tssrc/core/presence.tssrc/core/messages.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/core/connection.tssrc/core/chat-client.tssrc/core/occupancy.tssrc/react/hooks/use-messages.tssrc/core/typing.tssrc/core/room.tssrc/core/message.tssrc/core/room-reactions.tssrc/core/messages-reactions.tssrc/react/hooks/internal/use-room-context.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/core/rooms.tssrc/core/presence.tssrc/core/messages.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/core/connection.tssrc/core/chat-client.tssrc/core/occupancy.tssrc/react/hooks/use-messages.tssrc/core/typing.tssrc/core/room.tssrc/core/message.tssrc/core/room-reactions.tssrc/core/messages-reactions.tssrc/react/hooks/internal/use-room-context.ts
🧬 Code graph analysis (10)
src/core/rooms.ts (2)
src/core/room.ts (3)
name(492-494)options(499-501)Room(22-364)src/core/room-options.ts (1)
RoomOptions(117-137)
src/core/presence.ts (2)
src/core/index.ts (6)
RealtimePresenceParams(78-78)PresenceMember(66-66)PresenceData(66-66)PresenceEventType(27-27)PresenceListener(66-66)Subscription(76-76)src/core/subscription.ts (1)
Subscription(12-19)
src/core/messages.ts (3)
src/core/query.ts (1)
PaginatedResult(4-39)src/core/message.ts (1)
Message(51-247)src/core/serial.ts (1)
Serial(15-23)
src/core/connection.ts (1)
src/core/subscription.ts (1)
StatusSubscription(33-40)
src/core/chat-client.ts (1)
src/core/rooms.ts (1)
Rooms(15-138)
src/core/occupancy.ts (2)
src/core/subscription.ts (1)
Subscription(12-19)src/core/occupancy-parser.ts (1)
OccupancyData(6-16)
src/core/typing.ts (2)
src/core/index.ts (2)
TypingListener(77-77)Subscription(76-76)src/core/subscription.ts (1)
Subscription(12-19)
src/core/room.ts (9)
src/core/messages.ts (1)
Messages(232-536)src/core/presence.ts (1)
Presence(112-423)src/core/room-reactions.ts (1)
RoomReactions(65-161)src/core/typing.ts (1)
Typing(18-180)src/core/occupancy.ts (1)
Occupancy(19-136)src/core/room-status.ts (1)
RoomStatusListener(88-88)src/core/subscription.ts (1)
StatusSubscription(33-40)src/core/room-options.ts (1)
RoomOptions(117-137)src/core/discontinuity.ts (1)
DiscontinuityListener(7-7)
src/core/room-reactions.ts (1)
src/core/index.ts (1)
SendReactionParams(71-71)
src/core/messages-reactions.ts (3)
src/core/serial.ts (1)
Serial(15-23)src/core/chat-api.ts (2)
SendMessageReactionParams(96-113)DeleteMessageReactionParams(118-129)src/core/subscription.ts (1)
Subscription(12-19)
🔇 Additional comments (10)
src/core/message.ts (1)
1-1: Public API changes detected. Please update the documentation and the Ably CLI.The PR description mentions new public APIs (e.g.,
MessagesReactions.clientReactions, Presence subscribe overloads). Ensure docs and Ably CLI are updated accordingly.demo/api/ably-token-request/index.ts (1)
30-34: Align token capability with PR intent. The code still uses chat-only scope ('[chat]*'). If you intended to broaden to full capabilities ([*:*]*), gate it behind an env var (e.g.DEMO_ALLOW_ALL === 'true'), defaulting to chat-only as shown.src/core/rooms.ts (1)
119-129: LGTM: example aids discoverabilityThe clientOptions example is clear and matches API shape.
src/core/occupancy.ts (1)
63-89: LGTM: get() docs are clearREST semantics, example, and return type are well-explained.
src/react/hooks/use-presence.ts (1)
1-2: Type-only import for Typedoc linkingLGTM. Using
import type { ErrorInfo }with a scoped ESLint disable is fine to aid Typedoc linking without affecting bundles.Please confirm Typedoc cross-links render as intended in your build.
src/core/messages.ts (1)
233-233: Public API changes detected. Please update the documentation and the Ably CLI.Per PR summary (e.g., new MessagesReactions.clientReactions and Presence.subscribe overloads), ensure docs/CLI are updated.
src/core/messages-reactions.ts (2)
327-328: Public API changes detected. Please update the documentation and the Ably CLI.New method
clientReactions(messageSerial, clientId?)was added to the public interface and implementation. Ensure downstream docs/CLI are updated and versioned notes are prepared.Also applies to: 529-533
529-533: LGTM on REST passthrough; add timeout/error mapping confirmation.Implementation is straightforward. Please confirm
ChatApi.getClientReactionsapplies standard request timeout/retry and maps Ably errors toErrorInfoconsistently.src/core/presence.ts (2)
324-374: Public API changes detected. Please update the documentation and the Ably CLI.New
subscribeoverloads were added toPresenceand implemented inDefaultPresence. Ensure CLI/doc artifacts and usage guides reflect overload semantics.Also applies to: 376-422, 555-560
603-608: LGTM: presence event fan‑out.Emitting typed events via the internal emitter is clean and keeps API surface stable.
splindsay-92
force-pushed
the
typedocs/ensure-full-function-context-is-covered
branch
from
5cdb9a9 to
984ea73
Compare
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/core/connection.ts (1)
294-299: Use approved log levels
this._logger.infoviolates our allowed levels (trace, debug, error). Switch todebug(ortrace) to comply.- this._logger.info(`Connection state changed`, change); + this._logger.debug(`Connection state changed`, change);As per coding guidelines
src/core/chat-client.ts (2)
275-276: Bug: spreading possibly undefinedagents
{ ...(realtime.options.agents ?? realtime.options.agents), ... }spreadsundefinedwhenagentsis unset, throwing at runtime. Use an empty object fallback.- realtime.options.agents = { ...(realtime.options.agents ?? realtime.options.agents), [agent]: version ?? VERSION }; + realtime.options.agents = { ...(realtime.options.agents ?? {}), [agent]: version ?? VERSION };
146-152: Type soundness:clientIdcan be undefined; enforce or widen
- Option A (preferred): In
src/core/chat-client.ts’s constructor, validatethis._realtime.auth.clientId, thrownew Ably.ErrorInfo(...)if absent, import the correct enum with• Addimport { ErrorCode } from './errors.js'; if (!this._realtime.auth.clientId) { throw new Ably.ErrorInfo( 'ChatClient requires an Ably clientId', ErrorCode.MissingClientId, 400 ); }MissingClientIdto theErrorCodeenum insrc/core/errors.tsand@throws {Ably.ErrorInfo}to the constructor JSDoc.- Option B: Change the getter signature in
src/core/chat-client.tstoso callers handleget clientId(): string | undefined {undefined.src/core/presence.ts (1)
566-570: Replace raw 40000 with ErrorCode.BadRequest
- In subscribe() when presence events are disabled (lines 568–570): use
ErrorCode.BadRequest.- In subscribe() on invalid arguments (lines 573–575): use
ErrorCode.BadRequest.- In _assertChannelState() when room isn’t attached (lines 676–678): use
ErrorCode.BadRequest.
🧹 Nitpick comments (14)
src/react/hooks/internal/use-room-context.ts (2)
10-10: Use {@link} in @throws to comply with docs guideline.Switch to an inline link for the error type to match “Link using {@link} to types mentioned in JSDoc.”
Apply:
- * @throws {Ably.ErrorInfo} if the hook is not used within a ChatRoomProvider. + * @throws {@link Ably.ErrorInfo} if the hook is not used within a ChatRoomProvider.As per coding guidelines.
16-16: UseErrorCodes.BadRequestinstead of magic literalImport the shared enum and replace
40000withErrorCodes.BadRequestfor consistency:import { Ably } from 'ably/react'; +import { ErrorCodes } from '../../../core/errors'; if (!context) { - throw new Ably.ErrorInfo( - `${callingHook} hook must be used within a <ChatRoomProvider>`, - 40000, - 400, - ); + throw new Ably.ErrorInfo( + `${callingHook} hook must be used within a <ChatRoomProvider>`, + ErrorCodes.BadRequest, + 400, + ); }src/core/message.ts (1)
426-435: Use theErrorCode.BadRequestenum member instead of the magic40000.Replace each literal
40000in yourAbly.ErrorInfocalls (e.g., insrc/core/message.tsaround lines 426–435 and 458–460) withErrorCode.BadRequest, and add:import { ErrorCode } from './errors';src/core/connection.ts (4)
81-99: Nice docs; add type links for discoverabilityAdd {@link ConnectionStatus} to the returns text to enable TypeDoc cross-linking.
As per coding guidelines
104-123: Clarify type and link to Ably errorConsider “@returns {@link Ably.ErrorInfo} if present, otherwise undefined” and add a brief note that successful states typically have no error. Also use {@link Ably.ErrorInfo} for cross-linking.
As per coding guidelines
128-169: Document subscription semantics and link the return type
- Mention whether
subscription.off()is idempotent.- Link the return type using {@link StatusSubscription}.
- The example imports Ably but doesn’t use it; remove to avoid noise.
As per coding guidelines
174-195: Clarify disposal scopeState explicitly that
dispose()only tears down listeners for connection monitoring and does not close the underlying Ably connection.As per coding guidelines
src/core/chat-client.ts (4)
154-170: Good to exposerealtime, but reiterate invariantsAdd a one‑liner that mutating
realtime.optionsor creating channels directly can violate chat invariants; prefer going through Chat APIs unless documented. Add {@link Ably.Realtime}.As per coding guidelines
175-187: Type ofclientOptionssuggests normalized values; align the return type or wordingDocs promise “resolved … including defaults” but the signature returns
ChatClientOptionswhile the field isNormalizedChatClientOptions.Options:
- Return
NormalizedChatClientOptionsand consider re‑exporting it for public typing, or- Keep
ChatClientOptionsbut adjust docs to avoid implying normalization guarantees.
As per coding guidelinesAlso applies to: 189-191
225-255: Add throws documentation fordispose()
await this._rooms.dispose()may reject; add@throws {Ably.ErrorInfo}(with specificErrorCodeswhere applicable) in the JSDoc.As per coding guidelines
103-120: Add cross-links in JSDocUse {@link Rooms}, {@link Connection}, {@link Ably.Realtime}, and link relevant types in examples for richer TypeDoc.
As per coding guidelines
Also applies to: 126-141, 154-170, 175-187
src/core/room.ts (1)
123-142: Link Ably.ErrorInfo in JSDoc.Use {@link Ably.ErrorInfo} for consistency with guidelines that ask for {@link} links to types.
- * @returns ErrorInfo if an error caused the current status, undefined otherwise + * @returns {@link Ably.ErrorInfo} if an error caused the current status, undefined otherwisesrc/core/presence.ts (1)
555-597: Tighten overload handling; remove unnecessary assertions; support arrays explicitly.Current implementation asserts types and assumes single-event when
listeneris provided. Prefer narrowing and passing arrays directly to the emitter. Also unify the error message.- subscribe( - listenerOrEvents?: PresenceEventType | PresenceEventType[] | PresenceListener, - listener?: PresenceListener, - ): Subscription { - this._logger.trace('Presence.subscribe(); listenerOrEvents', { listenerOrEvents }); + subscribe( + listenerOrEvents?: PresenceEventType | PresenceEventType[] | PresenceListener, + listener?: PresenceListener, + ): Subscription { + this._logger.trace('Presence.subscribe();', { listenerOrEvents }); @@ - if (!listenerOrEvents && !listener) { - this._logger.error('could not subscribe to presence; invalid arguments'); - throw new Ably.ErrorInfo('could not subscribe listener: invalid arguments', 40000, 400); - } + if (!listenerOrEvents && !listener) { + const msg = 'could not subscribe to presence; invalid arguments'; + this._logger.error(msg); + throw new Ably.ErrorInfo(msg, 40000, 400); + } @@ - if (listener) { - const wrapped = wrap(listener); - this._emitter.on(listenerOrEvents as PresenceEventType, wrapped); + // event-specific subscription + if (listener) { + const wrapped = wrap(listener); + if (Array.isArray(listenerOrEvents)) { + this._emitter.on(listenerOrEvents, wrapped); + } else { + this._emitter.on(listenerOrEvents as PresenceEventType, wrapped); + } return { unsubscribe: () => { this._logger.trace('Presence.unsubscribe();', { events: listenerOrEvents }); this._emitter.off(wrapped); }, }; - } else { - const wrapped = wrap(listenerOrEvents as PresenceListener); - this._emitter.on(wrapped); + } + + // subscribe to all events + { + const wrapped = wrap(listenerOrEvents as PresenceListener); + this._emitter.on(wrapped); return { unsubscribe: () => { this._logger.trace('Presence.unsubscribe();'); this._emitter.off(wrapped); }, }; - } + }src/core/rooms.ts (1)
287-291: Replace magic number 40000 with ErrorCode.BadRequest
Use theErrorCode.BadRequestenum member instead of the literal 40000. Applies also at lines 301–303.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 ignored due to path filters (1)
demo/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
src/core/chat-client.ts(4 hunks)src/core/connection.ts(1 hunks)src/core/message.ts(3 hunks)src/core/messages-reactions.ts(1 hunks)src/core/messages.ts(2 hunks)src/core/occupancy.ts(1 hunks)src/core/presence.ts(1 hunks)src/core/room-reactions.ts(1 hunks)src/core/room.ts(1 hunks)src/core/rooms.ts(2 hunks)src/core/typing.ts(1 hunks)src/react/hooks/internal/use-room-context.ts(1 hunks)src/react/hooks/internal/use-room-reference-manager.ts(1 hunks)src/react/providers/chat-client-provider.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/core/typing.ts
- src/react/hooks/internal/use-room-reference-manager.ts
- src/core/messages.ts
- src/core/occupancy.ts
- src/react/providers/chat-client-provider.tsx
- src/core/room-reactions.ts
- src/core/messages-reactions.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: When importing in this project, use relative imports.
When importing the package ably, doimport * as Ably from 'ably'.
Use named exports over default exports.
Group related exports together.
Use PascalCase for interface names.
Use descriptive names that reflect the purpose for interfaces.
Prefix interfaces with 'I' is not required.
Use JSDoc style comments for interfaces.
Include a clear description of the interface's purpose in JSDoc.
Document all properties inline, alongside the property itself in interfaces.
Document all errors using@throwsin JSDoc.
Document the return type using@returnin JSDoc.
Link using {@link} to types mentioned in JSDoc.
Use PascalCase for enum names.
Use PascalCase for enum members.
Include JSDoc documentation for both enum and members.
Use PascalCase for class names.
Implement interfaces explicitly in classes.
Avoid extension wherever possible in classes.
Document class purpose and implementation details.
Use underscore prefix for private members in classes.
Group class members by visibility (private, protected, public).
Document all public methods and properties in classes.
Use JSDoc for all public methods.
Make use of@param,@returnsand@throwsin method documentation.
Use descriptive parameter names in methods.
Group related parameters into parameter objects in methods.
Use optional parameters with default values when appropriate in methods.
The error type used isErrorInfofrom the package 'ably'.
Use consistent logging levels (trace, debug, error).
Include relevant context in log messages, the second argument is a free-formed object for any relevant context.
Never log instances of Ably channels.
Structure log messages for easy parsing.
Minimize use of type assertions.
Use type guards when possible.
Document when and why type assertions are necessary.
Never useany. If we really can't avoid it, useunknown, but strong typing always preferred.
Use async/await over raw promises.
Handle promise re...
Files:
src/core/rooms.tssrc/core/message.tssrc/core/presence.tssrc/core/chat-client.tssrc/core/connection.tssrc/core/room.tssrc/react/hooks/internal/use-room-context.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Files:
src/core/rooms.tssrc/core/message.tssrc/core/presence.tssrc/core/chat-client.tssrc/core/connection.tssrc/core/room.tssrc/react/hooks/internal/use-room-context.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/core/rooms.tssrc/core/message.tssrc/core/presence.tssrc/core/chat-client.tssrc/core/connection.tssrc/core/room.tssrc/react/hooks/internal/use-room-context.ts
src/react/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React.
Prefix all hooks withuse(e.g.,useMessages,useLogger).
Use descriptive names that reflect the hook's purpose.
Use JSDoc style comments for hooks.
Include clear description of the hook's purpose in JSDoc.
Document all parameters and return values using@param,@returnsand@throwsin hooks.
Group related parameters into a single params object in hooks.
Use TypeScript interfaces to define parameter shapes for hooks.
Return objects with named properties for clarity in hooks.
Define return type interfaces explicitly for hooks.
Document each returned property in hooks.
Memoize callbacks withuseCallbackin hooks.
Memoize expensive computations withuseMemoin hooks.
Use refs for values that shouldn't trigger re-renders in hooks.
Always clean up subscriptions and listeners in hooks.
Handle component unmounting gracefully in hooks.
Clear timers and intervals in cleanup functions in hooks.
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React custom hooks
Prefix all React hook names withuse(e.g.,useMessages,useLogger)
Use descriptive names for hooks that reflect the hook's purpose
Use JSDoc style comments for hooks
Include a clear description of the hook's purpose in its JSDoc comment
Document all hook parameters and return values using@param,@returns, and@throwsin JSDoc
Group related hook parameters into a single params object
Use TypeScript interfaces to define parameter shapes for hooks
Return objects with named properties for clarity from hooks
Define return type interfaces explicitly for hook return values
Document each returned property of a hook in its JSDoc comment
Memoize callbacks in hooks withuseCallback
Memoize expensive computations in hooks withuseMemo
Use refs for values in hooks that shouldn't trigger re-renders
Always clean up subscriptions and listeners in hooks
Handle component unmounting gracefully in hooks
Clear timers and intervals...
Files:
src/react/hooks/internal/use-room-context.ts
🧬 Code graph analysis (5)
src/core/rooms.ts (2)
src/core/room.ts (3)
name(492-494)options(499-501)Room(22-364)src/core/room-options.ts (1)
RoomOptions(117-137)
src/core/presence.ts (2)
src/core/index.ts (6)
RealtimePresenceParams(78-78)PresenceMember(66-66)PresenceData(66-66)PresenceEventType(27-27)PresenceListener(66-66)Subscription(76-76)src/core/subscription.ts (1)
Subscription(12-19)
src/core/chat-client.ts (1)
src/core/rooms.ts (1)
Rooms(15-138)
src/core/connection.ts (1)
src/core/subscription.ts (1)
StatusSubscription(33-40)
src/core/room.ts (9)
src/core/messages.ts (1)
Messages(232-536)src/core/presence.ts (1)
Presence(112-423)src/core/room-reactions.ts (1)
RoomReactions(65-161)src/core/typing.ts (1)
Typing(18-180)src/core/occupancy.ts (1)
Occupancy(19-136)src/core/room-status.ts (1)
RoomStatusListener(88-88)src/core/subscription.ts (1)
StatusSubscription(33-40)src/core/room-options.ts (1)
RoomOptions(117-137)src/core/discontinuity.ts (1)
DiscontinuityListener(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: test node 18
🔇 Additional comments (12)
src/core/message.ts (3)
232-238: LGTM: with() @throws entries now reflect actual behavior.The documented
@throws {Ably.ErrorInfo}cases match the implementation paths (created-event and different-message).
198-201: Docs/contract mismatch: remove incorrect @throws on before().
before()does not throw; delete the@throws {Ably.ErrorInfo}line to keep docs accurate. As per coding guidelines.- * @throws {Ably.ErrorInfo} if serials of either message is invalid.
208-211: Docs/contract mismatch: remove incorrect @throws on after().
after()does not throw; delete the@throws {Ably.ErrorInfo}line. As per coding guidelines.- * @throws {Ably.ErrorInfo} if serials of either message is invalid.src/core/chat-client.ts (2)
61-72: Prevent duplicateconst realtimeClientin examples; make snippets copy/pasteableSplit the “production” and “development” alternatives into separate code fences or rename the second variable (e.g.,
realtimeClientWithKey). Ensure the logging example defines its ownrealtimeClientso each snippet is self‑contained.Also applies to: 68-72
146-191: Public API changes detected. Please update the documentation and the Ably CLI.As per coding guidelines
src/core/room.ts (3)
19-23: Doc polish looks good.Expanded docs, examples, and return descriptions read well and match the current runtime surface.
Also applies to: 36-49, 50-61, 63-76, 77-90, 91-103, 104-121, 188-236, 238-268, 270-309, 311-349, 349-364
150-151: Use StatusSubscription.off() (example and text).The API returns StatusSubscription (with off()), not a Subscription with unsubscribe(). Update the JSDoc and sample accordingly.
- * @returns Subscription object with an unsubscribe method + * @returns {@link StatusSubscription} with an `off()` method @@ - * // Clean up when done - * statusSubscription.unsubscribe(); + * // Clean up when done + * statusSubscription.off();Also applies to: 182-184
321-323: Discontinuity subscription teardown should call off(), not unsubscribe().onDiscontinuity returns StatusSubscription (off()). Align both the narrative and code sample.
- * @returns Subscription object with an unsubscribe method + * @returns {@link StatusSubscription} with an `off()` method @@ - * // Clean up - * discontinuitySubscription.unsubscribe(); + * // Clean up + * discontinuitySubscription.off();Also applies to: 343-345
src/core/rooms.ts (1)
194-213: Lifecycle/get/release flow LGTM.Single-instance semantics, abort on concurrent release, and “wait-for-release” re-get path are all solid and well‑logged.
Also applies to: 217-232, 241-267, 320-331, 341-359, 432-447, 454-466, 473-479
src/core/presence.ts (3)
374-375: Public API changes detected. Please update the documentation and the Ably CLI.Presence gained new subscribe overloads. Ensure external docs and CLI are updated accordingly.
Also applies to: 422-423
488-495: Presence core methods/readability LGTM.Docs are clear, logging is consistent, and promise paths handle errors with state emission.
Also applies to: 500-505, 510-520, 525-535, 540-550
674-678: Doc vs runtime: allow “attaching” or require “attached”?_assertChannelState() allows 'attaching' but JSDoc says “room must be attached”. Either tighten the check to
current === 'attached'or update JSDoc to say “attached (or attaching; operations are queued)”.
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/core/room.ts (1)
244-266: Document possible errors for detach().Detach can fail (network/transport/state). Add @throws to meet the guidelines on documenting errors.
* @returns Promise that resolves when the room is successfully detached + * @throws {Ably.ErrorInfo} if detaching fails due to network or state errorssrc/core/messages.ts (1)
620-631: Normalize log prefixes: replace "DefaultSubscriptionManager" with "Messages" (or "DefaultMessages").Several log statements use an outdated prefix, which hurts traceability. Use a consistent component name.
- this._logger.trace(`DefaultSubscriptionManager.getBeforeSubscriptionStart();`); + this._logger.trace(`Messages.getBeforeSubscriptionStart();`); - this._logger.error( - `DefaultSubscriptionManager.getBeforeSubscriptionStart(); listener has not been subscribed yet`, - ); + this._logger.error('Messages.getBeforeSubscriptionStart(); listener has not been subscribed yet'); - this._logger.error(`DefaultSubscriptionManager.handleAttach(); channelSerial is undefined`); + this._logger.error('Messages.handleAttach(); channelSerial is undefined'); - this._logger.debug('Messages._subscribeAtChannelAttach(); channel is attached already, using attachSerial', { + this._logger.debug('Messages._subscribeAtChannelAttach(); channel already attached, using attachSerial', { - this._logger.error(`DefaultSubscriptionManager.handleAttach(); attachSerial is undefined`); + this._logger.error('Messages.handleAttach(); attachSerial is undefined'); - this._logger.debug('Messages._subscribeAtChannelAttach(); channel is now attached, using attachSerial', { + this._logger.debug('Messages._subscribeAtChannelAttach(); channel attached, using attachSerial', { - this._logger.error(`DefaultSubscriptionManager.handleAttach(); attachSerial is undefined`); + this._logger.error('Messages.handleAttach(); attachSerial is undefined');Also applies to: 676-681, 708-723, 727-741
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (2)
src/core/messages.ts(2 hunks)src/core/room.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: When importing in this project, use relative imports.
When importing the package ably, doimport * as Ably from 'ably'.
Use named exports over default exports.
Group related exports together.
Use PascalCase for interface names.
Use descriptive names that reflect the purpose for interfaces.
Prefix interfaces with 'I' is not required.
Use JSDoc style comments for interfaces.
Include a clear description of the interface's purpose in JSDoc.
Document all properties inline, alongside the property itself in interfaces.
Document all errors using@throwsin JSDoc.
Document the return type using@returnin JSDoc.
Link using {@link} to types mentioned in JSDoc.
Use PascalCase for enum names.
Use PascalCase for enum members.
Include JSDoc documentation for both enum and members.
Use PascalCase for class names.
Implement interfaces explicitly in classes.
Avoid extension wherever possible in classes.
Document class purpose and implementation details.
Use underscore prefix for private members in classes.
Group class members by visibility (private, protected, public).
Document all public methods and properties in classes.
Use JSDoc for all public methods.
Make use of@param,@returnsand@throwsin method documentation.
Use descriptive parameter names in methods.
Group related parameters into parameter objects in methods.
Use optional parameters with default values when appropriate in methods.
The error type used isErrorInfofrom the package 'ably'.
Use consistent logging levels (trace, debug, error).
Include relevant context in log messages, the second argument is a free-formed object for any relevant context.
Never log instances of Ably channels.
Structure log messages for easy parsing.
Minimize use of type assertions.
Use type guards when possible.
Document when and why type assertions are necessary.
Never useany. If we really can't avoid it, useunknown, but strong typing always preferred.
Use async/await over raw promises.
Handle promise re...
Files:
src/core/room.tssrc/core/messages.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Files:
src/core/room.tssrc/core/messages.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/core/room.tssrc/core/messages.ts
🧬 Code graph analysis (2)
src/core/room.ts (9)
src/core/messages.ts (1)
Messages(232-536)src/core/presence.ts (1)
Presence(112-423)src/core/room-reactions.ts (1)
RoomReactions(65-161)src/core/typing.ts (1)
Typing(18-180)src/core/occupancy.ts (1)
Occupancy(19-136)src/core/room-status.ts (1)
RoomStatusListener(88-88)src/core/subscription.ts (1)
StatusSubscription(33-40)src/core/room-options.ts (1)
RoomOptions(117-137)src/core/discontinuity.ts (1)
DiscontinuityListener(7-7)
src/core/messages.ts (3)
src/core/query.ts (1)
PaginatedResult(4-39)src/core/message.ts (1)
Message(51-245)src/core/serial.ts (1)
Serial(15-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Cursor Bugbot
- GitHub Check: test node 24
- GitHub Check: test node 22
- GitHub Check: test node 20
- GitHub Check: test node 18
🔇 Additional comments (5)
src/core/messages.ts (2)
186-224: historyBeforeSubscribe docs match behavior—nice.Newest‑first ordering and reset-on-discontinuity semantics align with _getBeforeSubscriptionStart and _handleAttach implementation. No action needed.
If you have an integration test harness, consider adding a discontinuity test to assert the reset behavior after a forced detach without resume.
378-405: Template literal in send() example is correct now.The previous interpolation bug has been fixed. Thanks for addressing it.
src/core/room.ts (3)
149-151: Use StatusSubscription.off() in the @returns text (not "unsubscribe").Narrative says "Subscription object with an unsubscribe method" but the API exposes off(). The example already uses off(); update the @returns text to match.
- * @returns Subscription object with an unsubscribe method + * @returns {@link StatusSubscription} with an off() method to remove the listener
321-324: Align discontinuity teardown text with API: off(), not "unsubscribe".The return description still mentions "unsubscribe"; this should be off() to reflect StatusSubscription.
- * @returns Subscription object with an unsubscribe method + * @returns {@link StatusSubscription} with an off() method to stop receiving discontinuity events
195-201: attach() correctly rejects on both Suspended and Failed states – the catch block maps any channel.attach() error (whether the channel is Suspended or Failed) to the corresponding RoomStatus and rethrows; the docs are accurate and need no change.
There was a problem hiding this comment.
Please add a JIRA ticket to this PR
General note - we should have some way of delimiting in the examples what is "your function" and what is the Chat SDK. Otherwise users (and LLMs) are likely to start assuming that the SDK has functions that it does not.
I've done a partial review (just the core folder) so those changes can be addressed in other places if required.
src/core/chat-client.ts
Outdated
| * // Get a room | ||
| * const room = await chatClient.rooms.get('general-chat'); | ||
| * | ||
| * // Get a room with options |
There was a problem hiding this comment.
Worth a comment that defaults are taken for anything not specified as an override here and perhaps signposting towards the ChatClientOptions type?
There was a problem hiding this comment.
Should this be mentioned on the .get() function of the room instance instead? I'm tempted to remove the examples for feature accessors and force users/LLMs to rely on the interfaces defined for those features instead. Wdyt?
| * - Message subscriptions automatically reset their position on discontinuity. | ||
| * @param handler - Callback invoked when a discontinuity is detected | ||
| * @returns Subscription object with an unsubscribe method | ||
| * @example |
There was a problem hiding this comment.
This should definitely have a historyOnSubscribe example
There was a problem hiding this comment.
I've added an example to the historyOnSubscribe interface definition, perhaps it's good enough to link to that here?
splindsay-92
force-pushed
the
typedocs/ensure-full-function-context-is-covered
branch
from
ff2cf81 to
02a114f
Compare
github-actions
bot
temporarily deployed
to
staging/pull/641/typedoc
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/presence.ts (1)
577-601: Enforce listener argument when subscribing by event.With the new overload,
subscribe('enter')passes because the second parameter is optional, but at runtime we end up callingwrap(listenerOrEvents as PresenceListener)with a string.wrapexpects a function and will throw aTypeError, so this path now fails where it previously would have been a compile-time error. Please fail fast with a clearErrorInfowhen events are provided without a listener.- if (!listenerOrEvents && !listener) { - this._logger.error('could not subscribe to presence; invalid arguments'); - throw new Ably.ErrorInfo('could not subscribe listener: invalid arguments', 40000, 400); - } - - // Add listener to all events - if (listener) { - const wrapped = wrap(listener); - this._emitter.on(listenerOrEvents as PresenceEventType, wrapped); + const isListenerArgument = typeof listenerOrEvents === 'function'; + + if (!isListenerArgument && typeof listener !== 'function') { + this._logger.error('could not subscribe to presence; listener is required when specifying events'); + throw new Ably.ErrorInfo('could not subscribe listener: listener is required when specifying events', 40000, 400); + } + + if (!isListenerArgument) { + const wrapped = wrap(listener as PresenceListener); + this._emitter.on(listenerOrEvents as PresenceEventType | PresenceEventType[], wrapped); return { unsubscribe: () => { this._logger.trace('Presence.unsubscribe();', { events: listenerOrEvents }); this._emitter.off(wrapped); }, }; } else { - const wrapped = wrap(listenerOrEvents as PresenceListener); + const wrapped = wrap(listenerOrEvents as PresenceListener); this._emitter.on(wrapped); return { unsubscribe: () => { this._logger.trace('Presence.unsubscribe();'); this._emitter.off(wrapped); }, }; }src/core/chat-client.ts (1)
174-181: Remove empty@exampletag and update documentation.The
@exampletag at line 178 is empty, which creates malformed JSDoc. Either add example content or remove the tag. Given past feedback that simple getters don't need examples, removing it is appropriate.Additionally, this is a new public API addition.
Apply this diff:
/** * The configuration options used to initialize the chat client. * @returns The resolved client options including defaults - * @example */ get clientOptions(): ChatClientOptions {Public API changes detected. Please update the documentation and the Ably CLI.
Based on coding guidelines.
🧹 Nitpick comments (2)
src/core/chat-client.ts (2)
108-126: Consider adding@returnsJSDoc tag for consistency.The JSDoc describes the return value inline but doesn't use the
@returnstag. Per coding guidelines, method documentation should use@returns.Apply this diff:
/** * Provides access to the rooms instance for creating and managing chat rooms. - * @returns The Rooms instance for managing chat rooms + * @returns The {@link Rooms} instance for managing chat rooms * @example
132-147: Add@returnsJSDoc tag for consistency.Similar to the rooms getter, this lacks a
@returnstag.Apply this diff:
/** * Provides access to the underlying connection to Ably for monitoring connectivity. - * @returns The Connection instance + * @returns The {@link Connection} instance * @example
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 ignored due to path filters (2)
demo/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
package.json(2 hunks)src/core/chat-client.ts(4 hunks)src/core/connection.ts(1 hunks)src/core/message.ts(1 hunks)src/core/messages-reactions.ts(2 hunks)src/core/messages.ts(2 hunks)src/core/occupancy.ts(1 hunks)src/core/presence.ts(1 hunks)src/core/room-reactions.ts(2 hunks)src/core/room.ts(1 hunks)src/core/rooms.ts(2 hunks)src/core/typing.ts(1 hunks)src/react/hooks/internal/use-room-context.ts(1 hunks)src/react/hooks/internal/use-room-reference-manager.ts(1 hunks)src/react/hooks/use-messages.ts(1 hunks)src/react/hooks/use-presence.ts(1 hunks)src/react/hooks/use-room-reactions.ts(1 hunks)src/react/hooks/use-typing.ts(1 hunks)src/react/providers/chat-client-provider.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/core/rooms.ts
- src/core/messages.ts
- src/core/occupancy.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/react/hooks/use-presence.ts
- package.json
- src/core/room.ts
- src/react/hooks/use-typing.ts
- src/react/hooks/use-room-reactions.ts
- src/react/hooks/internal/use-room-context.ts
- src/react/hooks/internal/use-room-reference-manager.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: When importing in this project, use relative imports.
When importing the package ably, doimport * as Ably from 'ably'.
Use named exports over default exports.
Group related exports together.
Use PascalCase for interface names.
Use descriptive names that reflect the purpose for interfaces.
Prefix interfaces with 'I' is not required.
Use JSDoc style comments for interfaces.
Include a clear description of the interface's purpose in JSDoc.
Document all properties inline, alongside the property itself in interfaces.
Document all errors using@throwsin JSDoc.
Document the return type using@returnin JSDoc.
Link using {@link} to types mentioned in JSDoc.
Use PascalCase for enum names.
Use PascalCase for enum members.
Include JSDoc documentation for both enum and members.
Use PascalCase for class names.
Implement interfaces explicitly in classes.
Avoid extension wherever possible in classes.
Document class purpose and implementation details.
Use underscore prefix for private members in classes.
Group class members by visibility (private, protected, public).
Document all public methods and properties in classes.
Use JSDoc for all public methods.
Make use of@param,@returnsand@throwsin method documentation.
Use descriptive parameter names in methods.
Group related parameters into parameter objects in methods.
Use optional parameters with default values when appropriate in methods.
The error type used isErrorInfofrom the package 'ably'.
Use consistent logging levels (trace, debug, error).
Include relevant context in log messages, the second argument is a free-formed object for any relevant context.
Never log instances of Ably channels.
Structure log messages for easy parsing.
Minimize use of type assertions.
Use type guards when possible.
Document when and why type assertions are necessary.
Never useany. If we really can't avoid it, useunknown, but strong typing always preferred.
Use async/await over raw promises.
Handle promise re...
Files:
src/react/providers/chat-client-provider.tsxsrc/core/typing.tssrc/core/chat-client.tssrc/core/message.tssrc/core/room-reactions.tssrc/core/presence.tssrc/core/messages-reactions.tssrc/react/hooks/use-messages.tssrc/core/connection.ts
src/react/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React.
Prefix all hooks withuse(e.g.,useMessages,useLogger).
Use descriptive names that reflect the hook's purpose.
Use JSDoc style comments for hooks.
Include clear description of the hook's purpose in JSDoc.
Document all parameters and return values using@param,@returnsand@throwsin hooks.
Group related parameters into a single params object in hooks.
Use TypeScript interfaces to define parameter shapes for hooks.
Return objects with named properties for clarity in hooks.
Define return type interfaces explicitly for hooks.
Document each returned property in hooks.
Memoize callbacks withuseCallbackin hooks.
Memoize expensive computations withuseMemoin hooks.
Use refs for values that shouldn't trigger re-renders in hooks.
Always clean up subscriptions and listeners in hooks.
Handle component unmounting gracefully in hooks.
Clear timers and intervals in cleanup functions in hooks.
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React custom hooks
Prefix all React hook names withuse(e.g.,useMessages,useLogger)
Use descriptive names for hooks that reflect the hook's purpose
Use JSDoc style comments for hooks
Include a clear description of the hook's purpose in its JSDoc comment
Document all hook parameters and return values using@param,@returns, and@throwsin JSDoc
Group related hook parameters into a single params object
Use TypeScript interfaces to define parameter shapes for hooks
Return objects with named properties for clarity from hooks
Define return type interfaces explicitly for hook return values
Document each returned property of a hook in its JSDoc comment
Memoize callbacks in hooks withuseCallback
Memoize expensive computations in hooks withuseMemo
Use refs for values in hooks that shouldn't trigger re-renders
Always clean up subscriptions and listeners in hooks
Handle component unmounting gracefully in hooks
Clear timers and intervals...
Files:
src/react/providers/chat-client-provider.tsxsrc/react/hooks/use-messages.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Files:
src/react/providers/chat-client-provider.tsxsrc/core/typing.tssrc/core/chat-client.tssrc/core/message.tssrc/core/room-reactions.tssrc/core/presence.tssrc/core/messages-reactions.tssrc/react/hooks/use-messages.tssrc/core/connection.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/react/providers/chat-client-provider.tsxsrc/core/typing.tssrc/core/chat-client.tssrc/core/message.tssrc/core/room-reactions.tssrc/core/presence.tssrc/core/messages-reactions.tssrc/react/hooks/use-messages.tssrc/core/connection.ts
🧬 Code graph analysis (6)
src/core/typing.ts (1)
src/core/subscription.ts (1)
Subscription(12-19)
src/core/chat-client.ts (1)
src/core/rooms.ts (1)
Rooms(15-138)
src/core/room-reactions.ts (1)
src/core/index.ts (1)
SendReactionParams(72-72)
src/core/presence.ts (1)
src/core/subscription.ts (1)
Subscription(12-19)
src/core/messages-reactions.ts (3)
src/core/serial.ts (1)
Serial(15-23)src/core/chat-api.ts (2)
SendMessageReactionParams(78-95)DeleteMessageReactionParams(100-111)src/core/subscription.ts (1)
Subscription(12-19)
src/core/connection.ts (1)
src/core/index.ts (2)
ConnectionStatus(8-8)ErrorInfo(79-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test node 24
- GitHub Check: test node 22
- GitHub Check: test node 20
- GitHub Check: test node 18
- GitHub Check: Cursor Bugbot
🔇 Additional comments (11)
src/react/hooks/use-messages.ts (1)
1-2: LGTM! Type-only import for JSDoc references.The type-only import of
ErrorInfowith the eslint-disable comment is appropriate for JSDoc-only type references. This pattern avoids runtime overhead while maintaining type safety in documentation.src/core/typing.ts (4)
20-63: LGTM! Comprehensive documentation for subscribe.The expanded JSDoc provides clear behavioral context, usage examples, and all required documentation tags. The example effectively demonstrates real-world usage patterns.
67-99: LGTM! Clear documentation for current.The JSDoc clearly explains the snapshot nature of the method and includes a helpful example showing the prerequisite subscription pattern.
103-135: LGTM! Thorough documentation for keystroke.The JSDoc accurately documents the throttling behavior, serialization guarantees, error conditions with specific error codes, and includes a practical example with error handling.
139-180: LGTM! Complete documentation for stop.The JSDoc accurately describes the no-op behavior, serialization semantics, error conditions with codes, and provides a realistic usage example showing the keystroke-to-stop flow.
src/core/room-reactions.ts (2)
17-17: LGTM! Enhanced property documentation.The updated description provides concrete examples of reaction names (emojis and strings), improving clarity for API consumers.
104-156: LGTM! Excellent documentation for subscribe.The JSDoc provides comprehensive behavioral context, clear use cases, and an extensive example demonstrating realistic usage patterns including conditional logic for different reaction types.
src/core/messages-reactions.ts (1)
334-340: Public API update notice.Public API changes detected. Please update the documentation and the Ably CLI.
src/core/chat-client.ts (3)
153-158: LGTM! Clean clientId accessor.Simple getter with appropriate documentation. No example needed for this straightforward property access.
160-172: Public API changes detected. Please update the documentation and the Ably CLI.New public getter
realtimehas been added to expose the underlying Ably Realtime client. The warning about unexpected behavior is appropriate.Based on coding guidelines.
215-245: Excellent documentation expansion for dispose.The expanded JSDoc clearly communicates the disposal semantics, including the critical point that all rooms are released and the client becomes unusable. The example demonstrates proper usage with error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/messages-reactions.ts (1)
179-227: Consider reordering attach/subscribe in exampleA past reviewer (AndyTWF) suggested moving attachment before subscription in examples, as most users would want subscriptions active during init before attaching. This is a common pattern across the codebase.
Consider reordering the example to attach first, then show how events are received:
* const subscription = room.messages.reactions.subscribe((event: MessageReactionSummaryEvent) => { * // ... event handling * }); * + * // Attach to the room to start receiving events + * await room.attach(); + * * // Handle unique reactions (only one reaction per user) * if (summary.unique) { * // ... * } - * - * // Attach to the room to start receiving events - * await room.attach();This pattern better reflects the typical initialization sequence.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (3)
src/core/messages-reactions.ts(2 hunks)src/core/messages.ts(2 hunks)src/core/occupancy.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/occupancy.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: When importing in this project, use relative imports.
When importing the package ably, doimport * as Ably from 'ably'.
Use named exports over default exports.
Group related exports together.
Use PascalCase for interface names.
Use descriptive names that reflect the purpose for interfaces.
Prefix interfaces with 'I' is not required.
Use JSDoc style comments for interfaces.
Include a clear description of the interface's purpose in JSDoc.
Document all properties inline, alongside the property itself in interfaces.
Document all errors using@throwsin JSDoc.
Document the return type using@returnin JSDoc.
Link using {@link} to types mentioned in JSDoc.
Use PascalCase for enum names.
Use PascalCase for enum members.
Include JSDoc documentation for both enum and members.
Use PascalCase for class names.
Implement interfaces explicitly in classes.
Avoid extension wherever possible in classes.
Document class purpose and implementation details.
Use underscore prefix for private members in classes.
Group class members by visibility (private, protected, public).
Document all public methods and properties in classes.
Use JSDoc for all public methods.
Make use of@param,@returnsand@throwsin method documentation.
Use descriptive parameter names in methods.
Group related parameters into parameter objects in methods.
Use optional parameters with default values when appropriate in methods.
The error type used isErrorInfofrom the package 'ably'.
Use consistent logging levels (trace, debug, error).
Include relevant context in log messages, the second argument is a free-formed object for any relevant context.
Never log instances of Ably channels.
Structure log messages for easy parsing.
Minimize use of type assertions.
Use type guards when possible.
Document when and why type assertions are necessary.
Never useany. If we really can't avoid it, useunknown, but strong typing always preferred.
Use async/await over raw promises.
Handle promise re...
Files:
src/core/messages-reactions.tssrc/core/messages.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Files:
src/core/messages-reactions.tssrc/core/messages.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/core/messages-reactions.tssrc/core/messages.ts
🧬 Code graph analysis (2)
src/core/messages-reactions.ts (3)
src/core/serial.ts (1)
Serial(15-23)src/core/chat-api.ts (2)
SendMessageReactionParams(78-95)DeleteMessageReactionParams(100-111)src/core/subscription.ts (1)
Subscription(12-19)
src/core/messages.ts (3)
src/core/query.ts (1)
PaginatedResult(4-39)src/core/message.ts (1)
Message(68-251)src/core/serial.ts (1)
Serial(15-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test node 22
🔇 Additional comments (13)
src/core/messages.ts (7)
188-288: LGTM - Comprehensive documentation with clear examplesThe expanded JSDoc for
historyBeforeSubscribeprovides excellent context about continuity handling and discontinuity scenarios. The two examples clearly demonstrate both initial population and discontinuity refresh patterns.
299-344: LGTM - Clear subscription documentationThe JSDoc effectively explains the subscription mechanism and provides a practical example showing event handling and lifecycle management.
348-399: Good pagination example - Consider addressing past reviewer commentThe documentation includes a helpful pagination loop example. However, a past reviewer comment (AndyTWF) suggested making this a loop to show pagination over time, which this implementation addresses well.
444-487: Address reviewer's comment about attachmentsPast reviewer comment (AndyTWF at line 447) requested explicitly calling out that attachments are not required. The current documentation mentions metadata and headers are optional but doesn't explicitly state that attachments (if that refers to file attachments) are not required or not supported.
Could you clarify what "attachments" refers to in the reviewer's comment? If it means that
metadataandheadersare optional parameters, consider adding a note like:* @param params - Message parameters containing the text and optional metadata/headers + * **Note**: Only text content is supported. The `metadata` and `headers` parameters are optional.Otherwise, if "attachments" refers to file attachments as a feature, this should be explicitly documented as unsupported if that's the case.
491-546: LGTM - Comprehensive soft delete documentationThe documentation clearly explains soft delete semantics, timing considerations, and error cases with practical examples.
549-612: LGTM - Clear PUT-like semantics explanationThe documentation effectively explains the non-merge behavior for omitted fields and provides comprehensive error handling examples.
615-622: LGTM - Concise reactions property documentationClear and concise documentation for the reactions property.
src/core/messages-reactions.ts (6)
84-126: LGTM - Clear send() documentation with multiple examplesThe JSDoc effectively covers different reaction types (Distinct, Multiple) with practical examples. The note about room attachment requirement is clear.
130-175: LGTM - Comprehensive delete() documentationThe documentation clearly explains deletion behavior for different reaction types and provides comprehensive error handling examples.
231-281: LGTM - Clear subscribeRaw() documentation with requirement calloutThe documentation effectively explains the difference between raw and summary events, clearly states the requirement for enabling raw reactions, and provides a practical example.
285-334: New public API method - well documentedThe new
clientReactions()method is well documented with clear use case explanation (handling clipped summaries) and a comprehensive example.
536-540: Public API changes detectedThe addition of the
clientReactions()method to theMessagesReactionsinterface represents a new public API surface.As per coding guidelines: "If there are any changes to the public-facing API, add a review comment: 'Public API changes detected. Please update the documentation and the Ably CLI.'"
Public API changes detected. Please update the documentation and the Ably CLI.
Specifically, the new
clientReactions(messageSerial: Serial, clientId?: string): Promise<Message['reactions']>method has been added to theMessagesReactionsinterface and needs to be reflected in external documentation and tooling.
88-89: Inconsistent wording about room attachment requirementThe note states "does require the room to be attached" but a past reviewer (AndyTWF) suggested changing similar wording to "does not require" to avoid suggesting users shouldn't call the method when attached. However, the current wording appears correct if the room MUST be attached.
Please verify the actual requirement:
- If the room MUST be attached: Current wording is correct but could be clearer: "This method requires the room to be attached before it can be called."
- If the room does NOT need to be attached: Change to "This method uses the Ably Chat REST API and so does not require the room to be attached to be called."
The same pattern appears in lines 137-138, 291-292. Please ensure consistency across all methods based on the actual technical requirement.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/message-reactions.ts (1)
473-473: Use ErrorCodes enum instead of hardcoded error codes.Lines 473 and 504 use hardcoded error code
40001instead of importing and using theErrorCodesenum fromsrc/core/errors.ts.As per coding guidelines: "Use specific error codes from the ErrorCodes enum" and "use ErrorCodes from src/core/errors.ts".
Import the ErrorCodes enum at the top of the file:
import * as Ably from 'ably'; import { ChannelOptionsMerger } from './channel-manager.js'; import { ChatApi, DeleteMessageReactionParams as APIDeleteMessageReactionParams, SendMessageReactionParams as APISendMessageReactionParams, } from './chat-api.js'; +import { ErrorCodes } from './errors.js';Then replace the hardcoded values:
- throw new Ably.ErrorInfo(`cannot delete reaction of type ${type} without a name`, 40001, 400); + throw new Ably.ErrorInfo(`Cannot delete reaction of type ${type} without a name`, ErrorCodes.BadRequest, 400);- throw new Ably.ErrorInfo('Raw message reactions are not enabled', 40001, 400); + throw new Ably.ErrorInfo('Raw message reactions are not enabled', ErrorCodes.BadRequest, 400);Note: Also capitalized the error message on line 473 for consistency.
Based on coding guidelines.
Also applies to: 504-504
🧹 Nitpick comments (11)
src/react/hooks/use-occupancy.ts (1)
99-99: Consider clarifying the chatClient declaration in the example.The line
const chatClient: ChatClient; // existing ChatClient instanceis a type-only declaration without initialization, which might confuse readers copying the example code. Consider either:
- Showing actual initialization:
const chatClient = new ChatClient({ key: 'YOUR_API_KEY' });- Or making it clearer it's a placeholder:
const chatClient: ChatClient = ...; // assume existing instancesrc/core/room-reactions.ts (1)
106-159: Comprehensive subscription documentation with detailed example.The documentation clearly explains the subscribe behavior and the need for room attachment. The example is thorough, showing event handling, reaction type switching, and proper cleanup.
Minor observation: The example uses placeholder functions (
showFloatingHeart,showApplauseAnimation,showGenericReaction) that would need implementation. Consider adding a brief comment noting these are example functions to avoid confusion, though this is acceptable for illustrative code.Consider adding a clarifying comment:
* // Handle different reaction types * switch (reaction.name) { * case '❤️': - * // Show floating heart animation + * // Show floating heart animation (implement your custom UI here) * showFloatingHeart(reaction.isSelf ? 'own' : 'other');src/core/message-reactions.ts (1)
92-92: Inconsistent error documentation terminology.The
@throwsannotations use inconsistent terminology for the same error condition:
- Line 92: "status code 404"
- Line 143: "code 40400"
- Line 300: "status code 404"
Per coding guidelines, error code 40400 corresponds to HTTP status code 404 (first 3 digits match). For clarity and consistency, consider using the format:
@throws {Ably.ErrorInfo} with code 40400 (status 404)to document both the error code and HTTP status.Apply this pattern across all three locations:
-@throws {Ably.ErrorInfo} with status code 404 if the message does not exist. +@throws {Ably.ErrorInfo} with code 40400 (status 404) if the message does not exist.Also applies to: 143-143, 300-300
src/core/typing.ts (3)
67-95: Clarify “current()” snapshot when not attached.current() returns a snapshot of the cached set; if the room isn’t attached, it’s typically empty. Consider adding a sentence to avoid confusion.
98-131: Tighten serialization wording in keystroke/stop docs.Implementation cancels prior waiters (Mutex.cancel()) before acquiring; earlier operations may short‑circuit. Suggest: “Operations are serialized; the most recent call cancels pending ones and determines final state” instead of “resolve in order.”
Also applies to: 134-176
334-345: Prefer ErrorCodes enum instead of magic numbers.Replace hardcoded 40000/50000 with ErrorCodes enum and ensure status matches code prefix.
As per coding guidelines
Also applies to: 386-391, 400-411
src/react/hooks/use-typing.ts (2)
212-221: Defensive copy to avoid external mutation of Set.Store a cloned Set to decouple hook state from the emitted object.
Apply this diff:
- const { unsubscribe } = room.typing.subscribe((event) => { - setCurrentlyTyping(event.currentlyTyping); - }); + const { unsubscribe } = room.typing.subscribe((event) => { + setCurrentlyTyping(new Set(event.currentlyTyping)); + }); ... - const typing = room.typing.current(); + const typing = room.typing.current(); logger.debug('useTyping(); room attached, getting initial typers', { typing }); - setCurrentlyTyping(typing); + setCurrentlyTyping(new Set(typing));
266-269: Prefer async/await over .then for callbacks.Improves readability and aligns with guidelines.
Apply this diff:
- const keystroke = useCallback(() => context.room.then((room) => room.typing.keystroke()), [context]); - const stop = useCallback(() => context.room.then((room) => room.typing.stop()), [context]); + const keystroke = useCallback(async () => { + const room = await context.room; + return room.typing.keystroke(); + }, [context]); + const stop = useCallback(async () => { + const room = await context.room; + return room.typing.stop(); + }, [context]);src/core/presence.ts (3)
134-169: Align “attached” wording with implementation that allows “attaching”._assertChannelState permits both 'attaching' and 'attached'. Update docs to “attached (or attaching)” or tighten the check to strictly ‘attached’.
Also applies to: 173-205, 209-243, 246-286, 289-328, 331-375
507-511: Use ErrorCodes enum for thrown ErrorInfo.Replace 40000 with ErrorCodes.<…> and keep HTTP status aligned.
As per coding guidelines
590-597: PresenceMember still includes omitted fields at runtime.Spreading member copies id/timestamp/action onto the result despite the Omit type. Build the object without those fields to match the public shape.
Apply this diff:
- private _realtimeMemberToPresenceMember(member: Ably.PresenceMessage): PresenceMember { - return { - // Note that we're casting `extras` from ably-js's `any` to our `JsonObject | undefined`; although ably-js's types don't express it we can assume this type per https://sdk.ably.com/builds/ably/specification/main/features/#TP3i. - ...member, - data: member.data as PresenceData, - updatedAt: new Date(member.timestamp), - }; - } + private _realtimeMemberToPresenceMember(member: Ably.PresenceMessage): PresenceMember { + const { id: _id, action: _action, timestamp, ...rest } = member; + return { + ...rest, + data: member.data as PresenceData, + updatedAt: new Date(member.timestamp), + extras: member.extras as JsonObject | undefined, + }; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 ignored due to path filters (2)
demo/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
package.json(2 hunks)src/core/chat-api.ts(1 hunks)src/core/chat-client.ts(4 hunks)src/core/client-id.ts(1 hunks)src/core/connection.ts(1 hunks)src/core/message-reactions.ts(2 hunks)src/core/message.ts(1 hunks)src/core/messages.ts(2 hunks)src/core/occupancy.ts(1 hunks)src/core/presence.ts(1 hunks)src/core/room-reactions.ts(2 hunks)src/core/room.ts(1 hunks)src/core/rooms.ts(1 hunks)src/core/typing.ts(1 hunks)src/react/contexts/chat-room-context.tsx(1 hunks)src/react/hooks/internal/use-chat-client-context.ts(1 hunks)src/react/hooks/internal/use-room-context.ts(1 hunks)src/react/hooks/internal/use-room-reference-manager.ts(1 hunks)src/react/hooks/use-chat-client.ts(2 hunks)src/react/hooks/use-chat-connection.ts(2 hunks)src/react/hooks/use-messages.ts(3 hunks)src/react/hooks/use-occupancy.ts(3 hunks)src/react/hooks/use-presence-listener.ts(2 hunks)src/react/hooks/use-presence.ts(4 hunks)src/react/hooks/use-room-reactions.ts(3 hunks)src/react/hooks/use-room.ts(3 hunks)src/react/hooks/use-typing.ts(3 hunks)src/react/providers/chat-client-provider.tsx(2 hunks)src/react/providers/chat-room-provider.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/react/hooks/use-chat-connection.ts
- src/core/client-id.ts
- src/react/hooks/use-presence-listener.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- src/react/hooks/internal/use-room-context.ts
- src/react/hooks/use-chat-client.ts
- src/core/chat-api.ts
- src/react/hooks/internal/use-room-reference-manager.ts
- package.json
- src/react/hooks/use-messages.ts
- src/core/occupancy.ts
- src/react/providers/chat-room-provider.tsx
- src/core/messages.ts
- src/react/hooks/use-room-reactions.ts
- src/core/message.ts
- src/react/hooks/use-presence.ts
- src/core/rooms.ts
- src/core/room.ts
- src/react/providers/chat-client-provider.tsx
- src/core/chat-client.ts
- src/react/hooks/use-room.ts
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
src/**/*.{ts,tsx}: Import Ably as: import * as Ably from 'ably'
Include spec point comments in code using // @CHA- (e.g., // @CHA-M10a)
Files:
src/core/typing.tssrc/react/contexts/chat-room-context.tsxsrc/core/room-reactions.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-typing.tssrc/core/connection.tssrc/core/message-reactions.tssrc/core/presence.tssrc/react/hooks/internal/use-chat-client-context.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/core/typing.tssrc/react/contexts/chat-room-context.tsxsrc/core/room-reactions.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-typing.tssrc/core/connection.tssrc/core/message-reactions.tssrc/core/presence.tssrc/react/hooks/internal/use-chat-client-context.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use relative imports within the project; import Ably asimport * as Ably from 'ably'
Prefer named exports over default exports and group related exports
Use PascalCase for interface names; do not prefix with 'I'; document interfaces and properties with JSDoc, including @throws and @return
Use PascalCase for enums and enum members; include JSDoc for enums and members
Use PascalCase for class names; implement interfaces explicitly; avoid inheritance when possible; document classes
Prefix private members with underscore; group members by visibility; document all public members
Document all public methods with JSDoc using @param, @returns, @throws
Use descriptive parameter names; prefer parameter objects for related params; use optional params with defaults when appropriate
Use Ably.ErrorInfo for errors with message, code, and HTTP-derived status code; use ErrorCodes from src/core/errors.ts
Use consistent logging levels (trace, debug, error); include context object; never log Ably channel instances; structure logs clearly
Minimize type assertions; prefer type guards; never use any (use unknown if unavoidable) and document assertions
Prefer async/await over raw promises; handle rejections; document async behavior
For each @[Testable]@ spec point, include the spec annotation (e.g., // @CHA-M10a) in the relevant production code
**/*.{ts,tsx}: Use relative imports within the project
Import Ably asimport * as Ably from 'ably'
Export interfaces and types that are part of the public API
Prefer named exports over default exports
Group related exports together
Use PascalCase for interface names
Use descriptive interface names that reflect purpose
Do not prefix interfaces with 'I'
Use JSDoc comments for interfaces with a clear description
Document all interface properties inline with JSDoc
Document errors with @throws on interfaces when relevant
Document return types with @return in interface-related docs when relevant
Link referenced types in docs using ...
Files:
src/core/typing.tssrc/react/contexts/chat-room-context.tsxsrc/core/room-reactions.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-typing.tssrc/core/connection.tssrc/core/message-reactions.tssrc/core/presence.tssrc/react/hooks/internal/use-chat-client-context.ts
{src,test}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create all TypeScript and TSX files using kebab-case file names
Files:
src/core/typing.tssrc/react/contexts/chat-room-context.tsxsrc/core/room-reactions.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-typing.tssrc/core/connection.tssrc/core/message-reactions.tssrc/core/presence.tssrc/react/hooks/internal/use-chat-client-context.ts
{src,test,react,demo}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{src,test,react,demo}/**/*.{ts,tsx}: Use relative imports within the project
Use PascalCase for classes, interfaces, and enums
Prefix private members with an underscore (e.g., _roomId, _channel)
Avoid any; use unknown if necessary, and prefer strong typing
Use async/await over raw promises
Files:
src/core/typing.tssrc/react/contexts/chat-room-context.tsxsrc/core/room-reactions.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-typing.tssrc/core/connection.tssrc/core/message-reactions.tssrc/core/presence.tssrc/react/hooks/internal/use-chat-client-context.ts
src/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/core/**/*.{ts,tsx}: Error handling must use Ably.ErrorInfo with ErrorCodes enum; construct as new Ably.ErrorInfo(message, code, statusCode)
Log key operations with _logger.trace/_logger.debug/_logger.error using context objects; never log Ably channel instances
Files:
src/core/typing.tssrc/core/room-reactions.tssrc/core/connection.tssrc/core/message-reactions.tssrc/core/presence.ts
src/react/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react-conventions.mdc)
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React custom hooks
Prefix all React hook names withuse(e.g.,useMessages,useLogger)
Use descriptive names for hooks that reflect the hook's purpose
Use JSDoc style comments for hooks
Include a clear description of the hook's purpose in its JSDoc comment
Document all hook parameters and return values using@param,@returns, and@throwsin JSDoc
Group related hook parameters into a single params object
Use TypeScript interfaces to define parameter shapes for hooks
Return objects with named properties for clarity from hooks
Define return type interfaces explicitly for hook return values
Document each returned property of a hook in its JSDoc comment
Memoize callbacks in hooks withuseCallback
Memoize expensive computations in hooks withuseMemo
Use refs for values in hooks that shouldn't trigger re-renders
Always clean up subscriptions and listeners in hooks
Handle component unmounting gracefully in hooks
Clear timers and intervals in hook cleanup functionsReact hooks must follow conventions: useCallback/useMemo, refs for non-reactive values, and proper cleanup
Files:
src/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.tssrc/react/hooks/use-typing.tssrc/react/hooks/internal/use-chat-client-context.ts
src/react/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/react/**/*.{tsx,ts}: React hooks: use camelCase names prefixed with use and descriptive names
Document hooks with JSDoc including description, @param, @returns, and @throws
Group related hook parameters into a params object with TypeScript interfaces
Return objects with named properties; define and document explicit return type interfaces for hooks
Optimize hook performance with useCallback/useMemo and refs for non-reactive values
Always clean up subscriptions/listeners and timers in hooks; handle unmount gracefully
Files:
src/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.tssrc/react/hooks/use-typing.tssrc/react/hooks/internal/use-chat-client-context.ts
🧬 Code graph analysis (6)
src/core/typing.ts (2)
src/core/index.ts (2)
TypingListener(77-77)Subscription(76-76)src/core/subscription.ts (1)
Subscription(12-19)
src/core/room-reactions.ts (1)
src/core/index.ts (1)
SendReactionParams(72-72)
src/react/hooks/use-typing.ts (2)
src/core/typing.ts (2)
TypingListener(183-183)Typing(18-177)src/react/types/chat-status-response.ts (1)
ChatStatusResponse(10-22)
src/core/connection.ts (1)
src/core/index.ts (2)
ConnectionStatus(8-8)ErrorInfo(78-78)
src/core/message-reactions.ts (1)
src/core/chat-api.ts (2)
SendMessageReactionParams(74-91)DeleteMessageReactionParams(96-107)
src/core/presence.ts (2)
src/core/index.ts (3)
RealtimePresenceParams(78-78)PresenceMember(67-67)PresenceData(67-67)src/core/chat-client.ts (1)
clientId(176-178)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test node 18
- GitHub Check: test node 22
🔇 Additional comments (16)
src/react/contexts/chat-room-context.tsx (1)
33-36: LGTM! Clear documentation with appropriate usage guidance.The updated JSDoc effectively clarifies the context's purpose and correctly advises against direct usage in favor of ChatRoomProvider and room-specific hooks—a React best practice. The documentation is concise and aligns with the PR's goal of expanding TypeDoc/JSDoc coverage.
src/react/hooks/internal/use-chat-client-context.ts (1)
11-11: LGTM! Documentation improvement aligns with implementation.The updated
@throws {Ably.ErrorInfo}annotation correctly reflects the error type thrown at line 17 and standardizes the documentation across the codebase.src/react/hooks/use-occupancy.ts (2)
1-2: LGTM: Type-only import properly handled.The Ably import with eslint-disable for unused vars is appropriate since it's used only in JSDoc type annotations (e.g.,
@throws {Ably.ErrorInfo}on line 64). The import pattern aligns with the coding guidelines.
23-30: LGTM: Clear inline example.The inline example effectively demonstrates the listener parameter usage with a concise, practical code snippet.
src/core/room-reactions.ts (2)
18-22: LGTM! Clear and helpful examples.The updated documentation with concrete examples (emojis and strings like "confetti", "applause") is much more helpful than generic placeholders.
66-102: Excellent documentation with proper error semantics.The expanded JSDoc thoroughly describes the ephemeral nature of room reactions, attachment requirements, and error conditions. The example demonstrates the correct usage pattern (attach → send) with proper error handling.
Note: The past review comment requesting
@throwsdocumentation for code 40000 has been correctly addressed on line 79.src/core/connection.ts (5)
81-100: Excellent documentation enhancement with practical example.The added
@returnstag and comprehensive example effectively demonstrate status checking patterns. ThecanAttachToRoom()function provides a realistic use case that addresses the previous feedback about using room attachment scenarios rather than message sending.
104-124: Clear error handling documentation with dual examples.The documentation effectively illustrates both direct error checking and error monitoring through status change callbacks. The use of
reportErrorToMonitoring()as a placeholder is a standard documentation pattern.
128-170: Comprehensive lifecycle documentation with excellent example.The expanded documentation provides clear lifecycle guidance and a detailed example covering connection state handling, error management, and proper cleanup. The use of a switch statement to handle different connection states and the demonstration of subscription cleanup are best practices that will guide developers effectively.
174-184: Proper internal interface design.The
InternalConnectioninterface correctly extendsConnectionand adds thedispose()method marked as@internal, addressing the need to keep cleanup logic internal to the SDK. The documentation clearly explains the method's purpose.
264-270: Clean disposal implementation.The
dispose()method correctly cleans up both the Ably connection listener and all internal emitter listeners, preventing memory leaks. The trace-level logging follows the coding guidelines.src/core/message-reactions.ts (2)
38-76: Documentation improvements look good.The interface documentation clearly describes the optional
countfield for Multiple-type reactions and correctly indicates which fields are optional. The@defaultValueannotation forcountis appropriate.
445-460: Implementation correctly handles default values.The
send()method properly defaults the reaction type to_defaultTypeand setscountto 1 for Multiple-type reactions when not provided, matching the documented behavior.src/core/typing.ts (1)
20-63: Docs look good — attach note and example flow are clear.src/react/hooks/use-typing.ts (2)
101-106: Public API change: new currentlyTyping surface.Public API changes detected. Please update the documentation and the Ably CLI.
169-181: No change needed:ChatRoomProviderexpects thenameprop.
src/core/chat-client.ts
Outdated
| * // Preferred in production: Use auth URL that returns a token with clientId | ||
| * const realtime = new Ably.Realtime({ | ||
| * const realtimeClientWithTokens = new Ably.Realtime({ | ||
| * authUrl: '/api/ably-auth', // Your server endpoint that returns an Ably token with clientId |
There was a problem hiding this comment.
It doesn't have to be an Ably token. It can be JWT.
There was a problem hiding this comment.
Very true, and since we are preferring JWT, I'll remove reference to ably tokens.
src/core/message-reactions.ts
Outdated
| * @param messageSerial - The unique identifier of the message to react to. | ||
| * @param params - The reaction parameters | ||
| * @returns Promise that resolves when the reaction has been sent | ||
| * @throws {Ably.ErrorInfo} with status code 404 if the message does not exist. |
There was a problem hiding this comment.
Is it safe to say that it throws when it doesn't throw but the promise that's returned gets rejected?
There was a problem hiding this comment.
Good point, I'll remove this and mention it only in the return.
| * const messageSerial = '01726585978590-001@abcdefghij:001'; | ||
| * | ||
| * // Send a simple reaction to a message | ||
| * try { |
There was a problem hiding this comment.
do we want one wrapped in try-catch and the others not? The others read easier so maybe it's good to show once how to wrap/handle the error.
There was a problem hiding this comment.
I have no strong opinions either way tbh, I'm assuming a dev/LLM using this will understand it has to try/catch async functions, so perhaps one example is enough? Or we could just skip the try/catch and assume those using the SDK will know to use them (it's a pretty basic flow after all)..
| * | ||
| * **Note**: | ||
| * - The room must be attached to receive reaction events. | ||
| * - When there are many reacting clients, the client list may be clipped. Check the `clipped` flag and use {@link clientReactions} for complete client information when needed. |
There was a problem hiding this comment.
Another note that they might be rolled up on high throughput?
There was a problem hiding this comment.
How about
- When the rate of reactions is very high, multiple summaries may be rolled up into a single summary event, meaning the delta between sequential summaries is not guaranteed to be a single reaction change.
| * await room.attach(); | ||
| * | ||
| * // Clean up when done | ||
| * subscription.unsubscribe(); |
There was a problem hiding this comment.
I'd prefer not to include this here at calling detach has a wide impact that would break other ongoing chat operations/subscriptions. Perhaps the comment is misleading, would it be better if it just said -
//Later, unsubscribe when done
Wdyt?
src/core/messages.ts
Outdated
| * // Subscribe a listener to message events | ||
| * const subscription = room.messages.subscribe((event) => { | ||
| * console.log(`Message ${event.type}:`, event.message.text); | ||
| * switch (event.type) { |
There was a problem hiding this comment.
do we need the switch here or can we just remove it and call updateLocalMessageState straight away?
There was a problem hiding this comment.
It was more to highlight the different types clearly, so an LLM (or human) could see at a glance the types to handle, but also because until recently we had more than just those three types.
| * console.log('From:', event.message.clientId); | ||
| * console.log('At:', event.message.timestamp); | ||
| * | ||
| * // Handle different event types |
There was a problem hiding this comment.
we print event.type. is that enough for the example without this switch as well?
There was a problem hiding this comment.
No strong feelings here, how much context do with think an LLM/human might need? I'll remove for now, perhaps we can run the test bed against this function specifically later and see how it does
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (1)
src/core/chat-client.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Import Ably as: import * as Ably from 'ably'
Files:
src/core/chat-client.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/core/chat-client.ts
{src,test}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,test}/**/*.{ts,tsx}: Annotate source code with feature specification point IDs (e.g., // @CHA-M10a) where relevant
Create all TypeScript and TSX files using kebab-case filenames
{src,test}/**/*.{ts,tsx}: Use relative imports within the project
Use async/await instead of raw promises
Files:
src/core/chat-client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use relative imports within the project
Import Ably as: import * as Ably from 'ably'
Prefer named exports over default exports; group related exports together
Interfaces: use PascalCase names; descriptive; no mandatory 'I' prefix; document with JSDoc including property docs, @throws, @return, and {@link} links
Enums: use PascalCase for enum and members; include JSDoc for enum and each member
Classes: use PascalCase; implement interfaces explicitly; avoid inheritance where possible; document class purpose
Class members: prefix private members with underscore; group by visibility; document all public members
Public methods must have JSDoc with @param, @returns, @throws
Prefer parameter objects for related parameters; use descriptive names and optional parameters with defaults
Use Ably.ErrorInfo for errors; message as first arg, error code second, status code third; use ErrorCodes enum from src/core/errors.ts
Error message format must be: 'unable to ; ' (use semicolon; start with 'unable to')
Logging: use consistent levels (trace, debug, error), include context object; never log Ably channel instances; structure messages for parsing
Minimize type assertions; prefer type guards; document any assertions; never use any (use unknown if unavoidable)
Use async/await over raw promises; handle rejections; document async behavior
Reference feature-specification.mdc for mapping spec points in code where applicable
**/*.{ts,tsx}: Use PascalCase for classes, interfaces, and enums
Prefix private members with an underscore (e.g., _roomId, _channel)
Avoid any; prefer unknown and strong typing
**/*.{ts,tsx}: Use relative imports within the project
When importing the package ably, useimport * as Ably from 'ably'
Prefer named exports over default exports
Group related exports together
Use PascalCase for interface names
Use descriptive interface names that reflect purpose
Do not prefix interfaces with 'I'
Use JSDoc comments for interfaces with...
Files:
src/core/chat-client.ts
src/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/core/**/*.{ts,tsx}: Create errors using Ably's ErrorInfo: new Ably.ErrorInfo(message, code, statusCode)
Use ErrorCodes enum from src/core/errors.ts for all error codes when creating ErrorInfo
All error messages must match: "unable to ; " (start with "unable to", lowercase operation, semicolon separator)
Use _logger.trace(), _logger.debug(), and _logger.error() with context objects for key operations
Never log Ably channel instances
Include spec point comments in code where applicable (// @CHA-)
Files:
src/core/chat-client.ts
{src,test,demo}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use kebab-case for TypeScript and TSX filenames
Files:
src/core/chat-client.ts
src/core/**
📄 CodeRabbit inference engine (CLAUDE.md)
Core SDK source files live under src/core/
Files:
src/core/chat-client.ts
🧬 Code graph analysis (1)
src/core/chat-client.ts (1)
src/core/rooms.ts (1)
Rooms(15-115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Cursor Bugbot
- GitHub Check: Cursor Bugbot
- GitHub Check: test node 22
- GitHub Check: test node 24
- GitHub Check: test node 20
- GitHub Check: test node 18
🔇 Additional comments (6)
src/core/chat-client.ts (6)
124-142: LGTM!The documentation clearly explains the rooms accessor and provides helpful examples covering default options, custom options, and room release.
147-163: LGTM!The connection accessor documentation is clear and the example demonstrates appropriate connection monitoring patterns.
180-189: LGTM!The realtime accessor documentation appropriately warns users about the risks of direct Realtime client access while acknowledging legitimate advanced use cases.
194-197: LGTM!The clientOptions getter documentation is clear and appropriately concise for a simple configuration accessor.
233-264: LGTM!The dispose method documentation is thorough, with clear lifecycle guidance and a comprehensive example demonstrating proper cleanup with error handling.
176-178: Public API changes detected. Please update the documentation and the Ably CLI.The new
clientIdgetter exposes client identification as part of the public API.
splindsay-92
force-pushed
the
typedocs/ensure-full-function-context-is-covered
branch
from
2674f45 to
5c6b660
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/message-reactions.ts (2)
476-476: Fix error message format to comply with coding guidelines.The error message doesn't follow the required format: "unable to ; ".
As per coding guidelines
Apply this diff:
- throw new Ably.ErrorInfo(`cannot delete reaction of type ${type} without a name`, 40001, 400); + throw new Ably.ErrorInfo(`unable to delete reaction; name is required for type ${type}`, 40001, 400);
507-507: Fix error message format to comply with coding guidelines.The error message doesn't follow the required format: "unable to ; ".
As per coding guidelines
Apply this diff:
- throw new Ably.ErrorInfo('Raw message reactions are not enabled', 40001, 400); + throw new Ably.ErrorInfo('unable to subscribe to raw reactions; raw message reactions are not enabled', 40001, 400);
🧹 Nitpick comments (1)
src/core/typing.ts (1)
67-94: Clarify attachment requirement in the main description.While the example demonstrates attaching to the room (line 83), the main description (lines 67-71) should explicitly state that the room must be attached to get accurate typing state, similar to how other methods document their prerequisites.
Consider this enhancement to line 67-71:
/** * Gets the current set of users who are typing. * * Returns a Set containing the client IDs of all users currently typing in the room. - * This provides a snapshot of the typing state at the time of the call. + * This provides a snapshot of the typing state at the time of the call. The room must be + * attached for this data to be populated and accurate. * @returns Set of client IDs currently typing
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (6)
src/core/message-reactions.ts(2 hunks)src/core/messages.ts(2 hunks)src/core/occupancy.ts(1 hunks)src/core/presence.ts(1 hunks)src/core/room-reactions.ts(2 hunks)src/core/typing.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/core/room-reactions.ts
- src/core/messages.ts
- src/core/occupancy.ts
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Import Ably as: import * as Ably from 'ably'
Files:
src/core/message-reactions.tssrc/core/typing.tssrc/core/presence.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/core/message-reactions.tssrc/core/typing.tssrc/core/presence.ts
{src,test}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,test}/**/*.{ts,tsx}: Annotate source code with feature specification point IDs (e.g., // @CHA-M10a) where relevant
Create all TypeScript and TSX files using kebab-case filenames
{src,test}/**/*.{ts,tsx}: Use relative imports within the project
Use async/await instead of raw promises
Files:
src/core/message-reactions.tssrc/core/typing.tssrc/core/presence.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use relative imports within the project
Import Ably as: import * as Ably from 'ably'
Prefer named exports over default exports; group related exports together
Interfaces: use PascalCase names; descriptive; no mandatory 'I' prefix; document with JSDoc including property docs, @throws, @return, and {@link} links
Enums: use PascalCase for enum and members; include JSDoc for enum and each member
Classes: use PascalCase; implement interfaces explicitly; avoid inheritance where possible; document class purpose
Class members: prefix private members with underscore; group by visibility; document all public members
Public methods must have JSDoc with @param, @returns, @throws
Prefer parameter objects for related parameters; use descriptive names and optional parameters with defaults
Use Ably.ErrorInfo for errors; message as first arg, error code second, status code third; use ErrorCodes enum from src/core/errors.ts
Error message format must be: 'unable to ; ' (use semicolon; start with 'unable to')
Logging: use consistent levels (trace, debug, error), include context object; never log Ably channel instances; structure messages for parsing
Minimize type assertions; prefer type guards; document any assertions; never use any (use unknown if unavoidable)
Use async/await over raw promises; handle rejections; document async behavior
Reference feature-specification.mdc for mapping spec points in code where applicable
**/*.{ts,tsx}: Use PascalCase for classes, interfaces, and enums
Prefix private members with an underscore (e.g., _roomId, _channel)
Avoid any; prefer unknown and strong typing
**/*.{ts,tsx}: Use relative imports within the project
When importing the package ably, useimport * as Ably from 'ably'
Prefer named exports over default exports
Group related exports together
Use PascalCase for interface names
Use descriptive interface names that reflect purpose
Do not prefix interfaces with 'I'
Use JSDoc comments for interfaces with...
Files:
src/core/message-reactions.tssrc/core/typing.tssrc/core/presence.ts
src/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/core/**/*.{ts,tsx}: Create errors using Ably's ErrorInfo: new Ably.ErrorInfo(message, code, statusCode)
Use ErrorCodes enum from src/core/errors.ts for all error codes when creating ErrorInfo
All error messages must match: "unable to ; " (start with "unable to", lowercase operation, semicolon separator)
Use _logger.trace(), _logger.debug(), and _logger.error() with context objects for key operations
Never log Ably channel instances
Include spec point comments in code where applicable (// @CHA-)
Files:
src/core/message-reactions.tssrc/core/typing.tssrc/core/presence.ts
{src,test,demo}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use kebab-case for TypeScript and TSX filenames
Files:
src/core/message-reactions.tssrc/core/typing.tssrc/core/presence.ts
src/core/**
📄 CodeRabbit inference engine (CLAUDE.md)
Core SDK source files live under src/core/
Files:
src/core/message-reactions.tssrc/core/typing.tssrc/core/presence.ts
🧬 Code graph analysis (3)
src/core/message-reactions.ts (1)
src/core/chat-api.ts (2)
SendMessageReactionParams(74-91)DeleteMessageReactionParams(96-107)
src/core/typing.ts (2)
src/core/index.ts (2)
TypingListener(77-77)Subscription(76-76)src/core/subscription.ts (1)
Subscription(12-19)
src/core/presence.ts (2)
src/core/index.ts (3)
RealtimePresenceParams(78-78)PresenceMember(67-67)PresenceData(67-67)src/core/chat-client.ts (1)
clientId(176-178)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test node 20
- GitHub Check: test node 18
🔇 Additional comments (14)
src/core/message-reactions.ts (6)
41-59: LGTM!The interface documentation is clear and comprehensive. The examples for
name(emoji and custom names) and the clarification thattypeuses the room default when not set are helpful additions.
83-128: LGTM!The method documentation is comprehensive with excellent examples covering different reaction types. The REST API note and error semantics are accurate.
132-180: LGTM!The deletion documentation clearly explains the different behaviors for each reaction type with helpful examples. The error cases are properly documented.
184-233: LGTM!The subscription documentation is thorough, with excellent notes about room attachment requirements, client list clipping, and summary rollup behavior. The example clearly demonstrates handling all three reaction types.
237-287: LGTM!The raw subscription documentation is comprehensive, clearly explaining the requirement for
rawMessageReactionsto be enabled. The example demonstrates proper event handling with a switch statement.
291-337: LGTM!The clientReactions documentation is thorough, with a clear explanation of when to use this method (clipped summaries). The examples demonstrate both current client and specific client use cases effectively.
src/core/typing.ts (3)
20-63: LGTM! Comprehensive subscribe documentation.The expanded documentation effectively describes the real-time typing event flow and includes a practical example showing how to handle different typing indicator states. The attachment requirement is clearly stated.
98-130: LGTM! Comprehensive keystroke documentation.The documentation thoroughly covers throttling behavior, mutex serialization, connection/attachment prerequisites, and error conditions with a clear usage example.
134-175: LGTM! Comprehensive stop documentation.The documentation thoroughly covers the serialization behavior, no-op handling, connection/attachment prerequisites, and includes a practical example demonstrating the complete typing lifecycle.
src/core/presence.ts (5)
134-169: LGTM! Comprehensive get method documentation.The expanded documentation clearly describes the method's behavior with explicit attachment requirements, proper error handling, and practical examples showing both retrieving all members and filtering by client ID.
173-205: LGTM! Comprehensive isUserPresent documentation.The documentation effectively describes the method's purpose as a convenient boolean check and includes clear prerequisites, error handling, and a practical usage example.
209-242: LGTM! Comprehensive enter method documentation.The documentation thoroughly describes the enter semantics, including the behavior when already present (emits update events), attachment requirements, and includes a practical example with user metadata.
246-285: LGTM! Comprehensive update method documentation.The documentation effectively describes the PUT-like replacement semantics (not merge), attachment requirements, and includes an excellent example demonstrating how the entire data object is replaced rather than merged.
289-327: LGTM! Comprehensive leave method documentation.The documentation thoroughly describes the leave behavior including the no-op case, attachment requirements, and includes a practical example showing the complete presence lifecycle from enter to leave.
splindsay-92
force-pushed
the
typedocs/ensure-full-function-context-is-covered
branch
from
32cee17 to
1b9f1b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/core/messages.ts (5)
384-384: Consider linking the Serial type in parameter documentation.Per coding guidelines and a previous review comment, the
serialparameter description could reference the type using{@link Serial}to improve documentation clarity.Apply this diff:
- * @param serial - The unique serial identifier of the message to retrieve. + * @param serial - The unique {@link Serial} identifier of the message to retrieve.
429-429: Consider linking the parameter type in documentation.Per coding guidelines and a previous review comment (marked as addressed), the
paramsparameter description could reference the type using{@link SendMessageParams}for better documentation navigation.Apply this diff:
- * @param params - Message parameters containing the text and optional metadata/headers + * @param params - {@link SendMessageParams} containing the text and optional metadata/headers
482-483: Consider linking parameter types in documentation.Per coding guidelines and previous review comments (marked as addressed), the parameter descriptions could reference their types using
{@link}tags for better documentation navigation.Apply this diff:
- * @param serial - The unique identifier of the message to delete. - * @param details - Optional details to record about the delete action. + * @param serial - The unique {@link Serial} identifier of the message to delete (i.e. the {@link Message.serial} field). + * @param details - Optional {@link OperationDetails} to record about the delete action.
542-544: Consider linking parameter types in documentation.Per coding guidelines and previous review comments (marked as addressed), the parameter descriptions could reference their types using
{@link}tags for better documentation navigation.Apply this diff:
- * @param serial - The unique identifier of the message to update. - * @param updateParams - The new message content and properties. - * @param details - Optional details to record about the delete action. + * @param serial - The unique {@link Serial} identifier of the message to update. + * @param updateParams - The new {@link UpdateMessageParams} content and properties. + * @param details - Optional {@link OperationDetails} to record about the update action.Note: Line 544's description mentions "delete" but should say "update".
590-596: Consider linking the MessageReactions type for consistency.For improved documentation navigation and consistency with other property docs, consider linking the
MessageReactionstype.Apply this diff:
* Send, delete, and subscribe to message reactions. * - * This property provides access to the message reactions functionality, allowing you to + * This property provides access to the {@link MessageReactions} functionality, allowing you to * add reactions to specific messages, remove reactions, and subscribe to reaction events * in real-time.src/react/providers/chat-client-provider.tsx (1)
1-2: Consider type-only import for documentation-only Ably reference.The Ably import is only used in JSDoc comments for type references. Consider using a type-only import to make this more explicit:
-// eslint-disable-next-line @typescript-eslint/no-unused-vars -import * as Ably from 'ably'; +import type * as Ably from 'ably';This would eliminate the need for the eslint suppression while still providing TypeDoc with the necessary type information.
Note: If TypeDoc doesn't properly resolve
typeimports in JSDoc, the current approach is acceptable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 ignored due to path filters (2)
demo/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
package.json(1 hunks)src/core/chat-api.ts(1 hunks)src/core/chat-client.ts(4 hunks)src/core/client-id.ts(1 hunks)src/core/connection.ts(1 hunks)src/core/message-reactions.ts(2 hunks)src/core/message.ts(1 hunks)src/core/messages.ts(2 hunks)src/core/occupancy.ts(1 hunks)src/core/presence.ts(1 hunks)src/core/room-reactions.ts(2 hunks)src/core/room.ts(1 hunks)src/core/rooms.ts(1 hunks)src/core/typing.ts(1 hunks)src/react/contexts/chat-room-context.tsx(1 hunks)src/react/hooks/internal/use-chat-client-context.ts(1 hunks)src/react/hooks/internal/use-room-context.ts(1 hunks)src/react/hooks/internal/use-room-reference-manager.ts(1 hunks)src/react/hooks/use-chat-client.ts(2 hunks)src/react/hooks/use-chat-connection.ts(2 hunks)src/react/hooks/use-messages.ts(3 hunks)src/react/hooks/use-occupancy.ts(3 hunks)src/react/hooks/use-presence-listener.ts(2 hunks)src/react/hooks/use-presence.ts(4 hunks)src/react/hooks/use-room-reactions.ts(3 hunks)src/react/hooks/use-room.ts(3 hunks)src/react/hooks/use-typing.ts(3 hunks)src/react/providers/chat-client-provider.tsx(2 hunks)src/react/providers/chat-room-provider.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/react/hooks/use-room-reactions.ts
- src/react/hooks/use-presence-listener.ts
- src/react/hooks/use-messages.ts
- src/react/hooks/use-chat-connection.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- src/react/providers/chat-room-provider.tsx
- src/react/hooks/internal/use-room-context.ts
- src/react/hooks/use-chat-client.ts
- src/core/occupancy.ts
- src/core/typing.ts
- src/core/client-id.ts
- src/core/message.ts
- src/core/connection.ts
- src/core/room-reactions.ts
- src/core/room.ts
- src/react/hooks/use-room.ts
- src/react/hooks/use-typing.ts
- src/core/message-reactions.ts
- src/core/chat-api.ts
- src/react/hooks/internal/use-chat-client-context.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Import Ably as: import * as Ably from 'ably'
Files:
src/core/chat-client.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/core/presence.tssrc/core/rooms.tssrc/core/messages.tssrc/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/core/chat-client.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/core/presence.tssrc/core/rooms.tssrc/core/messages.tssrc/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.ts
{src,test}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,test}/**/*.{ts,tsx}: Annotate source code with feature specification point IDs (e.g., // @CHA-M10a) where relevant
Create all TypeScript and TSX files using kebab-case filenames
{src,test}/**/*.{ts,tsx}: Use relative imports within the project
Use async/await instead of raw promises
Files:
src/core/chat-client.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/core/presence.tssrc/core/rooms.tssrc/core/messages.tssrc/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use relative imports within the project
Import Ably as: import * as Ably from 'ably'
Prefer named exports over default exports; group related exports together
Interfaces: use PascalCase names; descriptive; no mandatory 'I' prefix; document with JSDoc including property docs, @throws, @return, and {@link} links
Enums: use PascalCase for enum and members; include JSDoc for enum and each member
Classes: use PascalCase; implement interfaces explicitly; avoid inheritance where possible; document class purpose
Class members: prefix private members with underscore; group by visibility; document all public members
Public methods must have JSDoc with @param, @returns, @throws
Prefer parameter objects for related parameters; use descriptive names and optional parameters with defaults
Use Ably.ErrorInfo for errors; message as first arg, error code second, status code third; use ErrorCodes enum from src/core/errors.ts
Error message format must be: 'unable to ; ' (use semicolon; start with 'unable to')
Logging: use consistent levels (trace, debug, error), include context object; never log Ably channel instances; structure messages for parsing
Minimize type assertions; prefer type guards; document any assertions; never use any (use unknown if unavoidable)
Use async/await over raw promises; handle rejections; document async behavior
Reference feature-specification.mdc for mapping spec points in code where applicable
**/*.{ts,tsx}: Use PascalCase for classes, interfaces, and enums
Prefix private members with an underscore (e.g., _roomId, _channel)
Avoid any; prefer unknown and strong typing
**/*.{ts,tsx}: Use relative imports within the project
When importing the package ably, useimport * as Ably from 'ably'
Prefer named exports over default exports
Group related exports together
Use PascalCase for interface names
Use descriptive interface names that reflect purpose
Do not prefix interfaces with 'I'
Use JSDoc comments for interfaces with...
Files:
src/core/chat-client.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/core/presence.tssrc/core/rooms.tssrc/core/messages.tssrc/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.ts
src/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/core/**/*.{ts,tsx}: Create errors using Ably's ErrorInfo: new Ably.ErrorInfo(message, code, statusCode)
Use ErrorCodes enum from src/core/errors.ts for all error codes when creating ErrorInfo
All error messages must match: "unable to ; " (start with "unable to", lowercase operation, semicolon separator)
Use _logger.trace(), _logger.debug(), and _logger.error() with context objects for key operations
Never log Ably channel instances
Include spec point comments in code where applicable (// @CHA-)
Files:
src/core/chat-client.tssrc/core/presence.tssrc/core/rooms.tssrc/core/messages.ts
{src,test,demo}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use kebab-case for TypeScript and TSX filenames
Files:
src/core/chat-client.tssrc/react/hooks/internal/use-room-reference-manager.tssrc/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/core/presence.tssrc/core/rooms.tssrc/core/messages.tssrc/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.ts
src/core/**
📄 CodeRabbit inference engine (CLAUDE.md)
Core SDK source files live under src/core/
Files:
src/core/chat-client.tssrc/core/presence.tssrc/core/rooms.tssrc/core/messages.ts
src/react/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react-conventions.mdc)
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React custom hooks
Prefix all React hook names withuse(e.g.,useMessages,useLogger)
Use descriptive names for hooks that reflect the hook's purpose
Use JSDoc style comments for hooks
Include a clear description of the hook's purpose in its JSDoc comment
Document all hook parameters and return values using@param,@returns, and@throwsin JSDoc
Group related hook parameters into a single params object
Use TypeScript interfaces to define parameter shapes for hooks
Return objects with named properties for clarity from hooks
Define return type interfaces explicitly for hook return values
Document each returned property of a hook in its JSDoc comment
Memoize callbacks in hooks withuseCallback
Memoize expensive computations in hooks withuseMemo
Use refs for values in hooks that shouldn't trigger re-renders
Always clean up subscriptions and listeners in hooks
Handle component unmounting gracefully in hooks
Clear timers and intervals in hook cleanup functions
src/react/**/*.{ts,tsx}: React hooks: use camelCase names prefixed with 'use' and descriptive hook names
Document hooks with JSDoc including description, @param, @returns, @throws
Group related hook parameters into a single params object typed with a TypeScript interface
Return objects with named properties from hooks; define and export explicit return type interfaces; document returned properties
Optimize performance: memoize callbacks with useCallback, computations with useMemo, and use refs to avoid unnecessary re-renders
Always clean up subscriptions/listeners and clear timers/intervals in effect cleanupsReact hooks must follow React conventions: useCallback/useMemo for stable refs, refs for non-reactive values, and proper cleanup
Files:
src/react/hooks/internal/use-room-reference-manager.tssrc/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.ts
src/react/**
📄 CodeRabbit inference engine (CLAUDE.md)
React SDK source files live under src/react/
Files:
src/react/hooks/internal/use-room-reference-manager.tssrc/react/providers/chat-client-provider.tsxsrc/react/hooks/use-presence.tssrc/react/contexts/chat-room-context.tsxsrc/react/hooks/use-occupancy.ts
🧬 Code graph analysis (4)
src/core/chat-client.ts (1)
src/core/rooms.ts (1)
Rooms(15-115)
src/core/presence.ts (2)
src/core/index.ts (3)
RealtimePresenceParams(78-78)PresenceMember(67-67)PresenceData(67-67)src/core/chat-client.ts (1)
clientId(176-178)
src/core/rooms.ts (2)
src/core/room.ts (3)
name(484-486)options(491-493)Room(23-367)src/core/room-options.ts (1)
RoomOptions(118-138)
src/core/messages.ts (2)
src/core/query.ts (1)
PaginatedResult(4-39)src/core/message.ts (1)
Message(64-147)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test node 22
- GitHub Check: test node 24
- GitHub Check: test node 20
🔇 Additional comments (15)
package.json (1)
125-125: LGTM—TypeDoc version bump supports expanded documentation.The upgrade from ^0.28.4 to ^0.28.13 is appropriate for this documentation-focused PR.
src/core/messages.ts (3)
176-275: Excellent documentation with comprehensive examples.The expanded JSDoc for
historyBeforeSubscribeprovides clear guidance on initial subscription and discontinuity handling. The examples correctly demonstrate state management with proper variable declarations.
285-319: Clear and well-structured subscription documentation.The JSDoc provides a straightforward example of subscribing to message events with proper lifecycle management.
322-374: Comprehensive pagination example with correct REST API note.The documentation correctly states that REST API methods do not require room attachment, and the pagination loop demonstrates proper usage patterns.
src/react/hooks/internal/use-room-reference-manager.ts (1)
21-21: LGTM! Improved error type documentation.The change from
ErrorInfotoAbly.ErrorInfomakes the error type more explicit and aligns with the coding guidelines requiring fully qualified Ably types.src/core/chat-client.ts (7)
52-107: Excellent constructor documentation with comprehensive examples.The three examples effectively demonstrate the main initialization patterns (JWT auth, API key auth, and custom logging). Each example is now self-contained and properly addresses the variable naming issues raised in previous reviews.
124-141: Clear and comprehensive rooms accessor documentation.The expanded JSDoc effectively documents the rooms management interface with practical examples showing acquisition, configuration, and release of rooms.
148-162: Well-documented connection accessor.The JSDoc clearly explains how to monitor connection status and handle status changes with practical examples.
176-178: Public API changes detected. Please update the documentation and the Ably CLI.The new
clientIdgetter exposes the client's identity publicly. The implementation correctly delegates tothis._realtime.auth.clientId, and the JSDoc appropriately explains the timing considerations for token-based versus key-based authentication.As per coding guidelines.
181-189: Good documentation with appropriate warnings.The expanded JSDoc for the
realtimeaccessor appropriately warns about potential issues with direct Ably client manipulation while still providing access for advanced scenarios.
195-196: Clear documentation of resolved options.The updated JSDoc appropriately clarifies that the returned options include defaults merged with provided options.
234-263: Comprehensive dispose documentation with lifecycle guidance.The expanded JSDoc effectively explains the disposal lifecycle, resource cleanup, non-reusability after disposal, and provides a clear example of the complete flow.
src/react/providers/chat-client-provider.tsx (2)
36-75: Excellent provider documentation with clear usage guidance.The expanded JSDoc effectively documents the provider's responsibilities (room reference counting, agent registration) and provides important guidance about memoization and provider requirements. The example clearly demonstrates proper usage patterns.
82-99: Internal integration logic correctly wires room reference management and agent registration.The implementation properly:
- Enables React-specific agent tracking via
addReactAgent()- Creates/reuses
RoomReferenceManagerbased on client identity- Conditionally registers UI kit agent when version is available
The type assertions are appropriate for accessing internal APIs (
addReactAgent,logger,addAgentWithVersion) that are marked@internalin theChatClientclass but are part of the internal integration contract between the core SDK and React layer.src/react/hooks/use-occupancy.ts (1)
1-2: Avoid importing Ably purely for docs.Pulling in
import * as Ably from 'ably'just to satisfy doc references causes the entire Ably runtime to be bundled into this hook, even though the code never uses it. That’s a big hit for our React build. Please drop the runtime import and, if you need type information, switch to a type-only import that’s actually consumed (e.g. re-export an alias used in the code) so tree-shaking can remove it.⛔ Skipped due to learnings
Learnt from: CR Repo: ably/ably-chat-js PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-10-15T17:15:55.238Z Learning: Applies to **/*.{ts,tsx} : Import Ably as: import * as Ably from 'ably'Learnt from: CR Repo: ably/ably-chat-js PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-10-15T17:17:26.484Z Learning: Applies to src/**/*.{ts,tsx} : Import Ably as: import * as Ably from 'ably'
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/core/presence.ts (1)
332-376: Confirm downstream updates for API change. Public API changes detected. Please update the documentation and the Ably CLI. As per coding guidelines.
🧹 Nitpick comments (2)
src/react/hooks/use-room-reactions.ts (1)
201-201: Verify type casting consistency with interface declarationThe return type casts
sendRoomReactiontoRoomReactions['send'], but the interface at line 73 declares an explicit signature(params: SendReactionParams) => Promise<void>. The AI summary indicates the cast should use the "new public signature" instead ofRoomReactions['send']directly.This inconsistency may cause type mismatches or confusion about the intended public API surface. Consider either:
- Removing the cast if TypeScript correctly infers the type from the interface
- Casting to the explicit signature declared in the interface if needed for type compatibility
- sendRoomReaction: sendRoomReaction as RoomReactions['send'], + sendRoomReaction,or
- sendRoomReaction: sendRoomReaction as RoomReactions['send'], + sendRoomReaction: sendRoomReaction as (params: SendReactionParams) => Promise<void>,src/core/rooms.ts (1)
57-57: Consider usingErrorCodeenum in the example for consistency.The example uses a hard-coded error code (
40000) instead of referencing theErrorCode.RoomExistsWithDifferentOptionsenum value. For consistency with the coding guidelines and to make the example more maintainable, consider updating the example to use the enum.Example:
+ * import { ChatClient, Room, ErrorCode } from '@ably/chat'; * - * if (error.code === 40000) { + * if (error.code === ErrorCode.RoomExistsWithDifferentOptions) { * console.error('Room already exists with different options'); * }
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (17)
src/core/message-reactions.ts(2 hunks)src/core/messages.ts(2 hunks)src/core/occupancy.ts(1 hunks)src/core/presence.ts(1 hunks)src/core/room-reactions.ts(2 hunks)src/core/room.ts(1 hunks)src/core/rooms.ts(1 hunks)src/core/typing.ts(1 hunks)src/react/helper/room-promise.ts(1 hunks)src/react/hooks/use-messages.ts(6 hunks)src/react/hooks/use-occupancy.ts(3 hunks)src/react/hooks/use-presence-listener.ts(2 hunks)src/react/hooks/use-presence.ts(5 hunks)src/react/hooks/use-room-reactions.ts(4 hunks)src/react/hooks/use-room.ts(3 hunks)src/react/hooks/use-typing.ts(3 hunks)src/react/providers/chat-room-provider.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/react/helper/room-promise.ts
- src/core/occupancy.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/react/hooks/use-presence-listener.ts
- src/core/room-reactions.ts
- src/core/room.ts
- src/core/messages.ts
- src/core/message-reactions.ts
- src/react/providers/chat-room-provider.tsx
- src/react/hooks/use-room.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Import Ably as: import * as Ably from 'ably'
Files:
src/core/typing.tssrc/core/presence.tssrc/core/rooms.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-messages.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-presence.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/core/typing.tssrc/core/presence.tssrc/core/rooms.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-messages.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-presence.ts
{src,test}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,test}/**/*.{ts,tsx}: Annotate source code with feature specification point IDs (e.g., // @CHA-M10a) where relevant
Create all TypeScript and TSX files using kebab-case filenames
{src,test}/**/*.{ts,tsx}: Use relative imports within the project
Use async/await instead of raw promises
Files:
src/core/typing.tssrc/core/presence.tssrc/core/rooms.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-messages.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-presence.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use relative imports within the project
Import Ably as: import * as Ably from 'ably'
Prefer named exports over default exports; group related exports together
Interfaces: use PascalCase names; descriptive; no mandatory 'I' prefix; document with JSDoc including property docs, @throws, @return, and {@link} links
Enums: use PascalCase for enum and members; include JSDoc for enum and each member
Classes: use PascalCase; implement interfaces explicitly; avoid inheritance where possible; document class purpose
Class members: prefix private members with underscore; group by visibility; document all public members
Public methods must have JSDoc with @param, @returns, @throws
Prefer parameter objects for related parameters; use descriptive names and optional parameters with defaults
Use Ably.ErrorInfo for errors; message as first arg, error code second, status code third; use ErrorCodes enum from src/core/errors.ts
Error message format must be: 'unable to ; ' (use semicolon; start with 'unable to')
Logging: use consistent levels (trace, debug, error), include context object; never log Ably channel instances; structure messages for parsing
Minimize type assertions; prefer type guards; document any assertions; never use any (use unknown if unavoidable)
Use async/await over raw promises; handle rejections; document async behavior
Reference feature-specification.mdc for mapping spec points in code where applicable
**/*.{ts,tsx}: Use PascalCase for classes, interfaces, and enums
Prefix private members with an underscore (e.g., _roomId, _channel)
Avoid any; prefer unknown and strong typing
**/*.{ts,tsx}: Use relative imports within the project
When importing the package ably, useimport * as Ably from 'ably'
Prefer named exports over default exports
Group related exports together
Use PascalCase for interface names
Use descriptive interface names that reflect purpose
Do not prefix interfaces with 'I'
Use JSDoc comments for interfaces with...
Files:
src/core/typing.tssrc/core/presence.tssrc/core/rooms.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-messages.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-presence.ts
src/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/core/**/*.{ts,tsx}: Create errors using Ably's ErrorInfo: new Ably.ErrorInfo(message, code, statusCode)
Use ErrorCodes enum from src/core/errors.ts for all error codes when creating ErrorInfo
All error messages must match: "unable to ; " (start with "unable to", lowercase operation, semicolon separator)
Use _logger.trace(), _logger.debug(), and _logger.error() with context objects for key operations
Never log Ably channel instances
Include spec point comments in code where applicable (// @CHA-)
Files:
src/core/typing.tssrc/core/presence.tssrc/core/rooms.ts
{src,test,demo}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use kebab-case for TypeScript and TSX filenames
Files:
src/core/typing.tssrc/core/presence.tssrc/core/rooms.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-messages.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-presence.ts
src/core/**
📄 CodeRabbit inference engine (CLAUDE.md)
Core SDK source files live under src/core/
Files:
src/core/typing.tssrc/core/presence.tssrc/core/rooms.ts
src/react/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react-conventions.mdc)
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React custom hooks
Prefix all React hook names withuse(e.g.,useMessages,useLogger)
Use descriptive names for hooks that reflect the hook's purpose
Use JSDoc style comments for hooks
Include a clear description of the hook's purpose in its JSDoc comment
Document all hook parameters and return values using@param,@returns, and@throwsin JSDoc
Group related hook parameters into a single params object
Use TypeScript interfaces to define parameter shapes for hooks
Return objects with named properties for clarity from hooks
Define return type interfaces explicitly for hook return values
Document each returned property of a hook in its JSDoc comment
Memoize callbacks in hooks withuseCallback
Memoize expensive computations in hooks withuseMemo
Use refs for values in hooks that shouldn't trigger re-renders
Always clean up subscriptions and listeners in hooks
Handle component unmounting gracefully in hooks
Clear timers and intervals in hook cleanup functions
src/react/**/*.{ts,tsx}: React hooks: use camelCase names prefixed with 'use' and descriptive hook names
Document hooks with JSDoc including description, @param, @returns, @throws
Group related hook parameters into a single params object typed with a TypeScript interface
Return objects with named properties from hooks; define and export explicit return type interfaces; document returned properties
Optimize performance: memoize callbacks with useCallback, computations with useMemo, and use refs to avoid unnecessary re-renders
Always clean up subscriptions/listeners and clear timers/intervals in effect cleanupsReact hooks must follow React conventions: useCallback/useMemo for stable refs, refs for non-reactive values, and proper cleanup
Files:
src/react/hooks/use-room-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-messages.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-presence.ts
src/react/**
📄 CodeRabbit inference engine (CLAUDE.md)
React SDK source files live under src/react/
Files:
src/react/hooks/use-room-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-messages.tssrc/react/hooks/use-occupancy.tssrc/react/hooks/use-presence.ts
🧬 Code graph analysis (7)
src/core/typing.ts (1)
src/core/subscription.ts (1)
Subscription(12-19)
src/core/presence.ts (1)
src/core/index.ts (3)
RealtimePresenceParams(78-78)PresenceMember(67-67)PresenceData(67-67)
src/core/rooms.ts (2)
src/core/room.ts (3)
name(484-486)options(491-493)Room(23-367)src/core/room-options.ts (1)
RoomOptions(118-138)
src/react/hooks/use-room-reactions.ts (1)
src/core/room-reactions.ts (2)
SendReactionParams(17-52)RoomReactions(65-161)
src/react/hooks/use-typing.ts (2)
src/core/typing.ts (4)
TypingListener(184-184)keystroke(323-374)Typing(19-178)stop(379-429)src/react/types/chat-status-response.ts (1)
ChatStatusResponse(10-22)
src/react/hooks/use-messages.ts (4)
src/core/messages.ts (7)
SendMessageParams(127-163)UpdateMessageParams(107-122)OperationDetails(91-102)HistoryParams(57-86)MessageListener(169-169)Messages(275-587)history(810-813)src/core/message.ts (1)
Message(64-147)src/core/query.ts (1)
PaginatedResult(4-39)src/core/message-reactions.ts (4)
SendMessageReactionParams(43-61)DeleteMessageReactionParams(66-78)MessageReactionListener(32-32)MessageRawReactionListener(38-38)
src/react/hooks/use-presence.ts (2)
src/core/presence.ts (5)
PresenceData(81-81)update(475-485)Presence(133-377)enter(460-470)leave(490-500)src/core/index.ts (2)
PresenceData(67-67)Presence(67-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test node 20
- GitHub Check: test node 24
- GitHub Check: test node 22
🔇 Additional comments (14)
src/react/hooks/use-room-reactions.ts (5)
1-2: LGTM: Import for TypeDoc referencesThe unused Ably import with ESLint suppression is appropriate for supporting TypeDoc generation and JSDoc {@link} references in the examples below.
22-29: LGTM: Listener example is correctThe example correctly accesses the nested reaction properties (
reactionEvent.reaction.name,reactionEvent.reaction.clientId), consistent with theRoomReactionEventstructure.
40-72: LGTM: Comprehensive documentation with correct examplesThe expanded JSDoc for
sendRoomReactionprovides thorough context about ephemeral room reactions, attachment requirements, and error semantics. The example correctly usesnameandmetadataproperties as defined inSendReactionParams.
76-138: LGTM: Excellent comprehensive documentationThe main hook documentation is thorough and provides a complete working example that demonstrates proper usage patterns, error handling, and provider setup. All property accesses and method signatures are correct.
88-88: Static analysis warning is a false positiveThe linter warns that
@throwsshould have a type, but the documentation at line 88 correctly specifies the error type using TypeDoc's{@link}syntax:{@link chat-js!ErrorCode.ReactHookMustBeUsedWithinProvider | ReactHookMustBeUsedWithinProvider}. This follows the same pattern used throughout the file and is valid TypeDoc syntax.src/core/rooms.ts (1)
66-106: Based on the script output and web search results, I have sufficient information to address the review comment. The evidence shows:
- Room enforces unusable state after release: The RoomLifecycleManager throws
unable to <op> room; room is releasederrors (with ErrorCode.RoomInInvalidState)- Room tracks released state: Tests and logs show
RoomStatus.Releasedenum and lifecycle tracking- Components dispose on release: Messages, Presence, RoomReactions, Occupancy, and MessageReactions all have dispose() methods called during release
- Event listener cleanup: The Ably documentation states listeners are automatically removed by the SDK when connection/room is released
The review comment's concerns are validated. However, the statement "it doesn't unsubscribe existing event listeners" needs clarification in the documentation.
Rewrite of review comment:
Clarify what happens to event listeners after room release.
The documentation states "it doesn't unsubscribe existing event listeners" (line 70), but this is ambiguous. Based on the codebase, the room's dispose() methods (called during release) clean up all subscriptions. Listeners don't receive events after release because the room becomes unusable—RoomLifecycleManager throws errors when any operation is attempted.
Revise line 70 to clarify: Rather than saying listeners aren't unsubscribed, explain that listeners stop receiving events because the room enters a released state and all operations fail. For example: "After release, listeners will no longer receive events as the room is no longer usable and the underlying channel is detached."
src/core/typing.ts (4)
21-63: Excellent documentation with comprehensive example.The expanded documentation for
subscribeprovides clear context about room attachment requirements, real-time updates, and includes a practical example showing typing indicator UI patterns. The example demonstrates proper subscription lifecycle management.
68-95: Clear documentation of snapshot behavior.The documentation for
currenteffectively describes it as returning a snapshot of the typing state, with a practical example showing Set operations and attachment requirements.
99-131: Comprehensive documentation of keystroke behavior.The expanded documentation thoroughly explains the throttling mechanism, serialization guarantees, connection requirements, and error conditions. The example demonstrates proper async error handling.
135-176: Well-documented stop method with realistic usage context.The documentation mirrors the keystroke structure while adding specific details about no-op behavior. The example provides realistic context (user sends message or deletes draft) showing when to call
stop().src/react/hooks/use-typing.ts (4)
1-2: Appropriate ESLint suppression for documentation-only import.The Ably import is used exclusively in JSDoc comments to reference
{@link Ably.ErrorInfo}types. The ESLint suppression is appropriate since the import serves documentation purposes rather than runtime code.
36-113: Improved type explicitness and comprehensive documentation.The change from
TypingSetEvent['currentlyTyping']toSet<string>(line 112) improves code clarity by making the type explicit and removing the need to importTypingSetEvent. The expanded documentation forkeystrokeandstopthoroughly covers behavior, throttling, serialization, and error conditions with helpful examples.
115-189: Excellent comprehensive documentation with working example.The hook documentation is thorough and the example demonstrates proper usage patterns including:
- Subscription to typing events
- Proper error handling for both
keystroke()andstop()- Provider setup with ChatClientProvider and ChatRoomProvider
- Realistic UI implementation with input handling
The static analysis warning on line 115 appears to be a false positive—the
@throwstag properly references the error type using TypeDoc link syntax.
283-291: Type casts ensure interface compatibility.The casts to
Typing['keystroke']andTyping['stop']on lines 288-289 defensively ensure the useCallback-wrapped functions exactly match the coreTypinginterface signatures. While potentially unnecessary due to TypeScript's type inference, these casts provide explicit guarantees that the React hook's API surface remains compatible with the core interface.
src/core/presence.ts
Outdated
| * @param data - Optional JSON-serializable data to associate with the user's presence | ||
| * @returns Promise that resolves when the user has successfully entered | ||
| * @throws {Ably.ErrorInfo} with code 40000 if the room is not attached | ||
| * @throws {RoomInInvalidState} if the room is not attached |
There was a problem hiding this comment.
In JSDoc it's supposed to be the error type rather than the enum, so I think we should prefer the format {Ably.ErrorInfo} with code RoomInInvalidState
src/react/hooks/use-typing.ts
Outdated
| roomError, | ||
| keystroke, | ||
| stop, | ||
| keystroke: keystroke as Typing['keystroke'], |
There was a problem hiding this comment.
I don't think we need the as here - the types match up?
There was a problem hiding this comment.
Yeh I guess we can just rely on the returned function type, I'll remove :)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/react/hooks/use-typing.ts (1)
288-289: Remove unnecessary type casts.The callbacks at lines 274-281 already have the correct inferred type
() => Promise<void>, which matches theTypinginterface method signatures exactly. The type casts at lines 288-289 are redundant and can be safely removed.return { connectionStatus, connectionError, roomStatus, roomError, keystroke, stop, currentlyTyping, };src/react/hooks/use-room-reactions.ts (1)
201-201: Type cast appears defensive; consider explicit callback typing instead.The callback at lines 188-194 has the signature
(params: SendReactionParams) => Promise<void>, which matchesRoomReactions['send']exactly. The cast is likely defensive programming. Instead of relying on a type assertion, explicitly type theuseCallbackgeneric:const sendRoomReaction = useCallback<RoomReactions['send']>( async (params: SendReactionParams) => { const room = await context.room; return room.reactions.send(params); }, [context], );This allows TypeScript to enforce the contract without a cast, improving clarity and aligning with guidelines to minimize type assertions.
src/react/hooks/use-messages.ts (1)
652-656: Remove unnecessary type casts; use direct type annotation pattern consistent with reaction methods.The type casts at lines 652-656 are not required. The implementations (lines 494-532) delegate to
room.messagesmethods that already match theMessagesinterface signatures, so TypeScript correctly infers the types without explicit casts. This is demonstrated by thesendReactionanddeleteReactioncallbacks (lines 535, 541), which use direct type annotation (const sendReaction: Messages['reactions']['send'] = useCallback(...)) instead of casts and work correctly. Using direct annotation for all five methods would improve consistency and align with the coding guideline to minimize type assertions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (20)
ably-common(1 hunks)eslint.config.js(1 hunks)src/core/client-id.ts(1 hunks)src/core/message-reactions.ts(2 hunks)src/core/message.ts(1 hunks)src/core/occupancy.ts(1 hunks)src/core/presence.ts(1 hunks)src/core/serial.ts(1 hunks)src/react/hooks/internal/use-chat-client-context.ts(1 hunks)src/react/hooks/internal/use-room-context.ts(1 hunks)src/react/hooks/internal/use-room-reference-manager.ts(1 hunks)src/react/hooks/use-chat-client.ts(2 hunks)src/react/hooks/use-chat-connection.ts(2 hunks)src/react/hooks/use-messages.ts(6 hunks)src/react/hooks/use-occupancy.ts(3 hunks)src/react/hooks/use-presence-listener.ts(2 hunks)src/react/hooks/use-room-reactions.ts(4 hunks)src/react/hooks/use-room.ts(3 hunks)src/react/hooks/use-typing.ts(3 hunks)src/react/providers/chat-room-provider.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/core/serial.ts
- ably-common
🚧 Files skipped from review as they are similar to previous changes (9)
- src/core/client-id.ts
- src/react/hooks/use-presence-listener.ts
- src/react/hooks/use-chat-client.ts
- src/react/hooks/internal/use-room-reference-manager.ts
- src/react/hooks/internal/use-chat-client-context.ts
- src/core/occupancy.ts
- src/react/providers/chat-room-provider.tsx
- src/core/presence.ts
- src/core/message.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/file-conventions.mdc)
Create all Typescript and TSX files using kebab-case.
Import Ably as: import * as Ably from 'ably'
Files:
src/react/hooks/use-chat-connection.tssrc/core/message-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-room.tssrc/react/hooks/use-messages.tssrc/react/hooks/internal/use-room-context.tssrc/react/hooks/use-occupancy.ts
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx}: If there are any changes to the public-facing API, add a review comment: "Public API changes detected. Please update the documentation and the Ably CLI."
Files:
src/react/hooks/use-chat-connection.tssrc/core/message-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-room.tssrc/react/hooks/use-messages.tssrc/react/hooks/internal/use-room-context.tssrc/react/hooks/use-occupancy.ts
src/react/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react-conventions.mdc)
src/react/**/*.{ts,tsx}: Use camelCase for hook names in React custom hooks
Prefix all React hook names withuse(e.g.,useMessages,useLogger)
Use descriptive names for hooks that reflect the hook's purpose
Use JSDoc style comments for hooks
Include a clear description of the hook's purpose in its JSDoc comment
Document all hook parameters and return values using@param,@returns, and@throwsin JSDoc
Group related hook parameters into a single params object
Use TypeScript interfaces to define parameter shapes for hooks
Return objects with named properties for clarity from hooks
Define return type interfaces explicitly for hook return values
Document each returned property of a hook in its JSDoc comment
Memoize callbacks in hooks withuseCallback
Memoize expensive computations in hooks withuseMemo
Use refs for values in hooks that shouldn't trigger re-renders
Always clean up subscriptions and listeners in hooks
Handle component unmounting gracefully in hooks
Clear timers and intervals in hook cleanup functions
src/react/**/*.{ts,tsx}: React hooks: use camelCase names prefixed with 'use' and descriptive hook names
Document hooks with JSDoc including description, @param, @returns, @throws
Group related hook parameters into a single params object typed with a TypeScript interface
Return objects with named properties from hooks; define and export explicit return type interfaces; document returned properties
Optimize performance: memoize callbacks with useCallback, computations with useMemo, and use refs to avoid unnecessary re-renders
Always clean up subscriptions/listeners and clear timers/intervals in effect cleanupsReact hooks must follow React conventions: useCallback/useMemo for stable refs, refs for non-reactive values, and proper cleanup
Files:
src/react/hooks/use-chat-connection.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-room.tssrc/react/hooks/use-messages.tssrc/react/hooks/internal/use-room-context.tssrc/react/hooks/use-occupancy.ts
{src,test}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{src,test}/**/*.{ts,tsx}: Annotate source code with feature specification point IDs (e.g., // @CHA-M10a) where relevant
Create all TypeScript and TSX files using kebab-case filenames
{src,test}/**/*.{ts,tsx}: Use relative imports within the project
Use async/await instead of raw promises
Files:
src/react/hooks/use-chat-connection.tssrc/core/message-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-room.tssrc/react/hooks/use-messages.tssrc/react/hooks/internal/use-room-context.tssrc/react/hooks/use-occupancy.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use relative imports within the project
Import Ably as: import * as Ably from 'ably'
Prefer named exports over default exports; group related exports together
Interfaces: use PascalCase names; descriptive; no mandatory 'I' prefix; document with JSDoc including property docs, @throws, @return, and {@link} links
Enums: use PascalCase for enum and members; include JSDoc for enum and each member
Classes: use PascalCase; implement interfaces explicitly; avoid inheritance where possible; document class purpose
Class members: prefix private members with underscore; group by visibility; document all public members
Public methods must have JSDoc with @param, @returns, @throws
Prefer parameter objects for related parameters; use descriptive names and optional parameters with defaults
Use Ably.ErrorInfo for errors; message as first arg, error code second, status code third; use ErrorCodes enum from src/core/errors.ts
Error message format must be: 'unable to ; ' (use semicolon; start with 'unable to')
Logging: use consistent levels (trace, debug, error), include context object; never log Ably channel instances; structure messages for parsing
Minimize type assertions; prefer type guards; document any assertions; never use any (use unknown if unavoidable)
Use async/await over raw promises; handle rejections; document async behavior
Reference feature-specification.mdc for mapping spec points in code where applicable
**/*.{ts,tsx}: Use PascalCase for classes, interfaces, and enums
Prefix private members with an underscore (e.g., _roomId, _channel)
Avoid any; prefer unknown and strong typing
**/*.{ts,tsx}: Use relative imports within the project
When importing the package ably, useimport * as Ably from 'ably'
Prefer named exports over default exports
Group related exports together
Use PascalCase for interface names
Use descriptive interface names that reflect purpose
Do not prefix interfaces with 'I'
Use JSDoc comments for interfaces with...
Files:
src/react/hooks/use-chat-connection.tssrc/core/message-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-room.tssrc/react/hooks/use-messages.tssrc/react/hooks/internal/use-room-context.tssrc/react/hooks/use-occupancy.ts
{src,test,demo}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use kebab-case for TypeScript and TSX filenames
Files:
src/react/hooks/use-chat-connection.tssrc/core/message-reactions.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-room.tssrc/react/hooks/use-messages.tssrc/react/hooks/internal/use-room-context.tssrc/react/hooks/use-occupancy.ts
src/react/**
📄 CodeRabbit inference engine (CLAUDE.md)
React SDK source files live under src/react/
Files:
src/react/hooks/use-chat-connection.tssrc/react/hooks/use-typing.tssrc/react/hooks/use-room-reactions.tssrc/react/hooks/use-room.tssrc/react/hooks/use-messages.tssrc/react/hooks/internal/use-room-context.tssrc/react/hooks/use-occupancy.ts
src/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/core/**/*.{ts,tsx}: Create errors using Ably's ErrorInfo: new Ably.ErrorInfo(message, code, statusCode)
Use ErrorCodes enum from src/core/errors.ts for all error codes when creating ErrorInfo
All error messages must match: "unable to ; " (start with "unable to", lowercase operation, semicolon separator)
Use _logger.trace(), _logger.debug(), and _logger.error() with context objects for key operations
Never log Ably channel instances
Include spec point comments in code where applicable (// @CHA-)
Files:
src/core/message-reactions.ts
src/core/**
📄 CodeRabbit inference engine (CLAUDE.md)
Core SDK source files live under src/core/
Files:
src/core/message-reactions.ts
🧬 Code graph analysis (6)
src/react/hooks/use-chat-connection.ts (1)
src/core/index.ts (2)
ConnectionStatus(8-8)ErrorInfo(78-78)
src/core/message-reactions.ts (1)
src/core/chat-api.ts (2)
SendMessageReactionParams(75-92)DeleteMessageReactionParams(97-108)
src/react/hooks/use-typing.ts (2)
src/core/typing.ts (4)
TypingListener(184-184)keystroke(323-374)Typing(19-178)stop(379-429)src/react/types/chat-status-response.ts (1)
ChatStatusResponse(10-22)
src/react/hooks/use-room-reactions.ts (2)
src/core/room-reactions.ts (2)
SendReactionParams(17-52)RoomReactions(65-161)src/core/index.ts (2)
SendReactionParams(72-72)RoomReactions(72-72)
src/react/hooks/use-room.ts (2)
src/core/index.ts (1)
RoomStatusChange(73-73)src/core/room-status.ts (1)
RoomStatusChange(66-82)
src/react/hooks/use-messages.ts (5)
src/core/messages.ts (7)
SendMessageParams(127-163)UpdateMessageParams(107-122)OperationDetails(91-102)HistoryParams(57-86)MessageListener(169-169)Messages(275-587)history(810-813)src/core/message.ts (1)
Message(64-147)src/core/query.ts (1)
PaginatedResult(4-39)src/core/message-reactions.ts (4)
SendMessageReactionParams(43-61)DeleteMessageReactionParams(66-78)MessageReactionListener(32-32)MessageRawReactionListener(38-38)src/core/chat-api.ts (7)
SendMessageReactionParams(75-92)DeleteMessageReactionParams(97-108)sendMessage(202-209)getMessage(184-187)updateMessage(211-213)history(124-154)deleteMessage(189-200)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test node 22
- GitHub Check: test node 24
- GitHub Check: test node 20
🔇 Additional comments (12)
src/react/hooks/internal/use-room-context.ts (1)
11-11: LGTM: Documentation improvement.The addition of "An" before the error type link improves grammatical correctness and readability of the JSDoc comment.
eslint.config.js (1)
106-106: LGTM - Appropriate ESLint configuration for documentation expansion.Disabling
jsdoc/require-throws-typeis appropriate given the extensive JSDoc documentation being added in this PR. The error types are typically evident from context (e.g.,Ably.ErrorInfoor specific error codes), making explicit type annotations in@throwstags redundant.src/react/hooks/use-chat-connection.ts (1)
16-98: LGTM - Comprehensive documentation additions.The expanded documentation provides excellent guidance on:
- Lifecycle management (automatic cleanup on unmount)
- Provider requirements
- Real-time state updates
- Practical usage examples with proper error handling
The example is thorough without being overly complex, showing a complete component structure with provider setup.
src/react/hooks/use-occupancy.ts (1)
1-113: LGTM - Excellent documentation and minor functional enhancement.The documentation additions provide comprehensive guidance on:
- Real-time occupancy tracking
- Provider integration and lifecycle management
- Proper usage patterns with examples
The code also includes a functional enhancement at line 117 where
onConnectionStatusChangeis now properly forwarded touseChatConnection, enabling connection status callbacks as documented in theStatusParams.src/core/message-reactions.ts (2)
45-60: LGTM - Well-documented parameter additions.The addition of the optional
countparameter is properly documented and implemented:
- Clear usage guidance (Multiple type only)
- Proper default value (1)
- Implementation correctly handles the new parameter (lines 457-463)
85-335: LGTM - Comprehensive API documentation.The expanded documentation for all methods is excellent:
- Clear behavioral descriptions (ephemeral, REST API usage, room attachment requirements)
- Detailed error semantics with specific codes
- Practical, copy-able examples with proper error handling
- Important notes about clipping, rollup, and real-time behavior
src/react/hooks/use-room.ts (1)
1-156: LGTM - Clear provider-based lifecycle documentation.The documentation effectively communicates:
- Provider-managed attachment (manual attach/detach rarely needed)
- Stable reference guarantees for attach/detach methods
- Proper provider hierarchy and usage patterns
- Comprehensive example with status callbacks
src/react/hooks/use-room-reactions.ts (1)
1-138: LGTM - Excellent documentation of room reaction semantics.The documentation clearly emphasizes:
- Ephemeral, non-persistent nature of room reactions (vs. message reactions)
- Provider-managed attachment
- Real-time feedback use cases
- Proper usage patterns with comprehensive examples
src/react/hooks/use-typing.ts (2)
112-112: LGTM - Improved type clarity.Changing
currentlyTypingfromTypingSetEvent['currentlyTyping']to the explicitSet<string>type improves clarity and makes the public API surface more maintainable.
24-189: LGTM - Comprehensive typing indicator documentation.The documentation excell at explaining:
- Throttling behavior and serialization of keystroke/stop calls
- Connection state requirements
- Provider-managed room attachment
- Practical usage patterns with input change handlers
src/react/hooks/use-messages.ts (2)
5-22: LGTM - Necessary imports for type-safe public API.The addition of
MessageandPaginatedResultimports enables the hook to expose properly typed return values that align with the core API, improving type safety and developer experience.
38-475: LGTM - Outstanding comprehensive documentation.The documentation for
useMessagesis exemplary:
- Each method clearly documents REST API usage, timing considerations, and error cases
- The comprehensive example (lines 379-475) demonstrates:
- Proper message state management with
Message.with()- Reaction summary updates
- Discontinuity handling
- Deleted message styling
- All parameter examples are clear and correct
- Past issues with reaction parameter naming have been resolved
splindsay-92
force-pushed
the
typedocs/ensure-full-function-context-is-covered
branch
from
27e11e1 to
e35fb69
Compare
…ions and examples.
…tions and examples.
…move unnecessary ESLint ignores
splindsay-92
force-pushed
the
typedocs/ensure-full-function-context-is-covered
branch
from
e35fb69 to
f088db7
Compare
Context
Following the findings from the LLM testbed work, we should ensure typedocs give a basic happy path example, and full behavioural context for functions.
Note
Expands documentation with examples and error semantics across core and React APIs, clarifies TypeScript signatures, and bumps typedoc with a small ESLint tweak.
ChatClient,Connection,Rooms,Room,Messages,MessageReactions,Presence,Occupancy,Typing, and React hooks/providers (useChatClient,useChatConnection,useMessages,usePresence,usePresenceListener,useTyping,useRoom,useRoomReactions,ChatClientProvider,ChatRoomProvider).sendMessage,updateMessage,deleteMessage, reactions helpers), and exposecurrentlyTypingasSet<string>.historyBeforeSubscribe).typedocto^0.28.13.jsdoc/require-throws-typerule.Written by Cursor Bugbot for commit f088db7. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Release Notes
New Features
clientIdgetter to ChatClient for accessing the client ID.countparameter for message reactions.Improvements
Dependencies