From 515f5f2da45f63e80859868b6207b84de577ee54 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Wed, 27 May 2026 17:19:35 +1000 Subject: [PATCH 01/10] Add AbortSignal, hide sendToConversation, uniform ChatError This is PR 1 of the v1-prep pass. Three orthogonal hardenings: 1. AbortSignal across the public surface - Add `OperationOptions { abortSignal? }` and per-method aliases (`StartOptions`, `GetRoomOptions`, `CreateRoomOptions`, `SendMessageOptions`, `GetUserInfoOptions`, `RoomMemberOperationOptions`, `StopOptions`). - Extend `ListRoomMessagesOptions` with `OperationOptions` so its `abortSignal` reaches every paged `invokeEvent` call inside `fetchPage`. - Thread the signal through `invokeWithReturnType` and `start` / `startCore` (initial connection, login, room hydration). - Add `@azure/abort-controller@^2.1.2` as an explicit dependency so `AbortSignalLike` is reachable from consumer typings. 2. `sendToConversation` is now `private` The conversation concept is an internal implementation detail; the only supported entry points are `sendToRoom` / `listRoomMessages`. Removing this method from the public surface is the one (small) breaking change in this PR. 3. Uniform `ChatError` for thrown failures `ensureStarted`, the `userId` getter, unknown-room rejections in `sendToRoom` / `listRoomMessages`, and the malformed-response guard in `sendToConversation` all now throw `ChatError` with a service-style `code` instead of a plain `Error`. Three new codes are exposed: `NotStarted`, `UnknownRoom`, `InvalidServerResponse`. `index.ts` re-exports the constants object as `KnownChatErrorCode`, matching the Azure SDK `Known` convention used by `KnownErrorCode` in cognitive-language and the `KnownValidationErrorCode` pattern in ARM packages. Keys are PascalCase (`KnownChatErrorCode.UnknownRoom`) so the value used at runtime and the wire string returned by the service match exactly. README: Methods table updated to show the trailing `options?` argument, plus a new "Errors" section documenting `KnownChatErrorCode`. Deferred to a follow-up "model" PR: public `Message` / `Room` / `UserProfile` types via hand-picked `Pick`/`Omit` aliases, `MessageEvent.conversationId` removal, ergonomics fixes (`createRoom`/`getRoom` options-bag), `pageSize` -> `maxPageSize`, sender-echo synthesis, and the `ChatMessage` empty-interface cleanup. Verification: `tsc --noEmit` clean, `tsx --test` 6/6 lifecycle tests green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/README.md | 56 ++++++-- sdk/webpubsub-chat-client/package.json | 1 + sdk/webpubsub-chat-client/src/chatClient.ts | 146 +++++++++++++++----- sdk/webpubsub-chat-client/src/constant.ts | 9 +- sdk/webpubsub-chat-client/src/index.ts | 33 +++++ sdk/webpubsub-chat-client/src/options.ts | 40 +++++- 6 files changed, 233 insertions(+), 52 deletions(-) diff --git a/sdk/webpubsub-chat-client/README.md b/sdk/webpubsub-chat-client/README.md index 9949b34ad..799a6ee30 100644 --- a/sdk/webpubsub-chat-client/README.md +++ b/sdk/webpubsub-chat-client/README.md @@ -98,15 +98,55 @@ When constructed from an existing `WebPubSubClient`, `ChatClient` owns that clie | Method | Description | |--------|-------------| -| `start()` | Connect and authenticate. Idempotent; concurrent calls share one in-flight promise. After `stop()` the client can be started again. | +| `start(options?)` | Connect and authenticate. Idempotent; concurrent calls share one in-flight promise. After `stop()` the client can be started again. Accepts `{ abortSignal }`. | | `stop()` | Disconnect and reset client state. Returns `Promise`. | -| `createRoom(title, members, roomId?)` | Create a new room with initial members. The current user is automatically added to the members list. | -| `getRoom(roomId, withMembers)` | Get room info | -| `addUserToRoom(roomId, userId)` | Add user to room (admin operation) | -| `removeUserFromRoom(roomId, userId)` | Remove user from room (admin operation) | -| `sendToRoom(roomId, message)` | Send text message to room, returns message ID | -| `listRoomMessages(options)` | Paged async iterator over room message history (auto-paginates). Use `for await` to stream every message, or `.byPage({ maxPageSize })` to load up to `maxPageSize` messages at a time. `options = { roomId, startId?, endId?, pageSize? }` | -| `getUserInfo(userId)` | Get user profile | +| `createRoom(title, members, roomId?, options?)` | Create a new room with initial members. The current user is automatically added to the members list. | +| `getRoom(roomId, withMembers, options?)` | Get room info | +| `addUserToRoom(roomId, userId, options?)` | Add user to room (admin operation) | +| `removeUserFromRoom(roomId, userId, options?)` | Remove user from room (admin operation) | +| `sendToRoom(roomId, message, options?)` | Send text message to room, returns message ID | +| `listRoomMessages(options)` | Paged async iterator over room message history (auto-paginates). Use `for await` to stream every message, or `.byPage({ maxPageSize })` to load up to `maxPageSize` messages at a time. `options = { roomId, startId?, endId?, pageSize?, abortSignal? }` | +| `getUserInfo(userId, options?)` | Get user profile | + +Every asynchronous method accepts an optional final `options` argument +extending `OperationOptions` (`{ abortSignal?: AbortSignalLike }`) to +cancel in-flight invocations: + +```ts +const ac = new AbortController(); +setTimeout(() => ac.abort(), 5000); +await client.sendToRoom(roomId, "hi", { abortSignal: ac.signal }); +``` + +#### Errors + +Operations reject with a `ChatError` (extends `Error`) carrying a +service-defined `code`. Known codes are exposed as `KnownChatErrorCode`; +the service may return additional codes in newer versions, so always +handle the unknown-code case. + +```ts +import { ChatError, KnownChatErrorCode } from "@azure/web-pubsub-chat-client"; + +try { + await client.sendToRoom(roomId, "hi"); +} catch (err) { + if (err instanceof ChatError && err.code === KnownChatErrorCode.UnknownRoom) { + // re-join or refresh local state + } else { + throw err; + } +} +``` + +| Code | Meaning | +|------|---------| +| `KnownChatErrorCode.RoomAlreadyExists` | Tried to create a room whose `roomId` is already in use. | +| `KnownChatErrorCode.UserAlreadyInRoom` | Adding a user that is already a member. | +| `KnownChatErrorCode.NoPermissionInRoom` | Caller lacks permission to perform the operation. | +| `KnownChatErrorCode.NotStarted` | An API was called before `start()` resolved. | +| `KnownChatErrorCode.UnknownRoom` | The room is not in the client's local cache (joined or created). | +| `KnownChatErrorCode.InvalidServerResponse` | The service returned a malformed response. | #### Event Listeners diff --git a/sdk/webpubsub-chat-client/package.json b/sdk/webpubsub-chat-client/package.json index 4df6dc845..9171f193f 100644 --- a/sdk/webpubsub-chat-client/package.json +++ b/sdk/webpubsub-chat-client/package.json @@ -52,6 +52,7 @@ "test:one": "tsx --test --test-name-pattern" }, "dependencies": { + "@azure/abort-controller": "^2.1.2", "@azure/core-paging": "^1.6.2", "@azure/logger": "^1.3.0", "@azure/web-pubsub-client": "1.0.4", diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index e35ecf269..6034d227b 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -27,7 +27,17 @@ import type { RoomJoinedEvent, RoomLeftEvent, } from "./events.js"; -import type { ListRoomMessagesOptions } from "./options.js"; +import type { + ListRoomMessagesOptions, + OperationOptions, + StartOptions, + StopOptions, + GetRoomOptions, + CreateRoomOptions, + SendMessageOptions, + GetUserInfoOptions, + RoomMemberOperationOptions, +} from "./options.js"; import { ERRORS, INVOCATION_NAME } from "./constant.js"; import { logger } from "./logger.js"; @@ -185,10 +195,17 @@ class ChatClient { } /** Invoke server event and return typed data */ - private async invokeWithReturnType(eventName: string, payload: any, dataType: WebPubSubDataType): Promise { + private async invokeWithReturnType( + eventName: string, + payload: any, + dataType: WebPubSubDataType, + options?: OperationOptions, + ): Promise { logger.verbose(`invoke event: '${eventName}', dataType: ${dataType}, payload:`, payload); try { - const rawResponse = await this.connection.invokeEvent(eventName, payload, dataType); + const rawResponse = await this.connection.invokeEvent(eventName, payload, dataType, { + abortSignal: options?.abortSignal, + }); logger.verbose(`invoke response for '${eventName}':`, rawResponse); const data = rawResponse.data as any; if (data && typeof data === "object" && typeof data.code === "string") { @@ -234,8 +251,11 @@ class ChatClient { * calls made on an already-started client resolve immediately. After * `stop()` the client can be started again; state from the previous * session is reset. + * + * @param options - Cancellation token for the start operation. Aborting + * leaves the client in its initial (not-started) state. */ - public async start(): Promise { + public async start(options?: StartOptions): Promise { if (this._startPromise) return this._startPromise; if (this._isStarted) return; @@ -246,7 +266,7 @@ class ChatClient { if (this._isStarted) return; } - const startPromise = this.startCore(); + const startPromise = this.startCore(options); this._startPromise = startPromise; try { await startPromise; @@ -257,21 +277,31 @@ class ChatClient { } } - private async startCore(): Promise { + private async startCore(options?: StartOptions): Promise { this.resetState(); try { - await this.connection.start(); + await this.connection.start({ abortSignal: options?.abortSignal }); this._connectionStoppedTCS = new PromiseCompletionSource(); this._isConnectionStopping = false; - const loginResponse = await this.invokeWithReturnType(INVOCATION_NAME.LOGIN, "", "text"); + const loginResponse = await this.invokeWithReturnType( + INVOCATION_NAME.LOGIN, + "", + "text", + options, + ); logger.info("loginResponse", loginResponse); const conversationIds = new Set(loginResponse.conversationIds || []); const roomInfos = await Promise.all( (loginResponse.roomIds || []).map(async (roomId) => { - const roomInfo = await this.invokeWithReturnType(INVOCATION_NAME.GET_ROOM, { id: roomId, withMembers: false }, "json"); + const roomInfo = await this.invokeWithReturnType( + INVOCATION_NAME.GET_ROOM, + { id: roomId, withMembers: false }, + "json", + options, + ); return { roomId, roomInfo }; - }) + }), ); this._userId = loginResponse.userId; @@ -294,24 +324,41 @@ class ChatClient { private ensureStarted(): void { if (!this._isStarted) { - throw new Error("Not started. Please call start() first."); + throw new ChatError("Not started. Please call start() first.", ERRORS.NotStarted); } } - public async getUserInfo(userId: string): Promise { + public async getUserInfo(userId: string, options?: GetUserInfoOptions): Promise { this.ensureStarted(); - return this.invokeWithReturnType(INVOCATION_NAME.GET_USER_PROPERTIES, { userId: userId }, "json"); - } - - public async sendToConversation(conversationId: string, message: string): Promise { + return this.invokeWithReturnType( + INVOCATION_NAME.GET_USER_PROPERTIES, + { userId: userId }, + "json", + options, + ); + } + + private async sendToConversation( + conversationId: string, + message: string, + options?: SendMessageOptions, + ): Promise { this.ensureStarted(); const payload = { conversation: { conversationId: conversationId }, content: message, }; - const resp = await this.invokeWithReturnType(INVOCATION_NAME.SEND_TEXT_MESSAGE, payload, "json"); + const resp = await this.invokeWithReturnType( + INVOCATION_NAME.SEND_TEXT_MESSAGE, + payload, + "json", + options, + ); if (!resp || !resp.id) { - throw new Error(`Failed to send message to conversation ${conversationId}, got invalid invoke response: ${JSON.stringify(resp)}`); + throw new ChatError( + `Failed to send message to conversation ${conversationId}, got invalid invoke response: ${JSON.stringify(resp)}`, + ERRORS.InvalidServerResponse, + ); } const msgId = resp.id; const roomId = Array.from(this._rooms.values()).find((r) => r.defaultConversationId === conversationId)?.roomId; @@ -336,22 +383,32 @@ class ChatClient { return msgId; } - public async sendToRoom(roomId: string, message: string): Promise { + public async sendToRoom(roomId: string, message: string, options?: SendMessageOptions): Promise { this.ensureStarted(); const conversationId = this._rooms.get(roomId)?.defaultConversationId; if (!conversationId) { - throw Error(`Failed to sendToRoom, not found roomId ${roomId}`); + throw new ChatError(`Failed to sendToRoom, not found roomId ${roomId}`, ERRORS.UnknownRoom); } - return await this.sendToConversation(conversationId, message); + return await this.sendToConversation(conversationId, message, options); } - public async getRoom(roomId: string, withMembers: boolean): Promise { + public async getRoom(roomId: string, withMembers: boolean, options?: GetRoomOptions): Promise { this.ensureStarted(); - return this.invokeWithReturnType(INVOCATION_NAME.GET_ROOM, { id: roomId, withMembers: withMembers }, "json"); + return this.invokeWithReturnType( + INVOCATION_NAME.GET_ROOM, + { id: roomId, withMembers: withMembers }, + "json", + options, + ); } /** Create a room and its initial members. If `roomId` is not set, the service will create a random one. */ - public async createRoom(title: string, members: string[], roomId?: string): Promise { + public async createRoom( + title: string, + members: string[], + roomId?: string, + options?: CreateRoomOptions, + ): Promise { this.ensureStarted(); let roomDetails = { title: title, @@ -360,27 +417,35 @@ class ChatClient { if (roomId) { roomDetails = { ...roomDetails, roomId: roomId }; } - const roomInfo = await this.invokeWithReturnType(INVOCATION_NAME.CREATE_ROOM, roomDetails, "json"); + const roomInfo = await this.invokeWithReturnType( + INVOCATION_NAME.CREATE_ROOM, + roomDetails, + "json", + options, + ); this._rooms.set(roomInfo.roomId, roomInfo); const event: RoomJoinedEvent = { room: roomInfo }; this._emitter.emit("roomJoined", event); return roomInfo; } - private async manageRoomMember(request: ManageRoomMemberRequest): Promise { - await this.invokeWithReturnType(INVOCATION_NAME.MANAGE_ROOM_MEMBER, request, "json"); + private async manageRoomMember( + request: ManageRoomMemberRequest, + options?: RoomMemberOperationOptions, + ): Promise { + await this.invokeWithReturnType(INVOCATION_NAME.MANAGE_ROOM_MEMBER, request, "json", options); } - private async ensureRoomCached(roomId: string): Promise { + private async ensureRoomCached(roomId: string, options?: OperationOptions): Promise { if (this._rooms.has(roomId)) { return; } - const roomInfo = await this.getRoom(roomId, false); + const roomInfo = await this.getRoom(roomId, false, options); this._rooms.set(roomId, roomInfo); } /** Add a user to a room. This is an admin operation where one user adds another user to a room. */ - public async addUserToRoom(roomId: string, userId: string): Promise { + public async addUserToRoom(roomId: string, userId: string, options?: RoomMemberOperationOptions): Promise { this.ensureStarted(); const payload: ManageRoomMemberRequest = { roomId: roomId, operation: "Add", userId: userId }; const isSelf = userId === this.userId; @@ -388,22 +453,22 @@ class ChatClient { // from the service so sendToRoom can use its conversation id. const shouldCacheRoomAfterSelfAdd = isSelf && !this._rooms.has(roomId); try { - await this.manageRoomMember(payload); + await this.manageRoomMember(payload, options); } catch (error) { - if (!isSelf || !(error instanceof ChatError && error.code === ERRORS.USER_ALREADY_IN_ROOM)) { + if (!isSelf || !(error instanceof ChatError && error.code === ERRORS.UserAlreadyInRoom)) { throw error; } } if (shouldCacheRoomAfterSelfAdd) { - await this.ensureRoomCached(roomId); + await this.ensureRoomCached(roomId, options); } } /** Remove a user from a room. This is an admin operation where one user removes another user from a room. */ - public async removeUserFromRoom(roomId: string, userId: string): Promise { + public async removeUserFromRoom(roomId: string, userId: string, options?: RoomMemberOperationOptions): Promise { this.ensureStarted(); const payload: ManageRoomMemberRequest = { roomId: roomId, operation: "Delete", userId: userId }; - await this.manageRoomMember(payload); + await this.manageRoomMember(payload, options); // RoomLeft notification is not guaranteed for server-managed membership; // eagerly clean up local cache and emit RoomLeft so callers see consistent state immediately. if (userId === this.userId) { @@ -447,7 +512,7 @@ class ChatClient { this.ensureStarted(); const conversationId = this._rooms.get(options.roomId)?.defaultConversationId; if (!conversationId) { - throw new Error(`Failed to listRoomMessages, not found roomId ${options.roomId}`); + throw new ChatError(`Failed to listRoomMessages, not found roomId ${options.roomId}`, ERRORS.UnknownRoom); } const defaultPageSize = options.pageSize ?? 100; @@ -470,6 +535,7 @@ class ChatClient { INVOCATION_NAME.LIST_MESSAGES, query, "json", + { abortSignal: options.abortSignal }, ); if (result.messages.length === 0) { return undefined; @@ -495,7 +561,7 @@ class ChatClient { public get userId(): string { if (!this._userId) { - throw new Error("User ID is not set. Please call start() first."); + throw new ChatError("User ID is not set. Please call start() first.", ERRORS.NotStarted); } return this._userId; } @@ -555,8 +621,12 @@ class ChatClient { * be started again via `start()`. Callers that want the same identity * should keep their authentication source (URL or credential) * constant. + * + * @param _options - Reserved for symmetry with other operations. + * Stopping is a local-state cleanup that does not perform a network + * round-trip and is therefore not cancellable today. */ - public async stop(): Promise { + public async stop(_options?: StopOptions): Promise { const startPromise = this._startPromise; if (startPromise) { await startPromise.catch(() => undefined); diff --git a/sdk/webpubsub-chat-client/src/constant.ts b/sdk/webpubsub-chat-client/src/constant.ts index ad8273db3..5ba3bf341 100644 --- a/sdk/webpubsub-chat-client/src/constant.ts +++ b/sdk/webpubsub-chat-client/src/constant.ts @@ -11,9 +11,12 @@ const INVOCATION_NAME = { } as const; const ERRORS = { - ROOM_ALREADY_EXISTS: "RoomAlreadyExists", - USER_ALREADY_IN_ROOM: "UserAlreadyInRoom", - NO_PERMISSION_IN_ROOM: "NoPermissionInRoom", + RoomAlreadyExists: "RoomAlreadyExists", + UserAlreadyInRoom: "UserAlreadyInRoom", + NoPermissionInRoom: "NoPermissionInRoom", + NotStarted: "NotStarted", + UnknownRoom: "UnknownRoom", + InvalidServerResponse: "InvalidServerResponse", } as const; export { INVOCATION_NAME, ERRORS }; diff --git a/sdk/webpubsub-chat-client/src/index.ts b/sdk/webpubsub-chat-client/src/index.ts index e5553498c..1d7e1c85b 100644 --- a/sdk/webpubsub-chat-client/src/index.ts +++ b/sdk/webpubsub-chat-client/src/index.ts @@ -1,4 +1,5 @@ import { ChatClient, ChatError } from './chatClient.js'; +import { ERRORS } from './constant.js'; export type { MessageInfo, @@ -21,7 +22,39 @@ export type { } from './events.js'; export type { + OperationOptions, + StartOptions, + StopOptions, + GetRoomOptions, + CreateRoomOptions, + SendMessageOptions, + GetUserInfoOptions, + RoomMemberOperationOptions, ListRoomMessagesOptions, } from './options.js'; +/** + * Known values of `ChatError.code`. Following the Azure SDK + * `Known` convention, this is a runtime constants object whose + * values are the wire codes returned by the chat service. Compare + * `error.code` against members of this object rather than against + * string literals. + * + * @example + * ```ts + * try { + * await client.sendToRoom(roomId, "hi"); + * } catch (err) { + * if (err instanceof ChatError && err.code === KnownChatErrorCode.UnknownRoom) { + * // ... + * } + * } + * ``` + * + * The service may add codes in newer versions, so always handle the + * unknown-code case as well. + */ +export const KnownChatErrorCode = ERRORS; + export { ChatClient, ChatError }; + diff --git a/sdk/webpubsub-chat-client/src/options.ts b/sdk/webpubsub-chat-client/src/options.ts index 4a7f79867..f2dfac216 100644 --- a/sdk/webpubsub-chat-client/src/options.ts +++ b/sdk/webpubsub-chat-client/src/options.ts @@ -1,12 +1,46 @@ /** * Options-object types for `ChatClient` methods. * - * As more methods migrate to the options-object pattern, additional - * interfaces will live here. + * Every asynchronous method accepts an options bag extending + * {@link OperationOptions} with at least an `abortSignal`. Future + * per-operation knobs (custom headers, retry policy, ...) live on + * these interfaces too. */ +import type { AbortSignalLike } from "@azure/abort-controller"; + +/** Base options accepted by every asynchronous `ChatClient` operation. */ +export interface OperationOptions { + /** + * Signal used to cancel the operation. Accepts either a browser + * `AbortSignal` or `@azure/abort-controller`'s polyfill. + */ + abortSignal?: AbortSignalLike; +} + +/** Options for {@link ChatClient.start}. */ +export interface StartOptions extends OperationOptions {} + +/** Options for {@link ChatClient.stop}. */ +export interface StopOptions extends OperationOptions {} + +/** Options for {@link ChatClient.getRoom}. */ +export interface GetRoomOptions extends OperationOptions {} + +/** Options for {@link ChatClient.createRoom}. */ +export interface CreateRoomOptions extends OperationOptions {} + +/** Options for {@link ChatClient.sendToRoom}. */ +export interface SendMessageOptions extends OperationOptions {} + +/** Options for {@link ChatClient.getUserInfo}. */ +export interface GetUserInfoOptions extends OperationOptions {} + +/** Options for {@link ChatClient.addUserToRoom} and {@link ChatClient.removeUserFromRoom}. */ +export interface RoomMemberOperationOptions extends OperationOptions {} + /** Options for {@link ChatClient.listRoomMessages}. */ -export interface ListRoomMessagesOptions { +export interface ListRoomMessagesOptions extends OperationOptions { /** Room to list messages from. Must be a room this client has created or joined. */ roomId: string; /** From a9e31a5e090bac67391502769a15574921df9c1d Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Wed, 27 May 2026 17:42:05 +1000 Subject: [PATCH 02/10] Add api-extractor and fix protected _conversationIds leak Tooling: @microsoft/api-extractor This is the canonical TypeScript API-surface lock for the Azure SDK ecosystem; every package under azure-sdk-for-js ships a generated review/.api.md that CI keeps in sync. We adopt the same workflow here: - Add @microsoft/api-extractor@^7.58.7 as a devDependency. - Add api-extractor.json (minimal config: report only, no doc-model or dts-rollup since we ship hand-curated dist typings). - Wire two npm scripts: extract-api -- api-extractor run --local (update report) check-api -- api-extractor run (fail if stale) - Commit the generated review/web-pubsub-chat-client.api.md so the public surface is reviewable as a diff on every PR. Bug fix surfaced by the report `_conversationIds: Set` was declared `protected`, leaking internal state into the API surface. Demoted to `private`; the field is purely an internal cache populated during login. TSDoc cleanup driven by api-extractor warnings - Replaced ambiguous {@link ChatClient.start} references (the static and instance overloads share the name) with inline code refs in the constructor JSDoc and in options.ts. - Replaced {@link WebPubSubClientCredential} with plain text -- that symbol is not re-exported from this package, so it can't be a link target. - Replaced {@link on} (ambiguous, also collides with EventEmitter) with plain `on()` reference. - Wrapped the inline @example in a ```ts code fence so the `=>` arrow no longer trips tsdoc-escape-greater-than. Hygiene - .gitignore: ignore api-extractor's `temp/` working directory and `package-lock.json` (the package uses yarn.lock as the lockfile). Verification - tsc --noEmit clean. - api-extractor run --local: zero warnings. - tsx --test tests/lifecycle.test.ts: 6/6 pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/.gitignore | 3 + sdk/webpubsub-chat-client/api-extractor.json | 36 ++++ sdk/webpubsub-chat-client/package.json | 5 +- .../review/web-pubsub-chat-client.api.md | 195 ++++++++++++++++++ sdk/webpubsub-chat-client/src/chatClient.ts | 12 +- sdk/webpubsub-chat-client/src/options.ts | 16 +- 6 files changed, 253 insertions(+), 14 deletions(-) create mode 100644 sdk/webpubsub-chat-client/api-extractor.json create mode 100644 sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md diff --git a/sdk/webpubsub-chat-client/.gitignore b/sdk/webpubsub-chat-client/.gitignore index 89827b2e1..c266ba67b 100644 --- a/sdk/webpubsub-chat-client/.gitignore +++ b/sdk/webpubsub-chat-client/.gitignore @@ -5,3 +5,6 @@ tsconfig.tsbuildinfo .env .yarn *.tgz +# api-extractor working directory +temp +package-lock.json diff --git a/sdk/webpubsub-chat-client/api-extractor.json b/sdk/webpubsub-chat-client/api-extractor.json new file mode 100644 index 000000000..bcbd3b5f7 --- /dev/null +++ b/sdk/webpubsub-chat-client/api-extractor.json @@ -0,0 +1,36 @@ +{ + "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json", + + "mainEntryPointFilePath": "/dist/index.d.ts", + + "apiReport": { + "enabled": true, + "reportFolder": "/review/", + "reportFileName": ".api.md" + }, + + "docModel": { + "enabled": false + }, + + "dtsRollup": { + "enabled": false + }, + + "tsdocMetadata": { + "enabled": false + }, + + "messages": { + "compilerMessageReporting": { + "default": { "logLevel": "warning" } + }, + "extractorMessageReporting": { + "default": { "logLevel": "warning" }, + "ae-missing-release-tag": { "logLevel": "none" } + }, + "tsdocMessageReporting": { + "default": { "logLevel": "warning" } + } + } +} diff --git a/sdk/webpubsub-chat-client/package.json b/sdk/webpubsub-chat-client/package.json index 9171f193f..a0175351d 100644 --- a/sdk/webpubsub-chat-client/package.json +++ b/sdk/webpubsub-chat-client/package.json @@ -49,7 +49,9 @@ "test:start-server": "node .\\examples\\quickstart\\server.js", "test": "npx tsx --test tests/**/*.ts", "test:integration": "tsx --test tests/integration.test.ts", - "test:one": "tsx --test --test-name-pattern" + "test:one": "tsx --test --test-name-pattern", + "extract-api": "api-extractor run --local", + "check-api": "api-extractor run" }, "dependencies": { "@azure/abort-controller": "^2.1.2", @@ -60,6 +62,7 @@ "ws": "^8.0.0" }, "devDependencies": { + "@microsoft/api-extractor": "^7.58.7", "@types/events": "^3.0.3", "@types/node": "^25.0.3", "esbuild": "^0.27.3", diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md new file mode 100644 index 000000000..98cea879f --- /dev/null +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -0,0 +1,195 @@ +## API Report File for "@azure/web-pubsub-chat-client" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts + +import type { AbortSignalLike } from '@azure/abort-controller'; +import { PagedAsyncIterableIterator } from '@azure/core-paging'; +import { WebPubSubClient } from '@azure/web-pubsub-client'; +import { WebPubSubClientCredential } from '@azure/web-pubsub-client'; +import { WebPubSubClientOptions } from '@azure/web-pubsub-client'; + +// @public (undocumented) +export class ChatClient { + constructor(clientAccessUrl: string, options?: WebPubSubClientOptions); + constructor(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions); + constructor(wpsClient: WebPubSubClient); + addUserToRoom(roomId: string, userId: string, options?: RoomMemberOperationOptions): Promise; + // (undocumented) + readonly connection: WebPubSubClient; + createRoom(title: string, members: string[], roomId?: string, options?: CreateRoomOptions): Promise; + // (undocumented) + getRoom(roomId: string, withMembers: boolean, options?: GetRoomOptions): Promise; + // (undocumented) + getUserInfo(userId: string, options?: GetUserInfoOptions): Promise; + hasJoinedRoom(roomId: string): boolean; + get isStarted(): boolean; + listRoomMessages(options: ListRoomMessagesOptions): PagedAsyncIterableIterator; + off(event: K, callback: ChatEventListener): void; + on(event: K, callback: ChatEventListener): Disposable_2; + onMemberJoined(callback: ChatEventListener<"memberJoined">): Disposable_2; + onMemberLeft(callback: ChatEventListener<"memberLeft">): Disposable_2; + onMessage(callback: ChatEventListener<"message">): Disposable_2; + onRoomJoined(callback: ChatEventListener<"roomJoined">): Disposable_2; + onRoomLeft(callback: ChatEventListener<"roomLeft">): Disposable_2; + removeUserFromRoom(roomId: string, userId: string, options?: RoomMemberOperationOptions): Promise; + get rooms(): RoomInfo[]; + // (undocumented) + sendToRoom(roomId: string, message: string, options?: SendMessageOptions): Promise; + static start(clientAccessUrl: string, options?: WebPubSubClientOptions): Promise; + // (undocumented) + static start(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions): Promise; + // (undocumented) + static start(wpsClient: WebPubSubClient): Promise; + start(options?: StartOptions): Promise; + stop(_options?: StopOptions): Promise; + // (undocumented) + get userId(): string; +} + +// @public (undocumented) +export class ChatError extends Error { + constructor(message: string, code: string); + // (undocumented) + readonly code: string; +} + +// @public (undocumented) +export type ChatEventListener = (event: ChatEventMap[K]) => void; + +// @public +export interface ChatEventMap { + // (undocumented) + memberJoined: MemberJoinedEvent; + // (undocumented) + memberLeft: MemberLeftEvent; + // (undocumented) + message: MessageEvent_2; + // (undocumented) + roomJoined: RoomJoinedEvent; + // (undocumented) + roomLeft: RoomLeftEvent; +} + +// @public (undocumented) +export type ChatEventName = keyof ChatEventMap; + +// @public +export interface ChatMessage extends MessageInfo { +} + +// @public +export interface CreateRoomOptions extends OperationOptions { +} + +// @public +type Disposable_2 = () => void; +export { Disposable_2 as Disposable } + +// @public +export interface GetRoomOptions extends OperationOptions { +} + +// @public +export interface GetUserInfoOptions extends OperationOptions { +} + +// @public +export const KnownChatErrorCode: { + readonly RoomAlreadyExists: "RoomAlreadyExists"; + readonly UserAlreadyInRoom: "UserAlreadyInRoom"; + readonly NoPermissionInRoom: "NoPermissionInRoom"; + readonly NotStarted: "NotStarted"; + readonly UnknownRoom: "UnknownRoom"; + readonly InvalidServerResponse: "InvalidServerResponse"; +}; + +// @public +export interface ListRoomMessagesOptions extends OperationOptions { + endId?: string; + pageSize?: number; + roomId: string; + startId?: string; +} + +// @public +export interface MemberJoinedEvent { + // (undocumented) + roomId: string; + // (undocumented) + title: string; + // (undocumented) + userId: string; +} + +// @public +export interface MemberLeftEvent { + // (undocumented) + roomId: string; + // (undocumented) + title: string; + // (undocumented) + userId: string; +} + +// @public +interface MessageEvent_2 { + conversationId: string; + message: ChatMessage; + roomId?: string; +} +export { MessageEvent_2 as MessageEvent } + +// Warning: (ae-forgotten-export) The symbol "Schemas" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +export type MessageInfo = Schemas["MessageInfo"]; + +// @public +export interface OperationOptions { + abortSignal?: AbortSignalLike; +} + +// @public (undocumented) +export type RoomInfo = Schemas["RoomInfo"]; + +// @public (undocumented) +export type RoomInfoWithMembers = Schemas["RoomInfoWithMembers"]; + +// @public +export interface RoomJoinedEvent { + // (undocumented) + room: RoomInfo; +} + +// @public +export interface RoomLeftEvent { + // (undocumented) + roomId: string; + // (undocumented) + title: string; +} + +// @public +export interface RoomMemberOperationOptions extends OperationOptions { +} + +// @public +export interface SendMessageOptions extends OperationOptions { +} + +// @public +export interface StartOptions extends OperationOptions { +} + +// @public +export interface StopOptions extends OperationOptions { +} + +// @public (undocumented) +export type UserProfile = Schemas["UserProfile"]; + +// (No @packageDocumentation comment for this package) + +``` diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index 6034d227b..c4743ba35 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -76,7 +76,7 @@ class ChatClient { private readonly _emitter = new EventEmitter(); private readonly _rooms = new Map(); - protected _conversationIds = new Set(); + private _conversationIds = new Set(); private _userId: string | undefined; private _isStarted = false; private _startPromise: Promise | undefined; @@ -88,14 +88,14 @@ class ChatClient { * Create a `ChatClient` from a client-access URL. * * The client is constructed but not started; call `start()` (or use - * the {@link ChatClient.start} static factory) to authenticate. + * the static `ChatClient.start(...)` factory) to authenticate. */ constructor(clientAccessUrl: string, options?: WebPubSubClientOptions); /** - * Create a `ChatClient` from a {@link WebPubSubClientCredential}. + * Create a `ChatClient` from a `WebPubSubClientCredential`. * * The client is constructed but not started; call `start()` (or use - * the {@link ChatClient.start} static factory) to authenticate. + * the static `ChatClient.start(...)` factory) to authenticate. */ constructor(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions); /** @@ -575,16 +575,18 @@ class ChatClient { * `chatClient.connection.on("connected", ...)` etc. * * @example + * ```ts * const dispose = client.on("message", (e) => console.log(e.message.content.text)); * // later * dispose(); + * ``` */ public on(event: K, callback: ChatEventListener): Disposable { this._emitter.on(event, callback as any); return () => this._emitter.off(event, callback as any); } - /** Remove a listener previously registered with {@link on}. */ + /** Remove a listener previously registered with `on()`. */ public off(event: K, callback: ChatEventListener): void { this._emitter.off(event, callback as any); } diff --git a/sdk/webpubsub-chat-client/src/options.ts b/sdk/webpubsub-chat-client/src/options.ts index f2dfac216..39a945796 100644 --- a/sdk/webpubsub-chat-client/src/options.ts +++ b/sdk/webpubsub-chat-client/src/options.ts @@ -18,28 +18,28 @@ export interface OperationOptions { abortSignal?: AbortSignalLike; } -/** Options for {@link ChatClient.start}. */ +/** Options for `ChatClient.start()`. */ export interface StartOptions extends OperationOptions {} -/** Options for {@link ChatClient.stop}. */ +/** Options for `ChatClient.stop()`. */ export interface StopOptions extends OperationOptions {} -/** Options for {@link ChatClient.getRoom}. */ +/** Options for `ChatClient.getRoom()`. */ export interface GetRoomOptions extends OperationOptions {} -/** Options for {@link ChatClient.createRoom}. */ +/** Options for `ChatClient.createRoom()`. */ export interface CreateRoomOptions extends OperationOptions {} -/** Options for {@link ChatClient.sendToRoom}. */ +/** Options for `ChatClient.sendToRoom()`. */ export interface SendMessageOptions extends OperationOptions {} -/** Options for {@link ChatClient.getUserInfo}. */ +/** Options for `ChatClient.getUserInfo()`. */ export interface GetUserInfoOptions extends OperationOptions {} -/** Options for {@link ChatClient.addUserToRoom} and {@link ChatClient.removeUserFromRoom}. */ +/** Options for `ChatClient.addUserToRoom()` and `ChatClient.removeUserFromRoom()`. */ export interface RoomMemberOperationOptions extends OperationOptions {} -/** Options for {@link ChatClient.listRoomMessages}. */ +/** Options for `ChatClient.listRoomMessages()`. */ export interface ListRoomMessagesOptions extends OperationOptions { /** Room to list messages from. Must be a room this client has created or joined. */ roomId: string; From d51c3dd8b3db7e4f815234a2cbbbfa172cbca316 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Thu, 28 May 2026 11:56:18 +1000 Subject: [PATCH 03/10] Options-bag cleanup: Options + move optionals in Three orthogonal ergonomics fixes to the public surface, all on the options bags. Aside from two small breaking changes on getRoom / createRoom positional arguments and the pageSize rename, these are mechanical type-level renames. 1. Options-type naming follows Options uniformly - `SendMessageOptions` -> `SendToRoomOptions` - `RoomMemberOperationOptions` -> split into `AddUserToRoomOptions` and `RemoveUserFromRoomOptions` The remaining options types (`StartOptions`, `StopOptions`, `GetRoomOptions`, `CreateRoomOptions`, `GetUserInfoOptions`, `ListRoomMessagesOptions`) already matched the convention. The base `OperationOptions` is unchanged. `sendToConversation` is private; its parameter is typed as the base `OperationOptions` rather than introducing a non-exported `SendToConversationOptions` alias. `manageRoomMember` (private) likewise. 2. Move trailing optional positional parameters into the options bag - `getRoom(roomId, withMembers, options?)` -> `getRoom(roomId, options?: { withMembers?, abortSignal? })` `withMembers` is now optional and defaults to `false` (saves the extra round-trip unless the caller asks). - `createRoom(title, members, roomId?, options?)` -> `createRoom(title, members, options?: { roomId?, abortSignal? })` `roomId` moves into the options bag alongside `abortSignal`. Rationale: a long tail of optional positionals at the end of a signature is hard to read at call sites and forces callers to pass `undefined` placeholders when they only want the last argument. Both fields now live next to `abortSignal`, which is the existing options-bag convention for the rest of the SDK. 3. `ListRoomMessagesOptions.pageSize` -> `maxPageSize` Matches the `@azure/core-paging` `PageSettings.maxPageSize` convention already used by `listRoomMessages(...).byPage({ maxPageSize })`. The same name now applies at both the iterator level (default round-trip size) and the per-page override, so the two knobs stop looking like unrelated concepts. Call-site updates - `tests/integration.test.ts`: every `createRoom(title, members, roomId)` rewritten to `createRoom(title, members, { roomId })`, the single `getRoom(roomId, true)` rewritten to `getRoom(roomId, { withMembers: true })`, and the one `listRoomMessages({ ..., pageSize: 100 })` rewritten to `{ ..., maxPageSize: 100 }`. - Internal `ensureRoomCached` no longer passes the now-removed `withMembers: false` positional - the default does it. - `examples/quickstart/client.js` only used the no-roomId form and does not call `getRoom` / pass `pageSize`, so no change needed. - `README.md`: methods table updated for the new signatures and the `pageSize` -> `maxPageSize` rename. Verification - tsc --noEmit clean. - `tsx --test tests/lifecycle.test.ts`: 6/6 pass. - `api-extractor run --local`: zero warnings; regenerated `review/web-pubsub-chat-client.api.md` reflects exactly the renames + the two field additions (`GetRoomOptions.withMembers?`, `CreateRoomOptions.roomId?`) + `ListRoomMessagesOptions.pageSize?` -> `maxPageSize?`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/README.md | 6 +-- .../review/web-pubsub-chat-client.api.md | 29 +++++++----- sdk/webpubsub-chat-client/src/chatClient.ts | 47 +++++++++++++------ sdk/webpubsub-chat-client/src/index.ts | 5 +- sdk/webpubsub-chat-client/src/options.ts | 38 +++++++++++---- .../tests/integration.test.ts | 18 +++---- 6 files changed, 92 insertions(+), 51 deletions(-) diff --git a/sdk/webpubsub-chat-client/README.md b/sdk/webpubsub-chat-client/README.md index 799a6ee30..80a72eca7 100644 --- a/sdk/webpubsub-chat-client/README.md +++ b/sdk/webpubsub-chat-client/README.md @@ -100,12 +100,12 @@ When constructed from an existing `WebPubSubClient`, `ChatClient` owns that clie |--------|-------------| | `start(options?)` | Connect and authenticate. Idempotent; concurrent calls share one in-flight promise. After `stop()` the client can be started again. Accepts `{ abortSignal }`. | | `stop()` | Disconnect and reset client state. Returns `Promise`. | -| `createRoom(title, members, roomId?, options?)` | Create a new room with initial members. The current user is automatically added to the members list. | -| `getRoom(roomId, withMembers, options?)` | Get room info | +| `createRoom(title, members, options?)` | Create a new room with initial members. The current user is automatically added to the members list. Options: `{ roomId?, abortSignal? }` — supply `roomId` to choose an explicit id, otherwise the service assigns one. | +| `getRoom(roomId, options?)` | Get room info. Options: `{ withMembers?, abortSignal? }` — set `withMembers: true` to populate the members list (extra round-trip). | | `addUserToRoom(roomId, userId, options?)` | Add user to room (admin operation) | | `removeUserFromRoom(roomId, userId, options?)` | Remove user from room (admin operation) | | `sendToRoom(roomId, message, options?)` | Send text message to room, returns message ID | -| `listRoomMessages(options)` | Paged async iterator over room message history (auto-paginates). Use `for await` to stream every message, or `.byPage({ maxPageSize })` to load up to `maxPageSize` messages at a time. `options = { roomId, startId?, endId?, pageSize?, abortSignal? }` | +| `listRoomMessages(options)` | Paged async iterator over room message history (auto-paginates). Use `for await` to stream every message, or `.byPage({ maxPageSize })` to load up to `maxPageSize` messages at a time. `options = { roomId, startId?, endId?, maxPageSize?, abortSignal? }` | | `getUserInfo(userId, options?)` | Get user profile | Every asynchronous method accepts an optional final `options` argument diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md index 98cea879f..50dce985e 100644 --- a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -10,17 +10,20 @@ import { WebPubSubClient } from '@azure/web-pubsub-client'; import { WebPubSubClientCredential } from '@azure/web-pubsub-client'; import { WebPubSubClientOptions } from '@azure/web-pubsub-client'; +// @public +export interface AddUserToRoomOptions extends OperationOptions { +} + // @public (undocumented) export class ChatClient { constructor(clientAccessUrl: string, options?: WebPubSubClientOptions); constructor(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions); constructor(wpsClient: WebPubSubClient); - addUserToRoom(roomId: string, userId: string, options?: RoomMemberOperationOptions): Promise; + addUserToRoom(roomId: string, userId: string, options?: AddUserToRoomOptions): Promise; // (undocumented) readonly connection: WebPubSubClient; - createRoom(title: string, members: string[], roomId?: string, options?: CreateRoomOptions): Promise; - // (undocumented) - getRoom(roomId: string, withMembers: boolean, options?: GetRoomOptions): Promise; + createRoom(title: string, members: string[], options?: CreateRoomOptions): Promise; + getRoom(roomId: string, options?: GetRoomOptions): Promise; // (undocumented) getUserInfo(userId: string, options?: GetUserInfoOptions): Promise; hasJoinedRoom(roomId: string): boolean; @@ -33,10 +36,10 @@ export class ChatClient { onMessage(callback: ChatEventListener<"message">): Disposable_2; onRoomJoined(callback: ChatEventListener<"roomJoined">): Disposable_2; onRoomLeft(callback: ChatEventListener<"roomLeft">): Disposable_2; - removeUserFromRoom(roomId: string, userId: string, options?: RoomMemberOperationOptions): Promise; + removeUserFromRoom(roomId: string, userId: string, options?: RemoveUserFromRoomOptions): Promise; get rooms(): RoomInfo[]; // (undocumented) - sendToRoom(roomId: string, message: string, options?: SendMessageOptions): Promise; + sendToRoom(roomId: string, message: string, options?: SendToRoomOptions): Promise; static start(clientAccessUrl: string, options?: WebPubSubClientOptions): Promise; // (undocumented) static start(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions): Promise; @@ -81,6 +84,7 @@ export interface ChatMessage extends MessageInfo { // @public export interface CreateRoomOptions extends OperationOptions { + roomId?: string; } // @public @@ -89,6 +93,7 @@ export { Disposable_2 as Disposable } // @public export interface GetRoomOptions extends OperationOptions { + withMembers?: boolean; } // @public @@ -108,7 +113,7 @@ export const KnownChatErrorCode: { // @public export interface ListRoomMessagesOptions extends OperationOptions { endId?: string; - pageSize?: number; + maxPageSize?: number; roomId: string; startId?: string; } @@ -151,6 +156,10 @@ export interface OperationOptions { abortSignal?: AbortSignalLike; } +// @public +export interface RemoveUserFromRoomOptions extends OperationOptions { +} + // @public (undocumented) export type RoomInfo = Schemas["RoomInfo"]; @@ -172,11 +181,7 @@ export interface RoomLeftEvent { } // @public -export interface RoomMemberOperationOptions extends OperationOptions { -} - -// @public -export interface SendMessageOptions extends OperationOptions { +export interface SendToRoomOptions extends OperationOptions { } // @public diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index c4743ba35..5baeb911c 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -34,9 +34,10 @@ import type { StopOptions, GetRoomOptions, CreateRoomOptions, - SendMessageOptions, + SendToRoomOptions, GetUserInfoOptions, - RoomMemberOperationOptions, + AddUserToRoomOptions, + RemoveUserFromRoomOptions, } from "./options.js"; import { ERRORS, INVOCATION_NAME } from "./constant.js"; @@ -341,7 +342,7 @@ class ChatClient { private async sendToConversation( conversationId: string, message: string, - options?: SendMessageOptions, + options?: OperationOptions, ): Promise { this.ensureStarted(); const payload = { @@ -383,7 +384,7 @@ class ChatClient { return msgId; } - public async sendToRoom(roomId: string, message: string, options?: SendMessageOptions): Promise { + public async sendToRoom(roomId: string, message: string, options?: SendToRoomOptions): Promise { this.ensureStarted(); const conversationId = this._rooms.get(roomId)?.defaultConversationId; if (!conversationId) { @@ -392,21 +393,37 @@ class ChatClient { return await this.sendToConversation(conversationId, message, options); } - public async getRoom(roomId: string, withMembers: boolean, options?: GetRoomOptions): Promise { + /** + * Fetch the latest service-side view of a room. + * + * @param roomId - Room to query. + * @param options - Optional `{ withMembers, abortSignal }`. When + * `withMembers` is `true` the returned `members` array is + * populated; defaults to `false` to save a round-trip. + */ + public async getRoom(roomId: string, options?: GetRoomOptions): Promise { this.ensureStarted(); return this.invokeWithReturnType( INVOCATION_NAME.GET_ROOM, - { id: roomId, withMembers: withMembers }, + { id: roomId, withMembers: options?.withMembers ?? false }, "json", options, ); } - /** Create a room and its initial members. If `roomId` is not set, the service will create a random one. */ + /** + * Create a room and its initial members. The current user is always + * included in the resulting member list. + * + * @param title - Display title for the room. + * @param members - Other user ids to invite. The caller is added + * automatically; duplicates are de-duplicated. + * @param options - Optional `{ roomId, abortSignal }`. Pass `roomId` + * to choose an id explicitly; omit to let the service assign one. + */ public async createRoom( title: string, members: string[], - roomId?: string, options?: CreateRoomOptions, ): Promise { this.ensureStarted(); @@ -414,8 +431,8 @@ class ChatClient { title: title, members: [...new Set([...members, this.userId])], // deduplicate and add self } as any; - if (roomId) { - roomDetails = { ...roomDetails, roomId: roomId }; + if (options?.roomId) { + roomDetails = { ...roomDetails, roomId: options.roomId }; } const roomInfo = await this.invokeWithReturnType( INVOCATION_NAME.CREATE_ROOM, @@ -431,7 +448,7 @@ class ChatClient { private async manageRoomMember( request: ManageRoomMemberRequest, - options?: RoomMemberOperationOptions, + options?: OperationOptions, ): Promise { await this.invokeWithReturnType(INVOCATION_NAME.MANAGE_ROOM_MEMBER, request, "json", options); } @@ -440,12 +457,12 @@ class ChatClient { if (this._rooms.has(roomId)) { return; } - const roomInfo = await this.getRoom(roomId, false, options); + const roomInfo = await this.getRoom(roomId, options); this._rooms.set(roomId, roomInfo); } /** Add a user to a room. This is an admin operation where one user adds another user to a room. */ - public async addUserToRoom(roomId: string, userId: string, options?: RoomMemberOperationOptions): Promise { + public async addUserToRoom(roomId: string, userId: string, options?: AddUserToRoomOptions): Promise { this.ensureStarted(); const payload: ManageRoomMemberRequest = { roomId: roomId, operation: "Add", userId: userId }; const isSelf = userId === this.userId; @@ -465,7 +482,7 @@ class ChatClient { } /** Remove a user from a room. This is an admin operation where one user removes another user from a room. */ - public async removeUserFromRoom(roomId: string, userId: string, options?: RoomMemberOperationOptions): Promise { + public async removeUserFromRoom(roomId: string, userId: string, options?: RemoveUserFromRoomOptions): Promise { this.ensureStarted(); const payload: ManageRoomMemberRequest = { roomId: roomId, operation: "Delete", userId: userId }; await this.manageRoomMember(payload, options); @@ -515,7 +532,7 @@ class ChatClient { throw new ChatError(`Failed to listRoomMessages, not found roomId ${options.roomId}`, ERRORS.UnknownRoom); } - const defaultPageSize = options.pageSize ?? 100; + const defaultPageSize = options.maxPageSize ?? 100; const firstPageLink: MessageRangeQuery = { conversation: { conversationId }, start: options.startId ?? null, diff --git a/sdk/webpubsub-chat-client/src/index.ts b/sdk/webpubsub-chat-client/src/index.ts index 1d7e1c85b..01a215e3a 100644 --- a/sdk/webpubsub-chat-client/src/index.ts +++ b/sdk/webpubsub-chat-client/src/index.ts @@ -27,9 +27,10 @@ export type { StopOptions, GetRoomOptions, CreateRoomOptions, - SendMessageOptions, + SendToRoomOptions, GetUserInfoOptions, - RoomMemberOperationOptions, + AddUserToRoomOptions, + RemoveUserFromRoomOptions, ListRoomMessagesOptions, } from './options.js'; diff --git a/sdk/webpubsub-chat-client/src/options.ts b/sdk/webpubsub-chat-client/src/options.ts index 39a945796..6d6bbb068 100644 --- a/sdk/webpubsub-chat-client/src/options.ts +++ b/sdk/webpubsub-chat-client/src/options.ts @@ -25,19 +25,36 @@ export interface StartOptions extends OperationOptions {} export interface StopOptions extends OperationOptions {} /** Options for `ChatClient.getRoom()`. */ -export interface GetRoomOptions extends OperationOptions {} +export interface GetRoomOptions extends OperationOptions { + /** + * When `true`, the returned `RoomInfoWithMembers.members` array is + * populated. Defaults to `false`; fetching members is an additional + * service round-trip and is skipped unless asked for. + */ + withMembers?: boolean; +} /** Options for `ChatClient.createRoom()`. */ -export interface CreateRoomOptions extends OperationOptions {} +export interface CreateRoomOptions extends OperationOptions { + /** + * Optional client-chosen room id. If omitted, the service assigns a + * random id. The id must be unique within the hub; reusing an + * existing id rejects with `KnownChatErrorCode.RoomAlreadyExists`. + */ + roomId?: string; +} /** Options for `ChatClient.sendToRoom()`. */ -export interface SendMessageOptions extends OperationOptions {} +export interface SendToRoomOptions extends OperationOptions {} /** Options for `ChatClient.getUserInfo()`. */ export interface GetUserInfoOptions extends OperationOptions {} -/** Options for `ChatClient.addUserToRoom()` and `ChatClient.removeUserFromRoom()`. */ -export interface RoomMemberOperationOptions extends OperationOptions {} +/** Options for `ChatClient.addUserToRoom()`. */ +export interface AddUserToRoomOptions extends OperationOptions {} + +/** Options for `ChatClient.removeUserFromRoom()`. */ +export interface RemoveUserFromRoomOptions extends OperationOptions {} /** Options for `ChatClient.listRoomMessages()`. */ export interface ListRoomMessagesOptions extends OperationOptions { @@ -52,10 +69,11 @@ export interface ListRoomMessagesOptions extends OperationOptions { */ endId?: string; /** - * Maximum number of messages to request per service round-trip when - * iterating with `for await`. Defaults to 100. Callers using - * `byPage(...)` can override this per page via - * `byPage({ maxPageSize })`. + * Default maximum number of messages to request per service + * round-trip when iterating with `for await`. Defaults to 100. + * Callers using `byPage(...)` can override this per page via + * `byPage({ maxPageSize })`; the name matches the + * `@azure/core-paging` `PageSettings.maxPageSize` convention. */ - pageSize?: number; + maxPageSize?: number; } diff --git a/sdk/webpubsub-chat-client/tests/integration.test.ts b/sdk/webpubsub-chat-client/tests/integration.test.ts index 5eabf4ecb..1c5f5542d 100644 --- a/sdk/webpubsub-chat-client/tests/integration.test.ts +++ b/sdk/webpubsub-chat-client/tests/integration.test.ts @@ -38,7 +38,7 @@ test("same user id start twice", { timeout: LONG_TEST_TIMEOUT }, async (t) => { assert.equal(chat1.userId, chat1UserId, `chat1 userId should be '${chat1UserId}'`); const roomName = `room-${Math.floor(Math.random() * 10000)}`; - const createdRoom = await chat0.createRoom(roomName, [chat1.userId], `uid_${roomName}`); + const createdRoom = await chat0.createRoom(roomName, [chat1.userId], { roomId: `uid_${roomName}` }); await chat0.sendToRoom(createdRoom.roomId, `Hello from chat0`); // sleep 100ms await new Promise((resolve) => setTimeout(resolve, 100)); @@ -75,7 +75,7 @@ test("same user on two clients still receives remote room messages", { timeout: admin = await createTestClient(); const roomId = `room-shared-${randomUUID().substring(0, 6)}`; - const createdRoom = await admin.createRoom("ut-shared-room", [sharedUserId], roomId); + const createdRoom = await admin.createRoom("ut-shared-room", [sharedUserId], { roomId }); sender = await createTestClient(sharedUserId); watcher = await createTestClient(sharedUserId); @@ -132,14 +132,14 @@ test("single client", { timeout: SHORT_TEST_TIMEOUT }, async (t) => { assert.ok(chat1.userId && typeof chat1.userId === "string"); const roomId = `room-id-${randomUUID().substring(0, 3)}`; - const created = await chat1.createRoom("ut-single-room", [], roomId); + const created = await chat1.createRoom("ut-single-room", [], { roomId }); assert.equal(created.roomId, roomId, "roomId should match"); assert.equal(created.title, "ut-single-room", "room title should match"); assert.ok(Array.isArray(created.members), "members should be an array"); assert.deepEqual(created.members, [chat1.userId], "members should contain only the creator"); assert.ok(created.members.includes(chat1.userId), "members should include the creator"); - const fetched = await chat1.getRoom(created.roomId, true); + const fetched = await chat1.getRoom(created.roomId, { withMembers: true }); assert.equal(fetched.roomId, created.roomId, "fetched roomId should match created"); assert.equal(fetched.title, created.title, "fetched title should match created"); assert.ok(Array.isArray(fetched.members), "fetched members should be an array"); @@ -176,7 +176,7 @@ test("create room with multiple users", { timeout: LONG_TEST_TIMEOUT }, async (t } const listedMsgs: MessageInfo[] = []; - for await (const message of chats[0].listRoomMessages({ roomId: createdRoom.roomId, startId: "0", pageSize: 100 })) { + for await (const message of chats[0].listRoomMessages({ roomId: createdRoom.roomId, startId: "0", maxPageSize: 100 })) { listedMsgs.push(message); } let listedMsgCount = 0; @@ -282,7 +282,7 @@ test("self remove updates local room cache immediately", { timeout: LONG_TEST_TI chat1 = await createTestClient(); const roomId = `room-leave-${randomUUID().substring(0, 6)}`; - const created = await chat1.createRoom("ut-self-leave", [], roomId); + const created = await chat1.createRoom("ut-self-leave", [], { roomId }); assert.equal(chat1.hasJoinedRoom(created.roomId), true, "room should be cached after creation"); await chat1.removeUserFromRoom(created.roomId, chat1.userId); @@ -302,7 +302,7 @@ test("self add restores local room cache without RoomJoined event", { timeout: L chat1 = await createTestClient(); const roomId = `room-self-add-${randomUUID().substring(0, 6)}`; - const created = await chat1.createRoom("ut-self-add", [], roomId); + const created = await chat1.createRoom("ut-self-add", [], { roomId }); assert.equal(chat1.hasJoinedRoom(created.roomId), true, "room should be cached after creation"); let roomJoinedEvents = 0; @@ -336,7 +336,7 @@ test("adding non-self user already in room throws ChatError with UserAlreadyInRo user1 = await createTestClient(); const roomId = `room-dup-add-${randomUUID().substring(0, 6)}`; - await admin.createRoom("ut-dup-add", [user1.userId], roomId); + await admin.createRoom("ut-dup-add", [user1.userId], { roomId }); // Second add of the same non-self user must throw a ChatError wrapping the server's UserAlreadyInRoom code. let thrown: unknown; @@ -364,7 +364,7 @@ test("removing non-member user throws ChatError with UserNotInRoom code", { time stranger = await createTestClient(); const roomId = `room-rm-nonmember-${randomUUID().substring(0, 6)}`; - await admin.createRoom("ut-rm-nonmember", [], roomId); + await admin.createRoom("ut-rm-nonmember", [], { roomId }); let thrown: unknown; try { From b650e13a09ee113554ebda97474fdc9dd032f7d1 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Thu, 28 May 2026 13:28:25 +1000 Subject: [PATCH 04/10] Clean up event-listener surface: drop Disposable, rename MessageEvent Two orthogonal cleanups on the event-emitter surface, both flagged by the api-extractor report. 1. `MessageEvent` -> `ChatMessageEvent` The local `MessageEvent` collided with the DOM `MessageEvent` interface from `lib.dom.d.ts` (the type of "message" events on `EventTarget` / `WebSocket` / `Window`, a different shape). api-extractor was suffixing ours to `MessageEvent_2` and re-exporting it under the unsuffixed name, which made the report noisy and the autocomplete misleading. Renaming makes the collision go away and makes the type self-describe (a chat message event, not a DOM message event). 2. Drop `Disposable`; all `on*()` listeners return `void` The local `Disposable` collided with the TypeScript 5.2+ built-in `Disposable` interface used by the explicit-resource-management proposal (`{ [Symbol.dispose](): void }`) - a different shape from our callable `() => void`, with all the same api-extractor noise as `MessageEvent`. Rather than rename to something like `Unsubscribe`, we dropped the unsubscribe-by-return-callable pattern entirely and aligned the chat-event API with the EventEmitter-style add/remove used by the underlying `WebPubSubClient`: // node_modules/@azure/web-pubsub-client/.../webPubSubClient.d.ts on(event: "connected", listener: ...): void; off(event: "connected", listener: ...): void; Three reasons: a) The chat `on*()` family already had a paired `off(event, cb)`. Returning an unsubscriber on top of that gave two unsubscribe paths and an implicit "which one is canonical?" choice. b) Users routinely write `client.on(...)` next to `client.connection.on(...)` (connection-lifecycle events live on the underlying client). Having the two surfaces use different idioms - returned callable vs `off(event, cb)` - is a real surprise. Single idiom across both makes the package consistent. c) No internal caller, no test, no example actually captured the returned callable. The pattern existed only as documentation; removing it costs us nothing observable. The chat event API now mirrors the connection event API one-to-one: client.on(event, cb) client.connection.on(event, cb) client.off(event, cb) client.connection.off(event, cb) Surface changes - `src/events.ts`: `MessageEvent` -> `ChatMessageEvent`; `Disposable` type deleted. - `src/chatClient.ts`: `event: MessageEvent` -> `ChatMessageEvent` (two construction sites); `on()` no longer returns a callable (just `this._emitter.on(...)`); all five `on` convenience methods return `void`; JSDoc example updated to the `on()`/`off()` paired form. - `src/index.ts`: drop `Disposable` re-export and the (briefly introduced) `Unsubscribe` rename; export `ChatMessageEvent`. - `README.md`: events-listeners section rewritten around `on(event, cb)` / `off(event, cb)`; events table updated to `ChatMessageEvent`. - `review/web-pubsub-chat-client.api.md`: regenerated; no more `_2` aliases, no more `Unsubscribe` / `Disposable` type, all six `on*()` methods now show `: void`. Tests do not reference any of the renamed / removed names. Verification - tsc --noEmit clean. - `tsx --test tests/lifecycle.test.ts`: 6/6 pass. - `api-extractor run --local`: zero warnings; zero `_2` matches in the regenerated report. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/README.md | 16 ++++--- .../review/web-pubsub-chat-client.api.md | 31 ++++++------- sdk/webpubsub-chat-client/src/chatClient.ts | 43 ++++++++++--------- sdk/webpubsub-chat-client/src/events.ts | 7 +-- sdk/webpubsub-chat-client/src/index.ts | 3 +- 5 files changed, 48 insertions(+), 52 deletions(-) diff --git a/sdk/webpubsub-chat-client/README.md b/sdk/webpubsub-chat-client/README.md index 80a72eca7..d951a04b5 100644 --- a/sdk/webpubsub-chat-client/README.md +++ b/sdk/webpubsub-chat-client/README.md @@ -152,21 +152,23 @@ try { Chat events are subscribed via a single generic `on(event, callback)` API or typed convenience methods (`onMessage`, `onRoomJoined`, ...). All -listener registrations return a `Disposable` (`() => void`) that removes -the listener when called. Use `off(event, callback)` to unsubscribe a -specific callback. +registrations return `void` and are removed by passing the same callback +reference to `off(event, callback)`. The shape matches the underlying +`WebPubSubClient.on/off`, so the connection-lifecycle and chat-event +APIs use the same idiom. ```ts -const dispose = client.on('message', (event) => { +const onMsg = (event) => { console.log(event.message.content.text); -}); +}; +client.on('message', onMsg); // later -dispose(); +client.off('message', onMsg); ``` | Event name | Convenience method | Payload | Description | |------------|-------------------|---------|-------------| -| `message` | `onMessage(cb)` | `MessageEvent` | New message received (or sent by this client). | +| `message` | `onMessage(cb)` | `ChatMessageEvent` | New message received (or sent by this client). | | `roomJoined` | `onRoomJoined(cb)` | `RoomJoinedEvent` | This client joined a room. | | `roomLeft` | `onRoomLeft(cb)` | `RoomLeftEvent` | This client left a room. | | `memberJoined` | `onMemberJoined(cb)` | `MemberJoinedEvent` | Another user joined a room this client is in. | diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md index 50dce985e..87b025bf5 100644 --- a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -30,12 +30,12 @@ export class ChatClient { get isStarted(): boolean; listRoomMessages(options: ListRoomMessagesOptions): PagedAsyncIterableIterator; off(event: K, callback: ChatEventListener): void; - on(event: K, callback: ChatEventListener): Disposable_2; - onMemberJoined(callback: ChatEventListener<"memberJoined">): Disposable_2; - onMemberLeft(callback: ChatEventListener<"memberLeft">): Disposable_2; - onMessage(callback: ChatEventListener<"message">): Disposable_2; - onRoomJoined(callback: ChatEventListener<"roomJoined">): Disposable_2; - onRoomLeft(callback: ChatEventListener<"roomLeft">): Disposable_2; + on(event: K, callback: ChatEventListener): void; + onMemberJoined(callback: ChatEventListener<"memberJoined">): void; + onMemberLeft(callback: ChatEventListener<"memberLeft">): void; + onMessage(callback: ChatEventListener<"message">): void; + onRoomJoined(callback: ChatEventListener<"roomJoined">): void; + onRoomLeft(callback: ChatEventListener<"roomLeft">): void; removeUserFromRoom(roomId: string, userId: string, options?: RemoveUserFromRoomOptions): Promise; get rooms(): RoomInfo[]; // (undocumented) @@ -68,7 +68,7 @@ export interface ChatEventMap { // (undocumented) memberLeft: MemberLeftEvent; // (undocumented) - message: MessageEvent_2; + message: ChatMessageEvent; // (undocumented) roomJoined: RoomJoinedEvent; // (undocumented) @@ -83,13 +83,16 @@ export interface ChatMessage extends MessageInfo { } // @public -export interface CreateRoomOptions extends OperationOptions { +export interface ChatMessageEvent { + conversationId: string; + message: ChatMessage; roomId?: string; } // @public -type Disposable_2 = () => void; -export { Disposable_2 as Disposable } +export interface CreateRoomOptions extends OperationOptions { + roomId?: string; +} // @public export interface GetRoomOptions extends OperationOptions { @@ -138,14 +141,6 @@ export interface MemberLeftEvent { userId: string; } -// @public -interface MessageEvent_2 { - conversationId: string; - message: ChatMessage; - roomId?: string; -} -export { MessageEvent_2 as MessageEvent } - // Warning: (ae-forgotten-export) The symbol "Schemas" needs to be exported by the entry point index.d.ts // // @public (undocumented) diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index 5baeb911c..c5ba05b6a 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -20,10 +20,9 @@ import type { ChatEventListener, ChatEventName, ChatMessage, - Disposable, + ChatMessageEvent, MemberJoinedEvent, MemberLeftEvent, - MessageEvent, RoomJoinedEvent, RoomLeftEvent, } from "./events.js"; @@ -140,7 +139,7 @@ class ChatClient { switch (type) { case "MessageCreated": { const body = data.body as NewMessageNotificationBody; - const event: MessageEvent = { + const event: ChatMessageEvent = { conversationId: body.conversation.conversationId ?? "", roomId: body.conversation.roomId ?? undefined, message: body.message as ChatMessage, @@ -367,7 +366,7 @@ class ChatClient { logger.warning(`Failed to find roomId for conversationId ${conversationId} when sending message.`); } // sender won't receive conversation message via notification mechanism, so emit event here - const event: MessageEvent = { + const event: ChatMessageEvent = { conversationId: conversationId, roomId: roomId, message: { @@ -584,8 +583,12 @@ class ChatClient { } /** - * Subscribe to a chat client event. Returns a disposer that removes the - * listener when called. + * Subscribe to a chat client event. + * + * Matches the underlying `WebPubSubClient.on(event, listener)` + * shape: returns `void`, paired with `off(event, listener)` for + * removal. Pass the same callback reference to `off()` to + * unsubscribe. * * Connection-lifecycle events (`connected`, `disconnected`, `stopped`) * are not exposed here — subscribe via @@ -593,14 +596,14 @@ class ChatClient { * * @example * ```ts - * const dispose = client.on("message", (e) => console.log(e.message.content.text)); + * const onMsg = (e) => console.log(e.message.content.text); + * client.on("message", onMsg); * // later - * dispose(); + * client.off("message", onMsg); * ``` */ - public on(event: K, callback: ChatEventListener): Disposable { + public on(event: K, callback: ChatEventListener): void { this._emitter.on(event, callback as any); - return () => this._emitter.off(event, callback as any); } /** Remove a listener previously registered with `on()`. */ @@ -609,28 +612,28 @@ class ChatClient { } /** Subscribe to new messages (including the sender-side event emitted by `sendToRoom` / `sendToConversation`). */ - public onMessage(callback: ChatEventListener<"message">): Disposable { - return this.on("message", callback); + public onMessage(callback: ChatEventListener<"message">): void { + this.on("message", callback); } /** Subscribe to room-join events for this client (created or invited). */ - public onRoomJoined(callback: ChatEventListener<"roomJoined">): Disposable { - return this.on("roomJoined", callback); + public onRoomJoined(callback: ChatEventListener<"roomJoined">): void { + this.on("roomJoined", callback); } /** Subscribe to events where this client leaves a room. */ - public onRoomLeft(callback: ChatEventListener<"roomLeft">): Disposable { - return this.on("roomLeft", callback); + public onRoomLeft(callback: ChatEventListener<"roomLeft">): void { + this.on("roomLeft", callback); } /** Subscribe to events where another user joins a room this client is in. */ - public onMemberJoined(callback: ChatEventListener<"memberJoined">): Disposable { - return this.on("memberJoined", callback); + public onMemberJoined(callback: ChatEventListener<"memberJoined">): void { + this.on("memberJoined", callback); } /** Subscribe to events where another user leaves a room this client is in. */ - public onMemberLeft(callback: ChatEventListener<"memberLeft">): Disposable { - return this.on("memberLeft", callback); + public onMemberLeft(callback: ChatEventListener<"memberLeft">): void { + this.on("memberLeft", callback); } /** diff --git a/sdk/webpubsub-chat-client/src/events.ts b/sdk/webpubsub-chat-client/src/events.ts index 1ed2e431c..a064630f3 100644 --- a/sdk/webpubsub-chat-client/src/events.ts +++ b/sdk/webpubsub-chat-client/src/events.ts @@ -8,7 +8,7 @@ import type { MessageInfo, RoomInfo } from "./generatedTypes.js"; export interface ChatMessage extends MessageInfo {} /** Payload of the `"message"` event. */ -export interface MessageEvent { +export interface ChatMessageEvent { /** Conversation the message belongs to. */ conversationId: string; /** Room id when the conversation is room-scoped; otherwise undefined. */ @@ -55,7 +55,7 @@ export interface MemberLeftEvent { * type. */ export interface ChatEventMap { - message: MessageEvent; + message: ChatMessageEvent; roomJoined: RoomJoinedEvent; roomLeft: RoomLeftEvent; memberJoined: MemberJoinedEvent; @@ -64,6 +64,3 @@ export interface ChatEventMap { export type ChatEventName = keyof ChatEventMap; export type ChatEventListener = (event: ChatEventMap[K]) => void; - -/** Returned from listener registrations. Call to remove the listener. */ -export type Disposable = () => void; diff --git a/sdk/webpubsub-chat-client/src/index.ts b/sdk/webpubsub-chat-client/src/index.ts index 01a215e3a..18ae88bd8 100644 --- a/sdk/webpubsub-chat-client/src/index.ts +++ b/sdk/webpubsub-chat-client/src/index.ts @@ -13,8 +13,7 @@ export type { ChatEventMap, ChatEventName, ChatEventListener, - Disposable, - MessageEvent, + ChatMessageEvent, RoomJoinedEvent, RoomLeftEvent, MemberJoinedEvent, From e83a97f1a2651027087bf68c60e18a373ea5bbd8 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Thu, 28 May 2026 14:11:39 +1000 Subject: [PATCH 05/10] Plumb StartOptions through static ChatClient.start() The instance `ChatClient.start(options?: StartOptions)` accepts an `abortSignal` and propagates it through `startCore` to `connection.start` and the login/get-room invocations, so a stuck start can be cancelled cleanly. The static `ChatClient.start(...)` helper - which constructs a new `ChatClient` and starts it in one step - was left calling `chatClient.start()` with no arguments, so there was no way to cancel a one-shot construct-and-start. This was an oversight from the original AbortSignal pass (commit `515f5f2`), not a deliberate omission: every other public async method already accepts its `Options` bag, so the static helper diverging here is a small but real inconsistency. Why two parameters instead of an intersection? The first iteration of this fix exposed `options?: WebPubSubClientOptions & StartOptions` on the URL / credential overloads, so a single bag could carry both transport config and a cancellation token. That works today but is fragile: - `WebPubSubClientOptions` ships from a separate package (`@azure/web-pubsub-client`) on a separate release cadence. If it ever gains a field that overlaps with `StartOptions` (`abortSignal` being the obvious one), the intersection silently changes shape - in the worst case, a field's type narrows to the upstream variant and breaks callers passing the polyfilled type. - Upstream itself draws this exact line: `WebPubSubClientOptions` holds transport config (protocol, autoReconnect, retry options, keep-alive intervals) while a separate `StartOptions` interface in the same package holds `abortSignal`. Our intersection deliberately blurred a boundary upstream chose to draw. `WebPubSubClientOptions` has too many genuinely useful fields (`autoReconnect`, `autoRejoinGroups`, `messageRetryOptions`, `reconnectRetryOptions`, keep-alive intervals, `protocol`) to drop silently from the static helper, so the fix is to give it its own positional parameter on the URL / credential overloads, slotted in between the connect target and the start options: static start(clientAccessUrl, webPubSubClientOptions?: WebPubSubClientOptions, options?: StartOptions): Promise static start(credential, webPubSubClientOptions?: WebPubSubClientOptions, options?: StartOptions): Promise static start(wpsClient, options?: StartOptions): Promise The `wpsClient` overload only takes `StartOptions` since transport construction has already happened; passing a `WebPubSubClient` that's already configured replaces the `webPubSubClientOptions` parameter. Implementation forwards `abortSignal` explicitly to `chatClient.start({ abortSignal })` rather than the whole options bag, so future StartOptions-only fields are picked up cleanly without leaking unrelated fields into the start path. README: the static-methods table annotates each row with the new parameter shape (`webPubSubClientOptions?: WebPubSubClientOptions`, `options?: StartOptions`) so readers know exactly what each positional means. Verification - tsc --noEmit clean. - lifecycle.test.ts: 6/6 pass. - api-extractor: zero warnings; regenerated report shows the URL / credential overloads with `webPubSubClientOptions?` and `options?: StartOptions` as separate parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/README.md | 6 +- .../review/web-pubsub-chat-client.api.md | 6 +- sdk/webpubsub-chat-client/src/chatClient.ts | 64 +++++++++++++++---- 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/sdk/webpubsub-chat-client/README.md b/sdk/webpubsub-chat-client/README.md index d951a04b5..7702edff1 100644 --- a/sdk/webpubsub-chat-client/README.md +++ b/sdk/webpubsub-chat-client/README.md @@ -81,9 +81,9 @@ When constructed from an existing `WebPubSubClient`, `ChatClient` owns that clie | Method | Description | |--------|-------------| -| `ChatClient.start(clientAccessUrl, options?)` | Create a client and start it in one step | -| `ChatClient.start(credential, options?)` | Create a client from a credential and start it | -| `ChatClient.start(wpsClient)` | Create a client from an existing `WebPubSubClient` and start it | +| `ChatClient.start(clientAccessUrl, webPubSubClientOptions?, options?)` | Construct from a URL and start (`webPubSubClientOptions?: WebPubSubClientOptions`, `options?: StartOptions`) | +| `ChatClient.start(credential, webPubSubClientOptions?, options?)` | Construct from a credential and start (same option shape as above) | +| `ChatClient.start(wpsClient, options?)` | Start a pre-constructed `WebPubSubClient` (`options?: StartOptions`) | #### Properties diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md index 87b025bf5..792e6a687 100644 --- a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -40,11 +40,11 @@ export class ChatClient { get rooms(): RoomInfo[]; // (undocumented) sendToRoom(roomId: string, message: string, options?: SendToRoomOptions): Promise; - static start(clientAccessUrl: string, options?: WebPubSubClientOptions): Promise; + static start(clientAccessUrl: string, webPubSubClientOptions?: WebPubSubClientOptions, options?: StartOptions): Promise; // (undocumented) - static start(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions): Promise; + static start(credential: WebPubSubClientCredential, webPubSubClientOptions?: WebPubSubClientOptions, options?: StartOptions): Promise; // (undocumented) - static start(wpsClient: WebPubSubClient): Promise; + static start(wpsClient: WebPubSubClient, options?: StartOptions): Promise; start(options?: StartOptions): Promise; stop(_options?: StopOptions): Promise; // (undocumented) diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index c5ba05b6a..72c468b2a 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -224,22 +224,64 @@ class ChatClient { /** * Create a chat client and `start()` it in one step. * - * Overloads accept the same arguments as the constructor. The - * returned promise resolves to a started client. + * Construction options for the underlying transport and the cancellation + * token for the start operation are kept as **separate parameters** so + * they never collide at the type level. `webPubSubClientOptions` is the + * full `WebPubSubClientOptions` bag (`protocol`, `autoReconnect`, + * `reconnectRetryOptions`, keep-alive intervals, ...). `options` is the + * same {@link StartOptions} the instance `start()` accepts. + * + * ```ts + * // Most callers only need the URL. + * const chat = await ChatClient.start(url); + * + * // Customise the transport, then start. + * const chat = await ChatClient.start( + * url, + * { autoReconnect: false, reconnectRetryOptions: { maxRetries: 5 } }, + * { abortSignal }, + * ); + * + * // Already have a WebPubSubClient? Hand it in directly. + * const chat = await ChatClient.start(wpsClient, { abortSignal }); + * ``` + * + * Why two parameters instead of an intersection? `WebPubSubClientOptions` + * ships from a separate package (`@azure/web-pubsub-client`) on its own + * release cadence, so intersecting it with our `StartOptions` would be + * fragile against upstream field additions (most obviously `abortSignal`). + * Keeping the two bags positional preserves the full set of + * construction knobs without exposing the intersection. */ - public static async start(clientAccessUrl: string, options?: WebPubSubClientOptions): Promise; - public static async start(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions): Promise; - public static async start(wpsClient: WebPubSubClient): Promise; - public static async start(arg1: string | WebPubSubClientCredential | WebPubSubClient, options?: WebPubSubClientOptions): Promise { + public static async start( + clientAccessUrl: string, + webPubSubClientOptions?: WebPubSubClientOptions, + options?: StartOptions, + ): Promise; + public static async start( + credential: WebPubSubClientCredential, + webPubSubClientOptions?: WebPubSubClientOptions, + options?: StartOptions, + ): Promise; + public static async start(wpsClient: WebPubSubClient, options?: StartOptions): Promise; + public static async start( + arg1: string | WebPubSubClientCredential | WebPubSubClient, + arg2?: WebPubSubClientOptions | StartOptions, + arg3?: StartOptions, + ): Promise { let chatClient: ChatClient; - if (typeof arg1 === "string") { - chatClient = new ChatClient(arg1, options); - } else if (isWebPubSubClient(arg1)) { + let startOptions: StartOptions | undefined; + if (isWebPubSubClient(arg1)) { chatClient = new ChatClient(arg1); + startOptions = arg2 as StartOptions | undefined; + } else if (typeof arg1 === "string") { + chatClient = new ChatClient(arg1, arg2 as WebPubSubClientOptions | undefined); + startOptions = arg3; } else { - chatClient = new ChatClient(arg1, options); + chatClient = new ChatClient(arg1, arg2 as WebPubSubClientOptions | undefined); + startOptions = arg3; } - await chatClient.start(); + await chatClient.start({ abortSignal: startOptions?.abortSignal }); return chatClient; } From e6dea280d704b83a6c4c73baecb5ffeffc81b724 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Thu, 28 May 2026 14:31:15 +1000 Subject: [PATCH 06/10] Tighten ChatMessageEvent: drop conversationId, require roomId PR 1 has progressively pulled every public mention of the "conversation" concept below the room abstraction: - `sendToConversation()` was made `private` in `515f5f2`. - The `_conversationIds` set on `ChatClient` was made `private` (was leaking via `protected`) in `a9e31a5`. - The `LOGIN` invocation's `conversationIds` array stays internal state; nothing in the public surface references it. This commit finishes the job on the last remaining public reference - `ChatMessageEvent` - by dropping `conversationId` outright and tightening `roomId` from optional to required. Drop `conversationId` From the user's mental model the conversation isn't a thing they manage: they create / join rooms, they send to rooms, they list room messages. The `roomId` on the event already tells them everything they can act on. The `conversationId` is a wire-protocol detail that we resolve to a room internally before invoking server events; leaking it on the event surface forces clients to either ignore it (most of them) or build their own resolver against it (none of them do today). The drop was originally tagged in the earlier session plan as "deferred to model PR" because it sat in the same bucket as the rest of the conversation-concept hiding work. But that work finished in `515f5f2` / `a9e31a5`; the only remaining `ChatMessageEvent` quality fix that genuinely needs the model rewrite is the sender-echo synthesis (the `bodyType: "text"` hardcode and the `createdAt` fakery), which touches the `ChatMessage` shape. Dropping the field is purely subtractive and has zero coupling to that work. Tighten `roomId` from `roomId?: string` to `roomId: string` The previous version was `roomId?: string` with the doc string "Undefined for non-room conversations (currently unused)". Two problems with that: - "non-room conversations" reintroduces the very concept the drop above is trying to hide. The doc was leaking through the back door what the type change had just closed off the front. - "currently unused" is hand-wavy. If a case is unused, the field shouldn't be modelled as optional at all - that is just defensive `undefined`-everywhere coding masquerading as protocol nuance. In practice `roomId` can only be undefined in two anomalous paths: a. Notification path. The wire `body.conversation.roomId` is `string | null` per the generated types, but in practice always populated today - the null is a future-protocol reservation, not a real case. b. Sender-echo path. `_rooms` is consulted via reverse lookup (`defaultConversationId -> roomId`) immediately after the forward lookup in `sendToRoom`. A miss is only possible if a `RoomLeft` notification arrives during the in-flight invoke call, leaving the room out of the cache. Narrow race window. For both cases the fix is the same: log a warning and skip the emit. Subscribers get a tight contract (every `ChatMessageEvent` has a `roomId` they can act on) and the anomalous cases are contained inside the SDK rather than pushed onto every caller as a `roomId?` they have to null-check defensively. In the sender-echo path the function still returns the `msgId` - the send itself succeeded server-side; only the local echo is suppressed. Suppressing is the right call: emitting a "message in room you just left" event would be more confusing than silence. Changes - `src/events.ts`: drop `conversationId: string;` from `ChatMessageEvent`; change `roomId?: string;` to `roomId: string;`. - `src/chatClient.ts`: * Notification path (`MessageCreated`): guard `body.conversation.roomId`, warn-and-skip if missing. * Sender-echo path (`sendToConversation`): the existing warning for failed reverse lookup now returns early instead of proceeding with a `roomId: undefined` event. Send result is still returned. - `review/web-pubsub-chat-client.api.md`: regenerated; one field removed, one optional turned required. Verification - tsc --noEmit clean. - lifecycle.test.ts: 6/6 pass. - api-extractor: zero warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../review/web-pubsub-chat-client.api.md | 3 +-- sdk/webpubsub-chat-client/src/chatClient.ts | 17 ++++++++++++----- sdk/webpubsub-chat-client/src/events.ts | 6 ++---- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md index 792e6a687..edb392676 100644 --- a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -84,9 +84,8 @@ export interface ChatMessage extends MessageInfo { // @public export interface ChatMessageEvent { - conversationId: string; message: ChatMessage; - roomId?: string; + roomId: string; } // @public diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index 72c468b2a..a5d33e660 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -139,9 +139,14 @@ class ChatClient { switch (type) { case "MessageCreated": { const body = data.body as NewMessageNotificationBody; + if (!body.conversation.roomId) { + logger.warning( + `MessageCreated notification missing roomId; skipping emit. conversationId=${body.conversation.conversationId}`, + ); + break; + } const event: ChatMessageEvent = { - conversationId: body.conversation.conversationId ?? "", - roomId: body.conversation.roomId ?? undefined, + roomId: body.conversation.roomId, message: body.message as ChatMessage, }; this._emitter.emit("message", event); @@ -405,12 +410,14 @@ class ChatClient { const msgId = resp.id; const roomId = Array.from(this._rooms.values()).find((r) => r.defaultConversationId === conversationId)?.roomId; if (!roomId) { - logger.warning(`Failed to find roomId for conversationId ${conversationId} when sending message.`); + logger.warning( + `Failed to find roomId for conversationId ${conversationId} when sending message; skipping local sender-echo emit.`, + ); + return msgId; } // sender won't receive conversation message via notification mechanism, so emit event here const event: ChatMessageEvent = { - conversationId: conversationId, - roomId: roomId, + roomId, message: { messageId: msgId, createdBy: this.userId, diff --git a/sdk/webpubsub-chat-client/src/events.ts b/sdk/webpubsub-chat-client/src/events.ts index a064630f3..d9f729346 100644 --- a/sdk/webpubsub-chat-client/src/events.ts +++ b/sdk/webpubsub-chat-client/src/events.ts @@ -9,10 +9,8 @@ export interface ChatMessage extends MessageInfo {} /** Payload of the `"message"` event. */ export interface ChatMessageEvent { - /** Conversation the message belongs to. */ - conversationId: string; - /** Room id when the conversation is room-scoped; otherwise undefined. */ - roomId?: string; + /** Room the message belongs to. */ + roomId: string; /** The message. */ message: ChatMessage; } From f55b7a8abc77fa9b45f919a735e4da53624ace32 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Thu, 28 May 2026 16:30:40 +1000 Subject: [PATCH 07/10] chat-client: privatize isStarted; rely on ensureStarted() exception path The public isStarted getter was a thin wrapper over the private _isStarted backing field, intended as a "is it safe to call methods?" check. In practice it adds no value and a little risk: - TOCTOU race. `if (client.isStarted) { await client.getRoom(...) }` gives a false sense of safety: a stop() from another caller or a network drop between the check and the call still throws. The check cannot be made meaningful without a lock that the SDK does not own. - Redundant with the exception path. Every operation method already calls ensureStarted() and throws ChatError(NotStarted). Callers must handle that exception anyway, so the proactive check pays no rent. - No sibling precedent. The underlying WebPubSubClient exposes no equivalent state-check getter; it is exception-driven throughout. - Zero internal callers. All in-package references already read this._isStarted directly. userId is left as-is: it is a getter-only property, which TypeScript renders to consumers as effectively `readonly userId: string` (no setter exists; assignment is a compile error). The lazy throw-on-not- started guard inside the getter is preferable to a `string | undefined` field, since it forces callers to call start() first via a clear runtime error rather than requiring null-check at every call site. Changes: - Drop the public `isStarted` getter from ChatClient (the private `_isStarted` field is unchanged). - Update lifecycle tests to assert on the internal flag via a small typed helper `isStarted(client)`. These are own-package smoke tests exercising the state machine; reaching past `private` here keeps the test intent intact without re-exposing the API on the public surface. - Drop the `isStarted` row from the README properties table; reword the `userId` row to note it is read-only. - Regenerate review/web-pubsub-chat-client.api.md (one-line removal). Verified: tsc clean, 6/6 lifecycle tests pass, api-extractor report regenerated without warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/README.md | 3 +-- .../review/web-pubsub-chat-client.api.md | 1 - sdk/webpubsub-chat-client/src/chatClient.ts | 5 ----- .../tests/lifecycle.test.ts | 21 ++++++++++++------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/sdk/webpubsub-chat-client/README.md b/sdk/webpubsub-chat-client/README.md index 7702edff1..4a24eab37 100644 --- a/sdk/webpubsub-chat-client/README.md +++ b/sdk/webpubsub-chat-client/README.md @@ -89,8 +89,7 @@ When constructed from an existing `WebPubSubClient`, `ChatClient` owns that clie | Property | Type | Description | |----------|------|-------------| -| `userId` | `string` | Current user's ID (throws if not started) | -| `isStarted` | `boolean` | `true` once `start()` has completed and `stop()` has not been called since | +| `userId` | `string` | Current user's ID (throws if not started). Read-only — set internally on `start()`. | | `rooms` | `RoomInfo[]` | Snapshot of currently joined rooms (not live-updated) | | `connection` | `WebPubSubClient` | Underlying WebPubSub connection owned by this chat client | diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md index edb392676..f3279bb82 100644 --- a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -27,7 +27,6 @@ export class ChatClient { // (undocumented) getUserInfo(userId: string, options?: GetUserInfoOptions): Promise; hasJoinedRoom(roomId: string): boolean; - get isStarted(): boolean; listRoomMessages(options: ListRoomMessagesOptions): PagedAsyncIterableIterator; off(event: K, callback: ChatEventListener): void; on(event: K, callback: ChatEventListener): void; diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index a5d33e660..29e9404af 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -364,11 +364,6 @@ class ChatClient { } } - /** Whether `start()` has completed successfully and `stop()` has not been called since. */ - public get isStarted(): boolean { - return this._isStarted; - } - private ensureStarted(): void { if (!this._isStarted) { throw new ChatError("Not started. Please call start() first.", ERRORS.NotStarted); diff --git a/sdk/webpubsub-chat-client/tests/lifecycle.test.ts b/sdk/webpubsub-chat-client/tests/lifecycle.test.ts index f74ce0fde..eb0dbcd71 100644 --- a/sdk/webpubsub-chat-client/tests/lifecycle.test.ts +++ b/sdk/webpubsub-chat-client/tests/lifecycle.test.ts @@ -6,6 +6,13 @@ import { ChatClient } from "../src/chatClient.js"; import { INVOCATION_NAME } from "../src/constant.js"; import type { RoomInfoWithMembers, UserProfile } from "../src/generatedTypes.js"; +/** + * `_isStarted` is private; accessor used by tests to assert the post-login + * state-machine flag without re-exposing it on the public surface. + */ +const isStarted = (client: ChatClient): boolean => + (client as unknown as { _isStarted: boolean })._isStarted; + class Deferred { public promise: Promise; public resolve!: (value: T | PromiseLike) => void; @@ -124,7 +131,7 @@ test("stop before start is a no-op", async () => { await client.stop(); - assert.equal(client.isStarted, false); + assert.equal(isStarted(client), false); assert.equal(fakeClient.stopCalls, 0); }); @@ -148,13 +155,13 @@ test("concurrent start calls wait for room hydration", async () => { }); await Promise.resolve(); - assert.equal(client.isStarted, false, "client should not be marked started until room hydration completes"); + assert.equal(isStarted(client), false, "client should not be marked started until room hydration completes"); assert.equal(secondStartResolved, false, "second start should wait for the in-flight start promise"); fakeClient.getRoomDelay.resolve(); await Promise.all([firstStart, secondStart]); - assert.equal(client.isStarted, true); + assert.equal(isStarted(client), true); assert.deepEqual(client.rooms.map((roomInfo) => roomInfo.roomId), ["room1"]); assert.equal(fakeClient.startCalls, 1); }); @@ -167,7 +174,7 @@ test("failed start rolls back chat state and stops the connection", async () => await assert.rejects(client.start(), /get room failed/); - assert.equal(client.isStarted, false); + assert.equal(isStarted(client), false); assert.throws(() => client.userId, /start\(\)/); assert.deepEqual(client.rooms, []); assert.equal(fakeClient.stopCalls, 1); @@ -195,7 +202,7 @@ test("stop waits for stopped event before allowing restart", async () => { await Promise.all([stopPromise, restartPromise]); assert.equal(stopResolved, true); - assert.equal(client.isStarted, true); + assert.equal(isStarted(client), true); assert.equal(fakeClient.startCalls, 2); assert.equal(fakeClient.stopCalls, 1); }); @@ -225,7 +232,7 @@ test("concurrent stop calls wait for the same stopped event", async () => { assert.equal(firstStopResolved, true); assert.equal(secondStopResolved, true); - assert.equal(client.isStarted, false); + assert.equal(isStarted(client), false); }); test("concurrent start calls during stop share a single restart", async () => { @@ -246,7 +253,7 @@ test("concurrent start calls during stop share a single restart", async () => { fakeClient.stopDelay.resolve(); await Promise.all([stopPromise, firstRestart, secondRestart]); - assert.equal(client.isStarted, true); + assert.equal(isStarted(client), true); assert.equal(fakeClient.startCalls, 2, "restart should call connection.start() exactly once"); assert.equal(fakeClient.stopCalls, 1); }); \ No newline at end of file From 9990c8a243f5d4b5bf2133de810b0b6c5917cbe9 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Thu, 28 May 2026 16:44:22 +1000 Subject: [PATCH 08/10] chat-client: mirror WebPubSubClient on/off shape (overloads per event, OnArgs, kebab-case) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align the ChatClient event surface with the underlying @azure/web-pubsub-client so the two sit cleanly side by side and the two `client.on(...)` calls (chat-domain vs connection-lifecycle) feel identical. Before: on(event: K, callback: ChatEventListener): void; off(event: K, callback: ChatEventListener): void; onMessage(cb), onRoomJoined(cb), onRoomLeft(cb), onMemberJoined(cb), onMemberLeft(cb) // 3 supporting types + 5 *Event payload types exported After (matches `WebPubSubClient.on(event: "server-message", listener: (e: OnServerDataMessageArgs) => void)`): on(event: "message", listener: (e: OnMessageArgs) => void): void; on(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; on(event: "room-left", listener: (e: OnRoomLeftArgs) => void): void; on(event: "member-joined", listener: (e: OnMemberJoinedArgs) => void): void; on(event: "member-left", listener: (e: OnMemberLeftArgs) => void): void; // + same 5 overloads of off(...) Three coupled changes: 1. Explicit overloads replace the generic indexed-map shape. Drops `ChatEventMap`, `ChatEventName`, `ChatEventListener` from the public surface — these only existed to back the generic. 2. Event payload types renamed to `OnArgs` to mirror upstream (`OnConnectedArgs`, `OnGroupDataMessageArgs`, ...): ChatMessageEvent -> OnMessageArgs RoomJoinedEvent -> OnRoomJoinedArgs RoomLeftEvent -> OnRoomLeftArgs MemberJoinedEvent -> OnMemberJoinedArgs MemberLeftEvent -> OnMemberLeftArgs 3. Event-name string literals switch from camelCase to kebab-case so they read identically alongside `client.connection.on("server- message", ...)`. `"message"` is unchanged; the rest become `"room-joined"`, `"room-left"`, `"member-joined"`, `"member-left"`. 4. Convenience helpers (`onMessage(cb)`, `onRoomJoined(cb)`, etc.) removed. Upstream has no equivalent, and they had an asymmetric- removal smell — there was never a paired `offMessage(cb)`, so a caller still had to use `off("message", cb)` to unregister. Net result: one entry point per event in each direction, all listed explicitly in the api report. Updated all internal emit() sites in chatClient.ts to use the new kebab-case names and the renamed argument types. README event-listener section reworked to drop the convenience-method column. Integration tests migrated from `chat.onMessage(cb)` / `chat.onRoomJoined(cb)` to `chat.on("message", cb)` / `chat.on("room-joined", cb)`. Verified: tsc clean, 6/6 lifecycle tests pass, api-extractor report regenerated without warnings; all 10 on/off overloads (5 each) and the 5 OnXArgs interfaces appear cleanly in review/web-pubsub-chat- client.api.md, with `ChatEventMap` / `ChatEventName` / `ChatEventListener` gone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/README.md | 28 +++--- .../review/web-pubsub-chat-client.api.md | 97 +++++++++---------- sdk/webpubsub-chat-client/src/chatClient.ts | 95 ++++++++---------- sdk/webpubsub-chat-client/src/events.ts | 47 +++------ sdk/webpubsub-chat-client/src/index.ts | 13 +-- .../tests/integration.test.ts | 16 +-- 6 files changed, 123 insertions(+), 173 deletions(-) diff --git a/sdk/webpubsub-chat-client/README.md b/sdk/webpubsub-chat-client/README.md index 4a24eab37..56f74ae31 100644 --- a/sdk/webpubsub-chat-client/README.md +++ b/sdk/webpubsub-chat-client/README.md @@ -30,12 +30,12 @@ const client = await ChatClient.start(wpsClient); console.log(`Started as: ${client.userId}`); // Listen for events -client.onMessage((event) => { +client.on('message', (event) => { const msg = event.message; console.log(`${msg.createdBy}: ${msg.content.text}`); }); -client.onRoomJoined((event) => { +client.on('room-joined', (event) => { console.log(`Joined room: ${event.room.title}`); }); @@ -149,12 +149,10 @@ try { #### Event Listeners -Chat events are subscribed via a single generic `on(event, callback)` API -or typed convenience methods (`onMessage`, `onRoomJoined`, ...). All -registrations return `void` and are removed by passing the same callback -reference to `off(event, callback)`. The shape matches the underlying -`WebPubSubClient.on/off`, so the connection-lifecycle and chat-event -APIs use the same idiom. +Chat events use the same shape as the underlying `WebPubSubClient`: +one `on(event, listener)` overload per event, returning `void`, paired +with `off(event, listener)` for removal. Pass the same callback +reference to `off()` to unregister. ```ts const onMsg = (event) => { @@ -165,13 +163,13 @@ client.on('message', onMsg); client.off('message', onMsg); ``` -| Event name | Convenience method | Payload | Description | -|------------|-------------------|---------|-------------| -| `message` | `onMessage(cb)` | `ChatMessageEvent` | New message received (or sent by this client). | -| `roomJoined` | `onRoomJoined(cb)` | `RoomJoinedEvent` | This client joined a room. | -| `roomLeft` | `onRoomLeft(cb)` | `RoomLeftEvent` | This client left a room. | -| `memberJoined` | `onMemberJoined(cb)` | `MemberJoinedEvent` | Another user joined a room this client is in. | -| `memberLeft` | `onMemberLeft(cb)` | `MemberLeftEvent` | Another user left a room this client is in. | +| Event name | Listener argument | Description | +|------------|-------------------|-------------| +| `message` | `OnMessageArgs` | New message received (or sent by this client). | +| `room-joined` | `OnRoomJoinedArgs` | This client joined a room. | +| `room-left` | `OnRoomLeftArgs` | This client left a room. | +| `member-joined` | `OnMemberJoinedArgs` | Another user joined a room this client is in. | +| `member-left` | `OnMemberLeftArgs` | Another user left a room this client is in. | Connection-lifecycle events (`connected`, `disconnected`, `stopped`) live on the underlying `WebPubSubClient`. Subscribe through `client.connection`: diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md index f3279bb82..0d4470d4b 100644 --- a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -28,13 +28,24 @@ export class ChatClient { getUserInfo(userId: string, options?: GetUserInfoOptions): Promise; hasJoinedRoom(roomId: string): boolean; listRoomMessages(options: ListRoomMessagesOptions): PagedAsyncIterableIterator; - off(event: K, callback: ChatEventListener): void; - on(event: K, callback: ChatEventListener): void; - onMemberJoined(callback: ChatEventListener<"memberJoined">): void; - onMemberLeft(callback: ChatEventListener<"memberLeft">): void; - onMessage(callback: ChatEventListener<"message">): void; - onRoomJoined(callback: ChatEventListener<"roomJoined">): void; - onRoomLeft(callback: ChatEventListener<"roomLeft">): void; + off(event: "message", listener: (e: OnMessageArgs) => void): void; + // (undocumented) + off(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; + // (undocumented) + off(event: "room-left", listener: (e: OnRoomLeftArgs) => void): void; + // (undocumented) + off(event: "member-joined", listener: (e: OnMemberJoinedArgs) => void): void; + // (undocumented) + off(event: "member-left", listener: (e: OnMemberLeftArgs) => void): void; + on(event: "message", listener: (e: OnMessageArgs) => void): void; + // (undocumented) + on(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; + // (undocumented) + on(event: "room-left", listener: (e: OnRoomLeftArgs) => void): void; + // (undocumented) + on(event: "member-joined", listener: (e: OnMemberJoinedArgs) => void): void; + // (undocumented) + on(event: "member-left", listener: (e: OnMemberLeftArgs) => void): void; removeUserFromRoom(roomId: string, userId: string, options?: RemoveUserFromRoomOptions): Promise; get rooms(): RoomInfo[]; // (undocumented) @@ -57,36 +68,10 @@ export class ChatError extends Error { readonly code: string; } -// @public (undocumented) -export type ChatEventListener = (event: ChatEventMap[K]) => void; - -// @public -export interface ChatEventMap { - // (undocumented) - memberJoined: MemberJoinedEvent; - // (undocumented) - memberLeft: MemberLeftEvent; - // (undocumented) - message: ChatMessageEvent; - // (undocumented) - roomJoined: RoomJoinedEvent; - // (undocumented) - roomLeft: RoomLeftEvent; -} - -// @public (undocumented) -export type ChatEventName = keyof ChatEventMap; - // @public export interface ChatMessage extends MessageInfo { } -// @public -export interface ChatMessageEvent { - message: ChatMessage; - roomId: string; -} - // @public export interface CreateRoomOptions extends OperationOptions { roomId?: string; @@ -119,8 +104,13 @@ export interface ListRoomMessagesOptions extends OperationOptions { startId?: string; } +// Warning: (ae-forgotten-export) The symbol "Schemas" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +export type MessageInfo = Schemas["MessageInfo"]; + // @public -export interface MemberJoinedEvent { +export interface OnMemberJoinedArgs { // (undocumented) roomId: string; // (undocumented) @@ -130,7 +120,7 @@ export interface MemberJoinedEvent { } // @public -export interface MemberLeftEvent { +export interface OnMemberLeftArgs { // (undocumented) roomId: string; // (undocumented) @@ -139,10 +129,25 @@ export interface MemberLeftEvent { userId: string; } -// Warning: (ae-forgotten-export) The symbol "Schemas" needs to be exported by the entry point index.d.ts -// -// @public (undocumented) -export type MessageInfo = Schemas["MessageInfo"]; +// @public +export interface OnMessageArgs { + message: ChatMessage; + roomId: string; +} + +// @public +export interface OnRoomJoinedArgs { + // (undocumented) + room: RoomInfo; +} + +// @public +export interface OnRoomLeftArgs { + // (undocumented) + roomId: string; + // (undocumented) + title: string; +} // @public export interface OperationOptions { @@ -159,20 +164,6 @@ export type RoomInfo = Schemas["RoomInfo"]; // @public (undocumented) export type RoomInfoWithMembers = Schemas["RoomInfoWithMembers"]; -// @public -export interface RoomJoinedEvent { - // (undocumented) - room: RoomInfo; -} - -// @public -export interface RoomLeftEvent { - // (undocumented) - roomId: string; - // (undocumented) - title: string; -} - // @public export interface SendToRoomOptions extends OperationOptions { } diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index 29e9404af..49e0cd058 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -17,14 +17,12 @@ import { RoomLeftNotificationBody, } from "./generatedTypes.js"; import type { - ChatEventListener, - ChatEventName, ChatMessage, - ChatMessageEvent, - MemberJoinedEvent, - MemberLeftEvent, - RoomJoinedEvent, - RoomLeftEvent, + OnMemberJoinedArgs, + OnMemberLeftArgs, + OnMessageArgs, + OnRoomJoinedArgs, + OnRoomLeftArgs, } from "./events.js"; import type { ListRoomMessagesOptions, @@ -145,7 +143,7 @@ class ChatClient { ); break; } - const event: ChatMessageEvent = { + const event: OnMessageArgs = { roomId: body.conversation.roomId, message: body.message as ChatMessage, }; @@ -156,21 +154,21 @@ class ChatClient { const roomInfo = data.body as NewRoomNotificationBody as RoomInfo; // Add to _rooms first so listeners can use listRoomMessages this._rooms.set(roomInfo.roomId, roomInfo); - const event: RoomJoinedEvent = { room: roomInfo }; - this._emitter.emit("roomJoined", event); + const event: OnRoomJoinedArgs = { room: roomInfo }; + this._emitter.emit("room-joined", event); break; } case "RoomMemberJoined": { const body = data.body as MemberJoinedNotificationBody; - const event: MemberJoinedEvent = { roomId: body.roomId, title: body.title, userId: body.userId }; - this._emitter.emit("memberJoined", event); + const event: OnMemberJoinedArgs = { roomId: body.roomId, title: body.title, userId: body.userId }; + this._emitter.emit("member-joined", event); break; } // someone (not self) left a specific room case "RoomMemberLeft": { const body = data.body as MemberLeftNotificationBody; - const event: MemberLeftEvent = { roomId: body.roomId, title: body.title, userId: body.userId }; - this._emitter.emit("memberLeft", event); + const event: OnMemberLeftArgs = { roomId: body.roomId, title: body.title, userId: body.userId }; + this._emitter.emit("member-left", event); break; } // self left a specific room @@ -179,8 +177,8 @@ class ChatClient { if (!this._rooms.has(body.roomId)) { break; } - const event: RoomLeftEvent = { roomId: body.roomId, title: body.title }; - this._emitter.emit("roomLeft", event); + const event: OnRoomLeftArgs = { roomId: body.roomId, title: body.title }; + this._emitter.emit("room-left", event); this._rooms.delete(body.roomId); break; } @@ -411,7 +409,7 @@ class ChatClient { return msgId; } // sender won't receive conversation message via notification mechanism, so emit event here - const event: ChatMessageEvent = { + const event: OnMessageArgs = { roomId, message: { messageId: msgId, @@ -484,8 +482,8 @@ class ChatClient { options, ); this._rooms.set(roomInfo.roomId, roomInfo); - const event: RoomJoinedEvent = { room: roomInfo }; - this._emitter.emit("roomJoined", event); + const event: OnRoomJoinedArgs = { room: roomInfo }; + this._emitter.emit("room-joined", event); return roomInfo; } @@ -535,8 +533,8 @@ class ChatClient { const roomInfo = this._rooms.get(roomId); if (roomInfo) { this._rooms.delete(roomId); - const event: RoomLeftEvent = { roomId, title: roomInfo.title }; - this._emitter.emit("roomLeft", event); + const event: OnRoomLeftArgs = { roomId, title: roomInfo.title }; + this._emitter.emit("room-left", event); } } } @@ -627,12 +625,12 @@ class ChatClient { } /** - * Subscribe to a chat client event. + * Subscribe to a chat-client event. * - * Matches the underlying `WebPubSubClient.on(event, listener)` - * shape: returns `void`, paired with `off(event, listener)` for - * removal. Pass the same callback reference to `off()` to - * unsubscribe. + * Mirrors the underlying `WebPubSubClient.on(event, listener)` shape: + * one explicit overload per event, returns `void`, paired with + * `off(event, listener)` for removal. Pass the same callback + * reference to `off()` to unsubscribe. * * Connection-lifecycle events (`connected`, `disconnected`, `stopped`) * are not exposed here — subscribe via @@ -640,44 +638,29 @@ class ChatClient { * * @example * ```ts - * const onMsg = (e) => console.log(e.message.content.text); + * const onMsg = (e: OnMessageArgs) => console.log(e.message.content.text); * client.on("message", onMsg); * // later * client.off("message", onMsg); * ``` */ - public on(event: K, callback: ChatEventListener): void { - this._emitter.on(event, callback as any); + public on(event: "message", listener: (e: OnMessageArgs) => void): void; + public on(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; + public on(event: "room-left", listener: (e: OnRoomLeftArgs) => void): void; + public on(event: "member-joined", listener: (e: OnMemberJoinedArgs) => void): void; + public on(event: "member-left", listener: (e: OnMemberLeftArgs) => void): void; + public on(event: string, listener: (e: any) => void): void { + this._emitter.on(event, listener); } /** Remove a listener previously registered with `on()`. */ - public off(event: K, callback: ChatEventListener): void { - this._emitter.off(event, callback as any); - } - - /** Subscribe to new messages (including the sender-side event emitted by `sendToRoom` / `sendToConversation`). */ - public onMessage(callback: ChatEventListener<"message">): void { - this.on("message", callback); - } - - /** Subscribe to room-join events for this client (created or invited). */ - public onRoomJoined(callback: ChatEventListener<"roomJoined">): void { - this.on("roomJoined", callback); - } - - /** Subscribe to events where this client leaves a room. */ - public onRoomLeft(callback: ChatEventListener<"roomLeft">): void { - this.on("roomLeft", callback); - } - - /** Subscribe to events where another user joins a room this client is in. */ - public onMemberJoined(callback: ChatEventListener<"memberJoined">): void { - this.on("memberJoined", callback); - } - - /** Subscribe to events where another user leaves a room this client is in. */ - public onMemberLeft(callback: ChatEventListener<"memberLeft">): void { - this.on("memberLeft", callback); + public off(event: "message", listener: (e: OnMessageArgs) => void): void; + public off(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; + public off(event: "room-left", listener: (e: OnRoomLeftArgs) => void): void; + public off(event: "member-joined", listener: (e: OnMemberJoinedArgs) => void): void; + public off(event: "member-left", listener: (e: OnMemberLeftArgs) => void): void; + public off(event: string, listener: (e: any) => void): void { + this._emitter.off(event, listener); } /** diff --git a/sdk/webpubsub-chat-client/src/events.ts b/sdk/webpubsub-chat-client/src/events.ts index d9f729346..ab1b830f9 100644 --- a/sdk/webpubsub-chat-client/src/events.ts +++ b/sdk/webpubsub-chat-client/src/events.ts @@ -7,58 +7,39 @@ import type { MessageInfo, RoomInfo } from "./generatedTypes.js"; */ export interface ChatMessage extends MessageInfo {} -/** Payload of the `"message"` event. */ -export interface ChatMessageEvent { +/** + * Argument of the `"message"` event listener. Naming mirrors the + * `OnArgs` convention used by the underlying `WebPubSubClient` + * (e.g. `OnGroupDataMessageArgs`). + */ +export interface OnMessageArgs { /** Room the message belongs to. */ roomId: string; /** The message. */ message: ChatMessage; } -/** Payload of the `"roomJoined"` event. Fired when the current client joins a room. */ -export interface RoomJoinedEvent { +/** Argument of the `"room-joined"` event listener. Fired when the current client joins a room. */ +export interface OnRoomJoinedArgs { room: RoomInfo; } -/** Payload of the `"roomLeft"` event. Fired when the current client leaves a room. */ -export interface RoomLeftEvent { +/** Argument of the `"room-left"` event listener. Fired when the current client leaves a room. */ +export interface OnRoomLeftArgs { roomId: string; title: string; } -/** Payload of the `"memberJoined"` event. Fired when another user joins a room this client is in. */ -export interface MemberJoinedEvent { +/** Argument of the `"member-joined"` event listener. Fired when another user joins a room this client is in. */ +export interface OnMemberJoinedArgs { roomId: string; title: string; userId: string; } -/** Payload of the `"memberLeft"` event. Fired when another user leaves a room this client is in. */ -export interface MemberLeftEvent { +/** Argument of the `"member-left"` event listener. Fired when another user leaves a room this client is in. */ +export interface OnMemberLeftArgs { roomId: string; title: string; userId: string; } - -/** - * Map of all chat-domain `ChatClient` events to their payload types. - * - * Connection-lifecycle events (`connected`, `disconnected`, `stopped`) - * live on the underlying transport and are subscribed via - * `chatClient.connection.on("connected", ...)` etc. - * - * Used by the generic `ChatClient.on` / `ChatClient.off` overloads for - * type narrowing. Convenience methods (`onMessage`, `onRoomJoined`, ...) - * are thin wrappers over `on(...)` and accept the corresponding payload - * type. - */ -export interface ChatEventMap { - message: ChatMessageEvent; - roomJoined: RoomJoinedEvent; - roomLeft: RoomLeftEvent; - memberJoined: MemberJoinedEvent; - memberLeft: MemberLeftEvent; -} - -export type ChatEventName = keyof ChatEventMap; -export type ChatEventListener = (event: ChatEventMap[K]) => void; diff --git a/sdk/webpubsub-chat-client/src/index.ts b/sdk/webpubsub-chat-client/src/index.ts index 18ae88bd8..691e25c77 100644 --- a/sdk/webpubsub-chat-client/src/index.ts +++ b/sdk/webpubsub-chat-client/src/index.ts @@ -10,14 +10,11 @@ export type { export type { ChatMessage, - ChatEventMap, - ChatEventName, - ChatEventListener, - ChatMessageEvent, - RoomJoinedEvent, - RoomLeftEvent, - MemberJoinedEvent, - MemberLeftEvent, + OnMessageArgs, + OnRoomJoinedArgs, + OnRoomLeftArgs, + OnMemberJoinedArgs, + OnMemberLeftArgs, } from './events.js'; export type { diff --git a/sdk/webpubsub-chat-client/tests/integration.test.ts b/sdk/webpubsub-chat-client/tests/integration.test.ts index 1c5f5542d..21a28f545 100644 --- a/sdk/webpubsub-chat-client/tests/integration.test.ts +++ b/sdk/webpubsub-chat-client/tests/integration.test.ts @@ -32,7 +32,7 @@ test("same user id start twice", { timeout: LONG_TEST_TIMEOUT }, async (t) => { chat1 = await createTestClient(); const chat1UserId = chat1.userId; let messageReceived = 0; - chat1.onMessage((notification) => { + chat1.on("message", (notification) => { messageReceived++; }); assert.equal(chat1.userId, chat1UserId, `chat1 userId should be '${chat1UserId}'`); @@ -49,7 +49,7 @@ test("same user id start twice", { timeout: LONG_TEST_TIMEOUT }, async (t) => { // second start with same userId chat1 = await createTestClient(chat1UserId); messageReceived = 0; - chat1.onMessage((notification) => { + chat1.on("message", (notification) => { messageReceived++; }); assert.equal(chat1.userId, chat1UserId, `chat1 userId should still be '${chat1UserId}' after restart`); @@ -85,10 +85,10 @@ test("same user on two clients still receives remote room messages", { timeout: const senderNotifications: any[] = []; const watcherNotifications: any[] = []; - sender.onMessage((notification) => { + sender.on("message", (notification) => { senderNotifications.push(notification); }); - watcher.onMessage((notification) => { + watcher.on("message", (notification) => { watcherNotifications.push(notification); }); @@ -160,10 +160,10 @@ test("create room with multiple users", { timeout: LONG_TEST_TIMEOUT }, async (t receivedMsgCounts = [0, 0, 0]; for (let i = 0; i < 3; i++) { - chats[i].onRoomJoined((event) => { + chats[i].on("room-joined", (event) => { joinedRoomCounts[i]++; }); - chats[i].onMessage((message) => { + chats[i].on("message", (message) => { receivedMsgCounts[i]++; }); } @@ -235,7 +235,7 @@ test("admin adds multiple users to a group", { timeout: LONG_TEST_TIMEOUT }, asy let messageReceivedCounts = new Array(chats.length).fill(0); chats.forEach((chat, index) => { - chat.onMessage((notification) => { + chat.on("message", (notification) => { messageReceivedCounts[index]++; }); }); @@ -306,7 +306,7 @@ test("self add restores local room cache without RoomJoined event", { timeout: L assert.equal(chat1.hasJoinedRoom(created.roomId), true, "room should be cached after creation"); let roomJoinedEvents = 0; - chat1.onRoomJoined((event) => { + chat1.on("room-joined", (event) => { if (event.room.roomId === created.roomId) { roomJoinedEvents += 1; } From 7a65f3fa48e984377b68461712f8303e188cf331 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Thu, 28 May 2026 16:51:27 +1000 Subject: [PATCH 09/10] chat-client: collapse to single (wpsClient) constructor; add started/stopped chat events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coupled API-shape changes: 1. Drop the URL and credential constructor overloads. The static `ChatClient.start(url, ...)` / `start(credential, ...)` factories already construct the underlying `WebPubSubClient` and atomically start the chat client, so the URL/credential constructors were a footgun: they left callers with a half-constructed `ChatClient` that threw `ChatError(NotStarted)` on every method call until the instance `start()` was awaited. After this change: new ChatClient(wpsClient: WebPubSubClient) // only constructor ChatClient.start(url, wpsOpts?, startOpts?): Promise<...> // builds & starts ChatClient.start(credential, wpsOpts?, startOpts?): Promise<...> // builds & starts ChatClient.start(wpsClient, startOpts?): Promise<...> // starts existing Internally the static URL/credential branches now `new WebPubSubClient(...)` first and pass the result to `new ChatClient(wpsClient)`, so there is exactly one transport construction path. 2. Add chat-domain "started" and "stopped" events. The pre-existing `client.connection.on("connected", ...)` / `connection.on("stopped", ...)` events fire at the transport boundary and miss the chat- domain transition: `connected` fires before LOGIN + room hydration complete, so a UI that flips to a "ready" state on `connected` will still hit `ChatError(NotStarted)` on operations. The new events fire on the chat-domain transition: on(event: "started", listener: (e: OnStartedArgs) => void): void; on(event: "stopped", listener: (e: OnStoppedArgs) => void): void; `OnStartedArgs` carries `{ userId }` — mirrors the upstream `OnConnectedArgs.userId` pattern and lets handlers consume the chat-domain identity without re-reading `client.userId`. `OnStoppedArgs` is empty, matching the upstream `OnStoppedArgs` shape exactly. Emission rules (single transition guarantee): - "started" fires exactly once at the end of a successful `startCore()`, after `_isStarted` has been set to true and `_userId` / `_rooms` are live. Re-calling `start()` on an already- started client is a no-op and does not re-emit. - "stopped" fires from inside `resetState()`, guarded on a local `wasStarted = this._isStarted` snapshot taken on entry. This guarantees one emission per started→not-started transition, covering both explicit `stop()` and transport-driven termination (the `connection.on("stopped")` callback we install in the constructor). The reset paths that run before `_isStarted` was ever set to true (the pre-start `resetState()` at the top of `startCore()` and the post-failure rollback) do not emit. Tests: - Two new lifecycle.test.ts cases exercise the emission rules: "started event fires once after start() completes with userId payload" (covers single fire + payload shape + no re-emit on no-op re-start), and "stopped event fires on started→not-started transitions only" (covers stop()-before-start() no-emit, explicit stop() emit, and transport-driven stop emit). - All 8 lifecycle tests pass; tsc clean; api-extractor report regenerated cleanly. The public surface now shows a single `constructor(wpsClient: WebPubSubClient)` and 7 `on`/`off` overloads each (5 chat events + started + stopped). README updated: - Constructor section reduced to a single signature with a pointer to `ChatClient.start(...)` for URL/credential construction. - Event listeners table gains `started` (`OnStartedArgs`) and `stopped` (`OnStoppedArgs`) rows at the top. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/README.md | 13 ++-- .../review/web-pubsub-chat-client.api.md | 19 +++++- sdk/webpubsub-chat-client/src/chatClient.ts | 63 ++++++++++--------- sdk/webpubsub-chat-client/src/events.ts | 21 +++++++ sdk/webpubsub-chat-client/src/index.ts | 2 + .../tests/lifecycle.test.ts | 42 +++++++++++++ 6 files changed, 120 insertions(+), 40 deletions(-) diff --git a/sdk/webpubsub-chat-client/README.md b/sdk/webpubsub-chat-client/README.md index 56f74ae31..3830cdf17 100644 --- a/sdk/webpubsub-chat-client/README.md +++ b/sdk/webpubsub-chat-client/README.md @@ -63,19 +63,12 @@ await client.stop(); #### Constructor ```typescript -// With existing WebPubSubClient new ChatClient(wpsClient: WebPubSubClient) - -// With client access URL -new ChatClient(clientAccessUrl: string, options?: WebPubSubClientOptions) - -// With credential -new ChatClient(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions) ``` -To construct and start in one step, prefer the static `ChatClient.start(...)` factory below. +`ChatClient` always wraps a pre-constructed `WebPubSubClient` and owns its lifecycle: `start()` starts the transport, `stop()` stops it. -When constructed from an existing `WebPubSubClient`, `ChatClient` owns that client's lifecycle: `start()` starts it and `stop()` stops it. +To construct from a client-access URL or `WebPubSubClientCredential`, use the static `ChatClient.start(...)` factory below — it builds the underlying `WebPubSubClient` and starts the chat client atomically, so callers never observe a half-constructed instance. #### Static Methods @@ -165,6 +158,8 @@ client.off('message', onMsg); | Event name | Listener argument | Description | |------------|-------------------|-------------| +| `started` | `OnStartedArgs` | `start()` completed successfully — `userId` and `rooms` are populated. | +| `stopped` | `OnStoppedArgs` | The client transitioned to not-started (explicit `stop()` or transport-driven). | | `message` | `OnMessageArgs` | New message received (or sent by this client). | | `room-joined` | `OnRoomJoinedArgs` | This client joined a room. | | `room-left` | `OnRoomLeftArgs` | This client left a room. | diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md index 0d4470d4b..8f1da0e5f 100644 --- a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -16,8 +16,6 @@ export interface AddUserToRoomOptions extends OperationOptions { // @public (undocumented) export class ChatClient { - constructor(clientAccessUrl: string, options?: WebPubSubClientOptions); - constructor(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions); constructor(wpsClient: WebPubSubClient); addUserToRoom(roomId: string, userId: string, options?: AddUserToRoomOptions): Promise; // (undocumented) @@ -28,6 +26,10 @@ export class ChatClient { getUserInfo(userId: string, options?: GetUserInfoOptions): Promise; hasJoinedRoom(roomId: string): boolean; listRoomMessages(options: ListRoomMessagesOptions): PagedAsyncIterableIterator; + off(event: "started", listener: (e: OnStartedArgs) => void): void; + // (undocumented) + off(event: "stopped", listener: (e: OnStoppedArgs) => void): void; + // (undocumented) off(event: "message", listener: (e: OnMessageArgs) => void): void; // (undocumented) off(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; @@ -37,6 +39,10 @@ export class ChatClient { off(event: "member-joined", listener: (e: OnMemberJoinedArgs) => void): void; // (undocumented) off(event: "member-left", listener: (e: OnMemberLeftArgs) => void): void; + on(event: "started", listener: (e: OnStartedArgs) => void): void; + // (undocumented) + on(event: "stopped", listener: (e: OnStoppedArgs) => void): void; + // (undocumented) on(event: "message", listener: (e: OnMessageArgs) => void): void; // (undocumented) on(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; @@ -149,6 +155,15 @@ export interface OnRoomLeftArgs { title: string; } +// @public +export interface OnStartedArgs { + userId: string; +} + +// @public +export interface OnStoppedArgs { +} + // @public export interface OperationOptions { abortSignal?: AbortSignalLike; diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index 49e0cd058..f73b3611d 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -23,6 +23,8 @@ import type { OnMessageArgs, OnRoomJoinedArgs, OnRoomLeftArgs, + OnStartedArgs, + OnStoppedArgs, } from "./events.js"; import type { ListRoomMessagesOptions, @@ -83,36 +85,20 @@ class ChatClient { private _isConnectionStopping = false; /** - * Create a `ChatClient` from a client-access URL. + * Create a `ChatClient` that wraps an existing `WebPubSubClient`. * - * The client is constructed but not started; call `start()` (or use - * the static `ChatClient.start(...)` factory) to authenticate. - */ - constructor(clientAccessUrl: string, options?: WebPubSubClientOptions); - /** - * Create a `ChatClient` from a `WebPubSubClientCredential`. + * `ChatClient` owns the transport's lifecycle: `start()` starts it + * and `stop()` stops it. The chat client is constructed but not + * started; call `start()` to authenticate, or use the static + * `ChatClient.start(...)` factory to construct-and-start in one step. * - * The client is constructed but not started; call `start()` (or use - * the static `ChatClient.start(...)` factory) to authenticate. + * To construct from a client-access URL or `WebPubSubClientCredential`, + * use the corresponding `ChatClient.start(...)` overload — it builds + * the underlying `WebPubSubClient` and starts the chat client + * atomically, so callers never observe a half-constructed instance. */ - constructor(credential: WebPubSubClientCredential, options?: WebPubSubClientOptions); - /** - * Create a `ChatClient` that reuses an existing `WebPubSubClient`. - * - * Passing an existing client gives `ChatClient` lifecycle ownership: - * `start()` starts it and `stop()` stops it. The client is constructed - * but not started; call `start()` to authenticate. - */ - constructor(wpsClient: WebPubSubClient); - - constructor(arg1: string | WebPubSubClientCredential | WebPubSubClient, options?: WebPubSubClientOptions) { - if (isWebPubSubClient(arg1)) { - this.connection = arg1; - } else if (typeof arg1 === "string") { - this.connection = new WebPubSubClient(arg1, options); - } else { - this.connection = new WebPubSubClient(arg1, options); - } + constructor(wpsClient: WebPubSubClient) { + this.connection = wpsClient; this.connection.on("group-message", (e) => { this._handleNotification(e.message.data as Notification); }); @@ -278,10 +264,12 @@ class ChatClient { chatClient = new ChatClient(arg1); startOptions = arg2 as StartOptions | undefined; } else if (typeof arg1 === "string") { - chatClient = new ChatClient(arg1, arg2 as WebPubSubClientOptions | undefined); + const wpsClient = new WebPubSubClient(arg1, arg2 as WebPubSubClientOptions | undefined); + chatClient = new ChatClient(wpsClient); startOptions = arg3; } else { - chatClient = new ChatClient(arg1, arg2 as WebPubSubClientOptions | undefined); + const wpsClient = new WebPubSubClient(arg1, arg2 as WebPubSubClientOptions | undefined); + chatClient = new ChatClient(wpsClient); startOptions = arg3; } await chatClient.start({ abortSignal: startOptions?.abortSignal }); @@ -355,6 +343,8 @@ class ChatClient { this._rooms.set(roomId, roomInfo); }); this._isStarted = true; + const startedEvent: OnStartedArgs = { userId: loginResponse.userId }; + this._emitter.emit("started", startedEvent); } catch (err) { this.resetState(); await this.stopConnection(); @@ -644,6 +634,8 @@ class ChatClient { * client.off("message", onMsg); * ``` */ + public on(event: "started", listener: (e: OnStartedArgs) => void): void; + public on(event: "stopped", listener: (e: OnStoppedArgs) => void): void; public on(event: "message", listener: (e: OnMessageArgs) => void): void; public on(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; public on(event: "room-left", listener: (e: OnRoomLeftArgs) => void): void; @@ -654,6 +646,8 @@ class ChatClient { } /** Remove a listener previously registered with `on()`. */ + public off(event: "started", listener: (e: OnStartedArgs) => void): void; + public off(event: "stopped", listener: (e: OnStoppedArgs) => void): void; public off(event: "message", listener: (e: OnMessageArgs) => void): void; public off(event: "room-joined", listener: (e: OnRoomJoinedArgs) => void): void; public off(event: "room-left", listener: (e: OnRoomLeftArgs) => void): void; @@ -686,11 +680,22 @@ class ChatClient { await this.stopConnection(); } + /** + * Reset chat-domain state. Emits `"stopped"` exactly once per + * started → not-started transition: if `_isStarted` was already + * false on entry (e.g. the pre-start guard inside `startCore()` or + * the post-failure rollback), no event fires. + */ private resetState(): void { + const wasStarted = this._isStarted; this._isStarted = false; this._userId = undefined; this._rooms.clear(); this._conversationIds.clear(); + if (wasStarted) { + const stoppedEvent: OnStoppedArgs = {}; + this._emitter.emit("stopped", stoppedEvent); + } } private async stopConnection(): Promise { diff --git a/sdk/webpubsub-chat-client/src/events.ts b/sdk/webpubsub-chat-client/src/events.ts index ab1b830f9..17a27c786 100644 --- a/sdk/webpubsub-chat-client/src/events.ts +++ b/sdk/webpubsub-chat-client/src/events.ts @@ -7,6 +7,27 @@ import type { MessageInfo, RoomInfo } from "./generatedTypes.js"; */ export interface ChatMessage extends MessageInfo {} +/** + * Argument of the `"started"` event listener. Fired after `start()` + * completes successfully — the underlying connection is open, chat- + * domain login has resolved, and `client.userId` / `client.rooms` are + * populated. Mirrors the `OnArgs` shape used by the underlying + * `WebPubSubClient` (e.g. `OnConnectedArgs`). + */ +export interface OnStartedArgs { + /** The chat-domain identity of this client. Equivalent to `client.userId`. */ + userId: string; +} + +/** + * Argument of the `"stopped"` event listener. Fired when the chat + * client transitions from started to not-started — either because + * `stop()` was called or the underlying connection terminated. Empty + * payload (reserved for future fields), matching upstream + * `WebPubSubClient.OnStoppedArgs`. + */ +export interface OnStoppedArgs {} + /** * Argument of the `"message"` event listener. Naming mirrors the * `OnArgs` convention used by the underlying `WebPubSubClient` diff --git a/sdk/webpubsub-chat-client/src/index.ts b/sdk/webpubsub-chat-client/src/index.ts index 691e25c77..6cb2783d2 100644 --- a/sdk/webpubsub-chat-client/src/index.ts +++ b/sdk/webpubsub-chat-client/src/index.ts @@ -15,6 +15,8 @@ export type { OnRoomLeftArgs, OnMemberJoinedArgs, OnMemberLeftArgs, + OnStartedArgs, + OnStoppedArgs, } from './events.js'; export type { diff --git a/sdk/webpubsub-chat-client/tests/lifecycle.test.ts b/sdk/webpubsub-chat-client/tests/lifecycle.test.ts index eb0dbcd71..1bdf221ce 100644 --- a/sdk/webpubsub-chat-client/tests/lifecycle.test.ts +++ b/sdk/webpubsub-chat-client/tests/lifecycle.test.ts @@ -256,4 +256,46 @@ test("concurrent start calls during stop share a single restart", async () => { assert.equal(isStarted(client), true); assert.equal(fakeClient.startCalls, 2, "restart should call connection.start() exactly once"); assert.equal(fakeClient.stopCalls, 1); +}); + +test("started event fires once after start() completes with userId payload", async () => { + const fakeClient = new FakeWebPubSubClient(); + fakeClient.loginResponse = { userId: "alice", roomIds: [], conversationIds: [] }; + const client = createClient(fakeClient); + + const startedEvents: Array<{ userId: string }> = []; + client.on("started", (e) => startedEvents.push(e)); + + await client.start(); + assert.equal(startedEvents.length, 1, "started should fire exactly once on successful start"); + assert.deepEqual(startedEvents[0], { userId: "alice" }, "started payload should carry the chat-domain userId"); + assert.equal(client.userId, "alice", "userId getter should be live by the time started fires"); + + // Re-starting an already-started client must not re-emit. + await client.start(); + assert.equal(startedEvents.length, 1, "started should not fire when start() is a no-op on an already-started client"); +}); + +test("stopped event fires on started→not-started transitions only", async () => { + const fakeClient = new FakeWebPubSubClient(); + const client = createClient(fakeClient); + + const stoppedEvents: unknown[] = []; + client.on("stopped", (e) => stoppedEvents.push(e)); + + // stop() before start() must not emit (no transition). + await client.stop(); + assert.equal(stoppedEvents.length, 0, "stopped should not fire when stop() runs on a never-started client"); + + // start → stop fires exactly once. + await client.start(); + await client.stop(); + assert.equal(stoppedEvents.length, 1, "stopped should fire exactly once per explicit stop()"); + + // restart → network-driven stop also fires once. + await client.start(); + fakeClient.stop(); + // Allow the queued microtask that emits the underlying "stopped" to flush. + await new Promise((resolve) => setTimeout(resolve, 0)); + assert.equal(stoppedEvents.length, 2, "stopped should fire when the transport terminates after a successful start"); }); \ No newline at end of file From 424f2035b0e7d7c4d6b9e70a4e2f9e9b94ded2a1 Mon Sep 17 00:00:00 2001 From: "Liangying.Wei" Date: Tue, 9 Jun 2026 15:18:53 +1000 Subject: [PATCH 10/10] chat-client: API polish (getUserProfile rename, positional listRoomMessages roomId, drop StopOptions) - Rename getUserInfo -> getUserProfile and GetUserInfoOptions -> GetUserProfileOptions so the method name matches its UserProfile return type (resolves PR #892 review comment). The generated UserProfile type is left untouched to stay aligned with the OpenAPI schema. - listRoomMessages(options) -> listRoomMessages(roomId, options?); the required roomId is now a positional parameter (consistent with the other room-scoped methods) and is removed from ListRoomMessagesOptions. - Remove StopOptions and the stop() options parameter. The underlying WebPubSubClient.stop() is not cancellable, so the advertised abortSignal could never be honored; dropping it avoids a misleading API surface. - Regenerate the API report and update README, quickstart example, and integration tests accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- sdk/webpubsub-chat-client/README.md | 8 +- .../examples/quickstart/client.js | 4 +- .../review/web-pubsub-chat-client.api.md | 266 +++++++++++++++++- sdk/webpubsub-chat-client/src/chatClient.ts | 35 ++- sdk/webpubsub-chat-client/src/index.ts | 5 +- sdk/webpubsub-chat-client/src/options.ts | 11 +- .../tests/integration.test.ts | 6 +- 7 files changed, 286 insertions(+), 49 deletions(-) diff --git a/sdk/webpubsub-chat-client/README.md b/sdk/webpubsub-chat-client/README.md index 3830cdf17..1d9929329 100644 --- a/sdk/webpubsub-chat-client/README.md +++ b/sdk/webpubsub-chat-client/README.md @@ -44,7 +44,7 @@ const room = await client.createRoom('My Room', ['bob']); await client.sendToRoom(room.roomId, 'Hello!'); // Get message history (auto-paginating async iterator) -for await (const msg of client.listRoomMessages({ roomId: room.roomId })) { +for await (const msg of client.listRoomMessages(room.roomId)) { console.log(`${msg.createdBy}: ${msg.content.text}`); } @@ -93,12 +93,12 @@ To construct from a client-access URL or `WebPubSubClientCredential`, use the st | `start(options?)` | Connect and authenticate. Idempotent; concurrent calls share one in-flight promise. After `stop()` the client can be started again. Accepts `{ abortSignal }`. | | `stop()` | Disconnect and reset client state. Returns `Promise`. | | `createRoom(title, members, options?)` | Create a new room with initial members. The current user is automatically added to the members list. Options: `{ roomId?, abortSignal? }` — supply `roomId` to choose an explicit id, otherwise the service assigns one. | -| `getRoom(roomId, options?)` | Get room info. Options: `{ withMembers?, abortSignal? }` — set `withMembers: true` to populate the members list (extra round-trip). | +| `getRoomDetail(roomId, options?)` | Get room info. Options: `{ withMembers?, abortSignal? }` — set `withMembers: true` to populate the members list (extra round-trip). | | `addUserToRoom(roomId, userId, options?)` | Add user to room (admin operation) | | `removeUserFromRoom(roomId, userId, options?)` | Remove user from room (admin operation) | | `sendToRoom(roomId, message, options?)` | Send text message to room, returns message ID | -| `listRoomMessages(options)` | Paged async iterator over room message history (auto-paginates). Use `for await` to stream every message, or `.byPage({ maxPageSize })` to load up to `maxPageSize` messages at a time. `options = { roomId, startId?, endId?, maxPageSize?, abortSignal? }` | -| `getUserInfo(userId, options?)` | Get user profile | +| `listRoomMessages(roomId, options?)` | Paged async iterator over room message history (auto-paginates). Use `for await` to stream every message, or `.byPage({ maxPageSize })` to load up to `maxPageSize` messages at a time. `options = { startId?, endId?, maxPageSize?, abortSignal? }` | +| `getUserProfile(userId, options?)` | Get user profile | Every asynchronous method accepts an optional final `options` argument extending `OperationOptions` (`{ abortSignal?: AbortSignalLike }`) to diff --git a/sdk/webpubsub-chat-client/examples/quickstart/client.js b/sdk/webpubsub-chat-client/examples/quickstart/client.js index 41541abea..b51ec0105 100644 --- a/sdk/webpubsub-chat-client/examples/quickstart/client.js +++ b/sdk/webpubsub-chat-client/examples/quickstart/client.js @@ -77,7 +77,7 @@ async function main() { // List message history (auto-paginating async iterator) console.log('\n--- Message History ---'); - for await (const msg of alice.listRoomMessages({ roomId: room.roomId })) { + for await (const msg of alice.listRoomMessages(room.roomId)) { console.log(` [${msg.createdBy}] [${msg.createdAt}] ${msg.content.text}`); } @@ -85,7 +85,7 @@ async function main() { // `byPage` lets the caller decide when to load the next batch — handy // for "load 50 latest, then 50 more on scroll-up" UI patterns. console.log('\n--- Message History (pages of 3) ---'); - const pages = alice.listRoomMessages({ roomId: room.roomId }).byPage({ maxPageSize: 3 }); + const pages = alice.listRoomMessages(room.roomId).byPage({ maxPageSize: 3 }); let pageNum = 0; while (true) { const { value, done } = await pages.next(); diff --git a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md index 8f1da0e5f..2cc579e99 100644 --- a/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md +++ b/sdk/webpubsub-chat-client/review/web-pubsub-chat-client.api.md @@ -21,11 +21,11 @@ export class ChatClient { // (undocumented) readonly connection: WebPubSubClient; createRoom(title: string, members: string[], options?: CreateRoomOptions): Promise; - getRoom(roomId: string, options?: GetRoomOptions): Promise; + getRoomDetail(roomId: string, options?: GetRoomOptions): Promise; // (undocumented) - getUserInfo(userId: string, options?: GetUserInfoOptions): Promise; + getUserProfile(userId: string, options?: GetUserProfileOptions): Promise; hasJoinedRoom(roomId: string): boolean; - listRoomMessages(options: ListRoomMessagesOptions): PagedAsyncIterableIterator; + listRoomMessages(roomId: string, options?: ListRoomMessagesOptions): PagedAsyncIterableIterator; off(event: "started", listener: (e: OnStartedArgs) => void): void; // (undocumented) off(event: "stopped", listener: (e: OnStoppedArgs) => void): void; @@ -62,7 +62,7 @@ export class ChatClient { // (undocumented) static start(wpsClient: WebPubSubClient, options?: StartOptions): Promise; start(options?: StartOptions): Promise; - stop(_options?: StopOptions): Promise; + stop(): Promise; // (undocumented) get userId(): string; } @@ -78,6 +78,252 @@ export class ChatError extends Error { export interface ChatMessage extends MessageInfo { } +// @public (undocumented) +export interface components { + // (undocumented) + headers: never; + // (undocumented) + parameters: never; + // (undocumented) + pathItems: never; + // (undocumented) + requestBodies: never; + // (undocumented) + responses: never; + // (undocumented) + schemas: { + NotificationType: "MessageCreated" | "MessageUpdated" | "MessageDeleted" | "RoomJoined" | "RoomLeft" | "RoomClosed" | "RoomMemberJoined" | "RoomMemberLeft" | "AddContact"; + Notification: { + notificationType: components["schemas"]["NotificationType"]; + body: components["schemas"]["NewMessageNotificationBody"] | components["schemas"]["NewRoomNotificationBody"] | components["schemas"]["UpdateMessageNotificationBody"] | components["schemas"]["AddContactNotificationBody"] | components["schemas"]["MemberJoinedNotificationBody"] | components["schemas"]["MemberLeftNotificationBody"] | components["schemas"]["RoomLeftNotificationBody"]; + }; + NewMessageNotificationBody: { + conversation: components["schemas"]["ChatConversation"]; + message: components["schemas"]["MessageInfo"]; + notificationType: "MessageCreated"; + }; + NewMessageNotification: components["schemas"]["Notification"] & { + notificationType?: "NewMessage"; + }; + NewRoomNotificationBody: { + roomId: string; + title: string; + defaultConversationId?: string; + properties?: Record | null; + notificationType: "RoomJoined"; + }; + NewRoomNotification: components["schemas"]["Notification"] & { + notificationType?: "NewRoom"; + body?: components["schemas"]["NewRoomNotificationBody"]; + }; + UpdateMessageNotificationBody: { + conversation: components["schemas"]["ChatConversation"]; + message: components["schemas"]["MessageInfo"]; + notificationType: "MessageUpdated"; + }; + UpdateMessageNotification: components["schemas"]["Notification"] & { + notificationType?: "UpdateMessage"; + body?: components["schemas"]["UpdateMessageNotificationBody"]; + }; + AddContactNotificationBody: { + userId: string; + notificationType: "AddContact"; + }; + AddContactNotification: components["schemas"]["Notification"] & { + notificationType?: "AddContact"; + body?: components["schemas"]["AddContactNotificationBody"]; + }; + MemberJoinedNotificationBody: { + roomId: string; + title: string; + userId: string; + notificationType: "RoomMemberJoined"; + }; + MemberJoinedNotification: components["schemas"]["Notification"] & { + notificationType?: "MemberJoined"; + body?: components["schemas"]["MemberJoinedNotificationBody"]; + }; + MemberLeftNotificationBody: { + roomId: string; + title: string; + userId: string; + notificationType: "RoomMemberLeft"; + }; + MemberLeftNotification: components["schemas"]["Notification"] & { + notificationType?: "MemberLeft"; + body?: components["schemas"]["MemberLeftNotificationBody"]; + }; + RoomLeftNotificationBody: { + roomId: string; + title: string; + notificationType: "RoomLeft"; + }; + RoomLeftNotification: components["schemas"]["Notification"] & { + notificationType?: "RoomLeft"; + body?: components["schemas"]["RoomLeftNotificationBody"]; + }; + RoomClosedNotificationBody: { + roomId: string; + title: string; + notificationType: "RoomClosed"; + }; + RoomClosedNotification: components["schemas"]["Notification"] & { + notificationType?: "RoomClosed"; + body?: components["schemas"]["RoomClosedNotificationBody"]; + }; + MessageDeletedNotificationBody: { + conversation: components["schemas"]["ChatConversation"]; + messageId: string; + notificationType: "MessageDeleted"; + }; + MessageDeletedNotification: components["schemas"]["Notification"] & { + notificationType?: "MessageDeleted"; + body?: components["schemas"]["MessageDeletedNotificationBody"]; + }; + UserProfile: { + userId: string; + roomIds?: string[]; + conversationIds?: string[]; + }; + ListUserConversationRequest: { + continuationToken?: string | null; + maxCount: number | null; + }; + ListUserConversationResponse: { + conversations: components["schemas"]["ChatConversation"][]; + continuationToken?: string | null; + }; + ChatConversation: { + roomId?: string | null; + topicId?: string | null; + conversationId?: string | null; + }; + ApprovalEnum: "AutoApprove" | "ManualApprove" | "AutoDeny"; + UserPolicy: { + addContact: components["schemas"]["ApprovalEnum"]; + publicProperties?: string[]; + friendProperties?: string[]; + privateProperties?: string[]; + }; + ContactResultState: "OK" | "Pending" | "Failed"; + AddContactResult: { + userId: string; + state: components["schemas"]["ContactResultState"]; + message: string; + }; + ContactRequest: { + userId: string; + message: string; + }; + ContactOperation: "Approve" | "Deny" | "Block"; + ContactRequestOperation: { + operation: components["schemas"]["ContactOperation"]; + userId: string; + }; + RoomInfo: { + roomId: string; + title: string; + defaultConversationId: string; + properties?: Record | null; + }; + RoomInfoWithMembers: components["schemas"]["RoomInfo"] & { + members: string[]; + }; + RoomMemberJoinEnum: "AutoApprove" | "ManualApprove" | "InviteOnly"; + RoomMessagePermissionEnum: "Allow" | "AdminOnly" | "Deny"; + RoomReactPermissionEnum: "Allow" | "Deny"; + RoomPolicy: { + memberJoin: components["schemas"]["RoomMemberJoinEnum"]; + messageTypeText?: components["schemas"]["RoomMessagePermissionEnum"]; + messageTypeImage?: components["schemas"]["RoomMessagePermissionEnum"]; + react?: components["schemas"]["RoomReactPermissionEnum"]; + }; + MessageRangeQuery: { + conversation: components["schemas"]["ChatConversation"]; + start?: string | null; + end?: string | null; + maxCount: number | null; + }; + MessageInfo: { + messageId: string; + createdBy?: string; + createdAt?: string; + bodyType?: string; + messageBodyType: string; + content: { + text?: string | null; + binary?: string | null; + }; + refMessageId?: string | null; + }; + CreateTextMessage: { + conversation: components["schemas"]["ChatConversation"]; + message: string; + refMessageId?: string | null; + extMentions?: string[] | null; + extDeleteAfterRead?: boolean | null; + extScheduled?: string | null; + }; + CreateMessage: { + conversation: components["schemas"]["ChatConversation"]; + messageType: string; + content: { + text?: string | null; + binary?: string | null; + }; + refMessageId?: string | null; + extMentions?: string[] | null; + extDeleteAfterRead?: boolean | null; + extScheduled?: string | null; + }; + MessageBody: { + conversation: components["schemas"]["ChatConversation"]; + messageId: string; + messageType: string; + messageBodyType: string; + content: { + text?: string | null; + binary?: string | null; + }; + refMessageId?: string | null; + }; + JoinRoomState: "OK" | "Pending" | "Failed"; + JoinRoomResult: { + room: string; + state: components["schemas"]["JoinRoomState"]; + message: string; + }; + JoinRoomRequest: { + userId: string; + message: string; + }; + JoinRoomOperationEnum: "Approve" | "Deny" | "Block"; + JoinRoomOperation: { + room: string; + userId: string; + operation: components["schemas"]["JoinRoomOperationEnum"]; + }; + RoomMember: { + userId: string; + role: string; + }; + RoomMemberOperationEnum: "Add" | "Delete" | "Update"; + RoomMemberOperation: { + operation: components["schemas"]["RoomMemberOperationEnum"]; + member: components["schemas"]["RoomMember"]; + }; + RoomMemberOperationType: "Add" | "Delete"; + ManageRoomMemberRequest: { + roomId: string; + operation: components["schemas"]["RoomMemberOperationType"]; + userId: string; + }; + SendMessageResponse: { + id: string; + }; + }; +} + // @public export interface CreateRoomOptions extends OperationOptions { roomId?: string; @@ -89,7 +335,7 @@ export interface GetRoomOptions extends OperationOptions { } // @public -export interface GetUserInfoOptions extends OperationOptions { +export interface GetUserProfileOptions extends OperationOptions { } // @public @@ -106,12 +352,9 @@ export const KnownChatErrorCode: { export interface ListRoomMessagesOptions extends OperationOptions { endId?: string; maxPageSize?: number; - roomId: string; startId?: string; } -// Warning: (ae-forgotten-export) The symbol "Schemas" needs to be exported by the entry point index.d.ts -// // @public (undocumented) export type MessageInfo = Schemas["MessageInfo"]; @@ -179,6 +422,9 @@ export type RoomInfo = Schemas["RoomInfo"]; // @public (undocumented) export type RoomInfoWithMembers = Schemas["RoomInfoWithMembers"]; +// @public (undocumented) +export type Schemas = components["schemas"]; + // @public export interface SendToRoomOptions extends OperationOptions { } @@ -187,10 +433,6 @@ export interface SendToRoomOptions extends OperationOptions { export interface StartOptions extends OperationOptions { } -// @public -export interface StopOptions extends OperationOptions { -} - // @public (undocumented) export type UserProfile = Schemas["UserProfile"]; diff --git a/sdk/webpubsub-chat-client/src/chatClient.ts b/sdk/webpubsub-chat-client/src/chatClient.ts index f73b3611d..bf0c359a4 100644 --- a/sdk/webpubsub-chat-client/src/chatClient.ts +++ b/sdk/webpubsub-chat-client/src/chatClient.ts @@ -30,11 +30,10 @@ import type { ListRoomMessagesOptions, OperationOptions, StartOptions, - StopOptions, GetRoomOptions, CreateRoomOptions, SendToRoomOptions, - GetUserInfoOptions, + GetUserProfileOptions, AddUserToRoomOptions, RemoveUserFromRoomOptions, } from "./options.js"; @@ -358,7 +357,7 @@ class ChatClient { } } - public async getUserInfo(userId: string, options?: GetUserInfoOptions): Promise { + public async getUserProfile(userId: string, options?: GetUserProfileOptions): Promise { this.ensureStarted(); return this.invokeWithReturnType( INVOCATION_NAME.GET_USER_PROPERTIES, @@ -432,7 +431,7 @@ class ChatClient { * `withMembers` is `true` the returned `members` array is * populated; defaults to `false` to save a round-trip. */ - public async getRoom(roomId: string, options?: GetRoomOptions): Promise { + public async getRoomDetail(roomId: string, options?: GetRoomOptions): Promise { this.ensureStarted(); return this.invokeWithReturnType( INVOCATION_NAME.GET_ROOM, @@ -488,7 +487,7 @@ class ChatClient { if (this._rooms.has(roomId)) { return; } - const roomInfo = await this.getRoom(roomId, options); + const roomInfo = await this.getRoomDetail(roomId, options); this._rooms.set(roomId, roomInfo); } @@ -538,7 +537,7 @@ class ChatClient { * * @example Stream every message (e.g. for export or full sync): * ```ts - * for await (const msg of client.listRoomMessages({ roomId })) { + * for await (const msg of client.listRoomMessages(roomId)) { * console.log(msg.content.text); * } * ``` @@ -546,7 +545,7 @@ class ChatClient { * @example Load history one page at a time (Teams-style scroll-back): * ```ts * // Load up to 50 messages per page. - * const pages = client.listRoomMessages({ roomId }).byPage({ maxPageSize: 50 }); + * const pages = client.listRoomMessages(roomId).byPage({ maxPageSize: 50 }); * const first = await pages.next(); * displayMessages(first.value); * // later, when the user scrolls up: @@ -556,18 +555,18 @@ class ChatClient { * * The room must be one this client has created or joined. */ - public listRoomMessages(options: ListRoomMessagesOptions): PagedAsyncIterableIterator { + public listRoomMessages(roomId: string, options?: ListRoomMessagesOptions): PagedAsyncIterableIterator { this.ensureStarted(); - const conversationId = this._rooms.get(options.roomId)?.defaultConversationId; + const conversationId = this._rooms.get(roomId)?.defaultConversationId; if (!conversationId) { - throw new ChatError(`Failed to listRoomMessages, not found roomId ${options.roomId}`, ERRORS.UnknownRoom); + throw new ChatError(`Failed to listRoomMessages, not found roomId ${roomId}`, ERRORS.UnknownRoom); } - const defaultPageSize = options.maxPageSize ?? 100; + const defaultPageSize = options?.maxPageSize ?? 100; const firstPageLink: MessageRangeQuery = { conversation: { conversationId }, - start: options.startId ?? null, - end: options.endId ?? null, + start: options?.startId ?? null, + end: options?.endId ?? null, maxCount: defaultPageSize, }; @@ -583,7 +582,7 @@ class ChatClient { INVOCATION_NAME.LIST_MESSAGES, query, "json", - { abortSignal: options.abortSignal }, + { abortSignal: options?.abortSignal }, ); if (result.messages.length === 0) { return undefined; @@ -665,11 +664,11 @@ class ChatClient { * should keep their authentication source (URL or credential) * constant. * - * @param _options - Reserved for symmetry with other operations. - * Stopping is a local-state cleanup that does not perform a network - * round-trip and is therefore not cancellable today. + * Stopping is not cancellable: it tears down the transport and clears + * local state, mirroring the underlying `WebPubSubClient.stop()`, which + * takes no options. */ - public async stop(_options?: StopOptions): Promise { + public async stop(): Promise { const startPromise = this._startPromise; if (startPromise) { await startPromise.catch(() => undefined); diff --git a/sdk/webpubsub-chat-client/src/index.ts b/sdk/webpubsub-chat-client/src/index.ts index 6cb2783d2..a71eceb6f 100644 --- a/sdk/webpubsub-chat-client/src/index.ts +++ b/sdk/webpubsub-chat-client/src/index.ts @@ -2,9 +2,11 @@ import { ChatClient, ChatError } from './chatClient.js'; import { ERRORS } from './constant.js'; export type { + components, MessageInfo, RoomInfo, RoomInfoWithMembers, + Schemas, UserProfile, } from './generatedTypes.js'; @@ -22,11 +24,10 @@ export type { export type { OperationOptions, StartOptions, - StopOptions, GetRoomOptions, CreateRoomOptions, SendToRoomOptions, - GetUserInfoOptions, + GetUserProfileOptions, AddUserToRoomOptions, RemoveUserFromRoomOptions, ListRoomMessagesOptions, diff --git a/sdk/webpubsub-chat-client/src/options.ts b/sdk/webpubsub-chat-client/src/options.ts index 6d6bbb068..4784c6f76 100644 --- a/sdk/webpubsub-chat-client/src/options.ts +++ b/sdk/webpubsub-chat-client/src/options.ts @@ -21,10 +21,7 @@ export interface OperationOptions { /** Options for `ChatClient.start()`. */ export interface StartOptions extends OperationOptions {} -/** Options for `ChatClient.stop()`. */ -export interface StopOptions extends OperationOptions {} - -/** Options for `ChatClient.getRoom()`. */ +/** Options for `ChatClient.getRoomDetail()`. */ export interface GetRoomOptions extends OperationOptions { /** * When `true`, the returned `RoomInfoWithMembers.members` array is @@ -47,8 +44,8 @@ export interface CreateRoomOptions extends OperationOptions { /** Options for `ChatClient.sendToRoom()`. */ export interface SendToRoomOptions extends OperationOptions {} -/** Options for `ChatClient.getUserInfo()`. */ -export interface GetUserInfoOptions extends OperationOptions {} +/** Options for `ChatClient.getUserProfile()`. */ +export interface GetUserProfileOptions extends OperationOptions {} /** Options for `ChatClient.addUserToRoom()`. */ export interface AddUserToRoomOptions extends OperationOptions {} @@ -58,8 +55,6 @@ export interface RemoveUserFromRoomOptions extends OperationOptions {} /** Options for `ChatClient.listRoomMessages()`. */ export interface ListRoomMessagesOptions extends OperationOptions { - /** Room to list messages from. Must be a room this client has created or joined. */ - roomId: string; /** * Inclusive lower bound on message id; omit to start from the earliest available message. */ diff --git a/sdk/webpubsub-chat-client/tests/integration.test.ts b/sdk/webpubsub-chat-client/tests/integration.test.ts index 21a28f545..a68b5e69a 100644 --- a/sdk/webpubsub-chat-client/tests/integration.test.ts +++ b/sdk/webpubsub-chat-client/tests/integration.test.ts @@ -139,7 +139,7 @@ test("single client", { timeout: SHORT_TEST_TIMEOUT }, async (t) => { assert.deepEqual(created.members, [chat1.userId], "members should contain only the creator"); assert.ok(created.members.includes(chat1.userId), "members should include the creator"); - const fetched = await chat1.getRoom(created.roomId, { withMembers: true }); + const fetched = await chat1.getRoomDetail(created.roomId, { withMembers: true }); assert.equal(fetched.roomId, created.roomId, "fetched roomId should match created"); assert.equal(fetched.title, created.title, "fetched title should match created"); assert.ok(Array.isArray(fetched.members), "fetched members should be an array"); @@ -176,7 +176,7 @@ test("create room with multiple users", { timeout: LONG_TEST_TIMEOUT }, async (t } const listedMsgs: MessageInfo[] = []; - for await (const message of chats[0].listRoomMessages({ roomId: createdRoom.roomId, startId: "0", maxPageSize: 100 })) { + for await (const message of chats[0].listRoomMessages(createdRoom.roomId, { startId: "0", maxPageSize: 100 })) { listedMsgs.push(message); } let listedMsgCount = 0; @@ -190,7 +190,7 @@ test("create room with multiple users", { timeout: LONG_TEST_TIMEOUT }, async (t // 5 messages and maxPageSize=2 we expect pages of size [2, 2, 1] (or // possibly with a trailing empty page that the iterator filters out). const pagesIter = chats[0] - .listRoomMessages({ roomId: createdRoom.roomId, startId: "0" }) + .listRoomMessages(createdRoom.roomId, { startId: "0" }) .byPage({ maxPageSize: 2 }); const collectedPages: MessageInfo[][] = []; while (true) {