From 19a8d475fb892c9b308685ebf015a50d6ba8e9b2 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 23 Nov 2023 10:04:06 -0300 Subject: [PATCH] Tighten type of publishing methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current type of `any` for an outgoing message is overly permissive and doesn’t help users understand the shape of the object they need to provide. So, we: 1. change the Message class to an interface, which represents a Message-shaped object; 2. make Message’s `id` and `timestamp` properties optional (since we’re going to also use this interface for outgoing messages, which don’t necessarily have these properties), and compensate for this loosening of the Message type by introducing an InboundMessage type to represent messages received from Ably; 3. update the signature publishing methods to accept a Message object. Note that we deviate from the feature spec in that, in the feature spec, the publishing methods accept a Message instance. There are a couple of reasons for this deviation: 1. Accepting a Message-shaped object instead of a Message instance is consistent with our usage of the library in all of our existing example code and our tests, and is, I think, how things tend to be done in JavaScript. 2. The types in the feature spec are wrong; as things stand there, a user needs to provide a Message to the publishing methods, but Message has non-optional `id` and `timestamp` properties even though a user is not expected to populate them. We haven’t yet figured out how to address this error in the feature spec (i.e. do we introduce an InboundMessage type like we have here, or do we add some comments and leave it for library authors to figure out?); I started a dicussion about it in [1], but we’ve decided that we’d like to proceed with this ably-js change (which, since it’s a breaking change, we want to get into v2) without waiting for it to be addressed in the feature spec. Resolves #1472. [1] https://github.com/ably/specification/pull/156 --- ably.d.ts | 65 ++++++++++--------- .../browser/template/src/index-default.ts | 17 +++-- .../browser/template/src/index-modules.ts | 18 +++-- 3 files changed, 60 insertions(+), 40 deletions(-) diff --git a/ably.d.ts b/ably.d.ts index bdcf0fac1a..ff61ca4534 100644 --- a/ably.d.ts +++ b/ably.d.ts @@ -2055,12 +2055,12 @@ declare namespace Types { */ presence: Presence; /** - * Retrieves a {@link Types.PaginatedResult} object, containing an array of historical {@link Message} objects for the channel. If the channel is configured to persist messages, then messages can be retrieved from history for up to 72 hours in the past. If not, messages can only be retrieved from history for up to two minutes in the past. + * Retrieves a {@link Types.PaginatedResult} object, containing an array of historical {@link InboundMessage} objects for the channel. If the channel is configured to persist messages, then messages can be retrieved from history for up to 72 hours in the past. If not, messages can only be retrieved from history for up to two minutes in the past. * * @param params - A set of parameters which are used to specify which messages should be retrieved. - * @returns A promise which, upon success, will be fulfilled with a {@link Types.PaginatedResult} object containing an array of {@link Message} objects. Upon failure, the promise will be rejected with an {@link ErrorInfo} object which explains the error. + * @returns A promise which, upon success, will be fulfilled with a {@link Types.PaginatedResult} object containing an array of {@link InboundMessage} objects. Upon failure, the promise will be rejected with an {@link ErrorInfo} object which explains the error. */ - history(params?: RestHistoryParams): Promise>; + history(params?: RestHistoryParams): Promise>; /** * Publishes an array of messages to the channel. * @@ -2068,7 +2068,7 @@ declare namespace Types { * @param options - Optional parameters, such as [`quickAck`](https://faqs.ably.com/why-are-some-rest-publishes-on-a-channel-slow-and-then-typically-faster-on-subsequent-publishes) sent as part of the query string. * @returns A promise which resolves upon success of the operation and rejects with an {@link ErrorInfo} object upon its failure. */ - publish(messages: any[], options?: PublishOptions): Promise; + publish(messages: Message[], options?: PublishOptions): Promise; /** * Publishes a message to the channel. * @@ -2076,7 +2076,7 @@ declare namespace Types { * @param options - Optional parameters, such as [`quickAck`](https://faqs.ably.com/why-are-some-rest-publishes-on-a-channel-slow-and-then-typically-faster-on-subsequent-publishes) sent as part of the query string. * @returns A promise which resolves upon success of the operation and rejects with an {@link ErrorInfo} object upon its failure. */ - publish(message: any, options?: PublishOptions): Promise; + publish(message: Message, options?: PublishOptions): Promise; /** * Publishes a single message to the channel with the given event name and payload. * @@ -2124,14 +2124,14 @@ declare namespace Types { * @param event - The event name. * @param listener - An event listener function. */ - unsubscribe(event: string, listener: messageCallback): void; + unsubscribe(event: string, listener: messageCallback): void; /** * Deregisters the given listener from all event names in the array. * * @param events - An array of event names. * @param listener - An event listener function. */ - unsubscribe(events: Array, listener: messageCallback): void; + unsubscribe(events: Array, listener: messageCallback): void; /** * Deregisters all listeners for the given event name. * @@ -2150,13 +2150,13 @@ declare namespace Types { * @param filter - A {@link MessageFilter}. * @param listener - An event listener function. */ - unsubscribe(filter: MessageFilter, listener?: messageCallback): void; + unsubscribe(filter: MessageFilter, listener?: messageCallback): void; /** * Deregisters the given listener (for any/all event names). This removes an earlier subscription. * * @param listener - An event listener function. */ - unsubscribe(listener: messageCallback): void; + unsubscribe(listener: messageCallback): void; /** * Deregisters all listeners to messages on this channel. This removes all earlier subscriptions. */ @@ -2179,12 +2179,12 @@ declare namespace Types { */ detach(): Promise; /** - * Retrieves a {@link Types.PaginatedResult} object, containing an array of historical {@link Message} objects for the channel. If the channel is configured to persist messages, then messages can be retrieved from history for up to 72 hours in the past. If not, messages can only be retrieved from history for up to two minutes in the past. + * Retrieves a {@link Types.PaginatedResult} object, containing an array of historical {@link InboundMessage} objects for the channel. If the channel is configured to persist messages, then messages can be retrieved from history for up to 72 hours in the past. If not, messages can only be retrieved from history for up to two minutes in the past. * * @param params - A set of parameters which are used to specify which presence members should be retrieved. - * @returns A promise which, upon success, will be fulfilled with a {@link Types.PaginatedResult} object containing an array of {@link Message} objects. Upon failure, the promise will be rejected with an {@link ErrorInfo} object which explains the error. + * @returns A promise which, upon success, will be fulfilled with a {@link Types.PaginatedResult} object containing an array of {@link InboundMessage} objects. Upon failure, the promise will be rejected with an {@link ErrorInfo} object which explains the error. */ - history(params?: RealtimeHistoryParams): Promise>; + history(params?: RealtimeHistoryParams): Promise>; /** * Sets the {@link ChannelOptions} for the channel. * @@ -2199,7 +2199,7 @@ declare namespace Types { * @param listener - An event listener function. * @returns A promise which, upon successful attachment to the channel, will be fulfilled with a {@link ChannelStateChange} object. If the channel was already attached the promise will be resolved with `null`. Upon failure, the promise will be rejected with an {@link ErrorInfo} object. */ - subscribe(event: string, listener?: messageCallback): Promise; + subscribe(event: string, listener?: messageCallback): Promise; /** * Registers a listener for messages on this channel for multiple event name values. * @@ -2207,7 +2207,7 @@ declare namespace Types { * @param listener - An event listener function. * @returns A promise which, upon successful attachment to the channel, will be fulfilled with a {@link ChannelStateChange} object. If the channel was already attached the promise will be resolved with `null`. Upon failure, the promise will be rejected with an {@link ErrorInfo} object. */ - subscribe(events: Array, listener?: messageCallback): Promise; + subscribe(events: Array, listener?: messageCallback): Promise; /** * {@label WITH_MESSAGE_FILTER} * @@ -2217,14 +2217,14 @@ declare namespace Types { * @param listener - An event listener function. * @returns A promise which, upon successful attachment to the channel, will be fulfilled with a {@link ChannelStateChange} object. If the channel was already attached the promise will be resolved with `null`. Upon failure, the promise will be rejected with an {@link ErrorInfo} object. */ - subscribe(filter: MessageFilter, listener?: messageCallback): Promise; + subscribe(filter: MessageFilter, listener?: messageCallback): Promise; /** * Registers a listener for messages on this channel. The caller supplies a listener function, which is called each time one or more messages arrives on the channel. * * @param callback - An event listener function. * @returns A promise which, upon successful attachment to the channel, will be fulfilled with a {@link ChannelStateChange} object. If the channel was already attached the promise will be resolved with `null`. Upon failure, the promise will be rejected with an {@link ErrorInfo} object. */ - subscribe(callback: messageCallback): Promise; + subscribe(callback: messageCallback): Promise; /** * Publishes a single message to the channel with the given event name and payload. When publish is called with this client library, it won't attempt to implicitly attach to the channel, so long as [transient publishing](https://ably.com/docs/realtime/channels#transient-publish) is available in the library. Otherwise, the client will implicitly attach. * @@ -2239,14 +2239,14 @@ declare namespace Types { * @param messages - An array of {@link Message} objects. * @returns A promise which resolves upon success of the operation and rejects with an {@link ErrorInfo} object upon its failure. */ - publish(messages: any[]): Promise; + publish(messages: Message[]): Promise; /** * Publish a message to the channel. When publish is called with this client library, it won't attempt to implicitly attach to the channel. * * @param message - A {@link Message} object. * @returns A promise which resolves upon success of the operation and rejects with an {@link ErrorInfo} object upon its failure. */ - publish(message: any): Promise; + publish(message: Message): Promise; /** * Returns a promise which is resolved when the channel reaches the specified {@link ChannelState}. If the channel is already in the specified state, the promise is resolved immediately. * @@ -2329,7 +2329,7 @@ declare namespace Types { /** * Contains an individual message that is sent to, or received from, Ably. */ - class Message { + interface Message { /** * The client ID of the publisher of this message. */ @@ -2353,7 +2353,7 @@ declare namespace Types { /** * Unique ID assigned by Ably to this message. */ - id: string; + id?: string; /** * The event name. */ @@ -2361,29 +2361,34 @@ declare namespace Types { /** * Timestamp of when the message was received by Ably, as milliseconds since the Unix epoch. */ - timestamp: number; + timestamp?: number; } + /** + * A message received from Ably. + */ + type InboundMessage = Message & Required>; + /** * Static utilities related to messages. */ interface MessageStatic { /** - * A static factory method to create a `Message` object from a deserialized Message-like object encoded using Ably's wire protocol. + * A static factory method to create an `InboundMessage` object from a deserialized InboundMessage-like object encoded using Ably's wire protocol. * - * @param JsonObject - A `Message`-like deserialized object. + * @param JsonObject - A `InboundMessage`-like deserialized object. * @param channelOptions - A {@link ChannelOptions} object. If you have an encrypted channel, use this to allow the library to decrypt the data. - * @returns A promise which will be fulfilled with a `Message` object. + * @returns A promise which will be fulfilled with an `InboundMessage` object. */ - fromEncoded: (JsonObject: any, channelOptions?: ChannelOptions) => Promise; + fromEncoded: (JsonObject: any, channelOptions?: ChannelOptions) => Promise; /** - * A static factory method to create an array of `Message` objects from an array of deserialized Message-like object encoded using Ably's wire protocol. + * A static factory method to create an array of `InboundMessage` objects from an array of deserialized InboundMessage-like object encoded using Ably's wire protocol. * - * @param JsonArray - An array of `Message`-like deserialized objects. + * @param JsonArray - An array of `InboundMessage`-like deserialized objects. * @param channelOptions - A {@link ChannelOptions} object. If you have an encrypted channel, use this to allow the library to decrypt the data. - * @returns A promise which will be fulfilled with an array of {@link Message} objects. + * @returns A promise which will be fulfilled with an array of {@link InboundMessage} objects. */ - fromEncodedArray: (JsonArray: any[], channelOptions?: ChannelOptions) => Promise; + fromEncodedArray: (JsonArray: any[], channelOptions?: ChannelOptions) => Promise; } /** @@ -2601,7 +2606,7 @@ declare namespace Types { */ class PaginatedResult { /** - * Contains the current page of results; for example, an array of {@link Message} or {@link PresenceMessage} objects for a channel history request. + * Contains the current page of results; for example, an array of {@link InboundMessage} or {@link PresenceMessage} objects for a channel history request. */ items: T[]; /** diff --git a/test/package/browser/template/src/index-default.ts b/test/package/browser/template/src/index-default.ts index 865bc21e60..87d5270c0c 100644 --- a/test/package/browser/template/src/index-default.ts +++ b/test/package/browser/template/src/index-default.ts @@ -14,12 +14,19 @@ globalThis.testAblyPackage = async function () { const channel = realtime.channels.get('channel'); await attachChannel(channel); - const receivedMessagePromise = new Promise((resolve) => { - channel.subscribe(() => { - resolve(); - }); + const receivedMessagePromise = new Promise((resolve) => { + channel.subscribe(resolve); }); + // Check that we can use the TypeScript overload that accepts name and data as separate arguments await channel.publish('message', { foo: 'bar' }); - await receivedMessagePromise; + const receivedMessage = await receivedMessagePromise; + + // Check that id and timestamp of a message received from Ably can be assigned to non-optional types + const { id: string, timestamp: number } = receivedMessage; + + channel.unsubscribe(); + + // Check that we can use the TypeScript overload that accepts a Message object + await channel.publish({ name: 'message', data: { foo: 'bar' } }); }; diff --git a/test/package/browser/template/src/index-modules.ts b/test/package/browser/template/src/index-modules.ts index 07a7751e3d..9ad31d69c1 100644 --- a/test/package/browser/template/src/index-modules.ts +++ b/test/package/browser/template/src/index-modules.ts @@ -22,13 +22,21 @@ globalThis.testAblyPackage = async function () { const channel = realtime.channels.get('channel'); await attachChannel(channel); - const receivedMessagePromise = new Promise((resolve) => { - channel.subscribe(() => { - resolve(); - }); + const receivedMessagePromise = new Promise((resolve) => { + channel.subscribe(resolve); }); + // Check that we can use the TypeScript overload that accepts name and data as separate arguments await channel.publish('message', { foo: 'bar' }); - await receivedMessagePromise; + const receivedMessage = await receivedMessagePromise; + + // Check that id and timestamp of a message received from Ably can be assigned to non-optional types + const { id: string, timestamp: number } = receivedMessage; + await checkStandaloneFunction(); + + channel.unsubscribe(); + + // Check that we can use the TypeScript overload that accepts a Message object + await channel.publish({ name: 'message', data: { foo: 'bar' } }); };