-
Notifications
You must be signed in to change notification settings - Fork 59
Merge main
into integration/liveobjects
branch 2025-04-16
#2006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Previously, fromValues() took an options object with a 'stringifyAction' bool property. If that was false, then it acted as a normal fromValues; if true, then it was effectively doing a partial fromDeserialized, parsing a wire protocol message into a message but without decoding the message.data. This seemed a bit silly. Removed the options object and replaced it with an explicit Message.fromWireProtocol. Also while doing that ended up changing a bunch of other things that seemed broken: - unnecessary type-assertions due to not having a wire-protocol type, or unnecessary use of any & unknown - channeloptions being type-asserted as cipheroptions, then type-assert back to channeloptions, which made no sense - standalone PresenceMessage.fromEncoded() methods were not passing in a Crypto object, so wouldn't be able to decrypt and encrypted presence message
Tests for TM2p pending serverside implementation of field name changes being deployed
Realtime is now returning 40101 (invalid credentials) when unable to extract an app ID from the key. The Realtime team have confirmed that this new behaviour is correct and that the test should be updated [1]. [1] https://ably-real-time.slack.com/archives/CURL4U2FP/p1733479167314059?thread_ts=1733420816.469159&cid=CURL4U2FP
This was previously untested. The new test in `crypto.test.js` is taken from a test Simon added in #1923, but I wanted to separate this test from all the other work that’s being done in that PR. I’ve also copied this new test to create a similar test for the modular variant of the library. Co-authored-by: Simon Woolf <[email protected]>
Mutable message field changes and Message/PresenceMessage.fromValues refactoring
Update expected error code in a test
chore: add release information for 2.6.0
Test that we can decrypt history messages
The test waits to receive an end message on a channel then it runs the assertions, this can cause flakes because we dont know if the event loop had any other pending IO which could include delivering the 2 expected filtered messages. Run the assertions in a setImmediate so that it executes in the next event loop run allowing any queued messages to be delivered. When running the test with logging the messages were being delivered but the handler was being called after the assertions had ran. RAR-656
it's not a channel mode. why is it in here?
as agreed in ably/sdk-api-reference#44 (comment) and ably/realtime#6810 , plus addition of summary
- extract common functionality to a new BaseMessage file - introduce proper WireMessage and WirePresenceMessage classes, such that: - you just call Message.encode() to get a WireMessage, and WireMessage.decode() to get a Message, and the type system will tell you which one you have - the toJSON override hack only applies to internal wire-protocol uses and doesn't interfere with the normal public Message/PresenceMessage classes - all necessary transformations between userfriendly message and wire protocol messages (encoding the data and the action) are now done by decode() and encode(), rather than (as before) the encoding happening in-place and the action encoding happening only on json-stringification - ProtocolMessage is now always a wire-protocol message and always holds WireMessages and WirePresence; all decoding is done by the channel - The PresencePlugin interface is now just the PresenceMessage and WirePresenceMessage types, rather than ad-hoc individual functions - The fromWireProtocol, encode, and decode functions in the DefaultMessage interface are no longer needed and can be removed - Cleaned up and simplified the message decoding step in the channel, which was unnecessarily complicated
Restructure and clean up message and presencemessage types
- Update CHANGELOG - Update Ably package version
CHORE: Add release information for v.2.6.1
Message: fix parent-populated version not being propagated to serial
Chore: Release version 2.6.2
Because of implicit `attach()` hooks can produce additional errors e.g. when updating ably client to avoid this situation, channels created inside `ChannelProvider` now use `attachOnSubscribe = false` flag and attach is happening explicitly.
[ECO-5184] feat: update channel hooks implementation
It seems like the Realtime connection key format, which as the removed comment implies is not guaranteed to be stable, has changed on sandbox. Stop using our knowledge of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/common/lib/types/basemessage.ts (1)
13-24
: ReplaceFunction
type with explicit function signatures for better type safety.Using the generic
Function
type masks the specific function signature and reduces type safety. This was flagged by static analysis tools and appears in a previous review as well.Apply this diff to define more explicit function types:
export type CipherOptions = { channelCipher: { - encrypt: Function; + encrypt: (data: ArrayBuffer | Uint8Array) => Promise<ArrayBuffer | Uint8Array>; algorithm: 'aes'; }; cipher?: { channelCipher: { - encrypt: Function; + encrypt: (data: ArrayBuffer | Uint8Array) => Promise<ArrayBuffer | Uint8Array>; algorithm: 'aes'; }; }; };🧰 Tools
🪛 Biome (1.9.4)
[error] 15-15: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 20-20: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🧹 Nitpick comments (9)
src/common/lib/types/basemessage.ts (9)
36-45
: Use type predicate for safer type narrowing in normaliseContext.The current implementation uses a type assertion (
as EncodingDecodingContext
) which bypasses TypeScript's type checking. Using a type predicate would provide better type safety.Consider using a type predicate instead:
-function normaliseContext(context: CipherOptions | EncodingDecodingContext | ChannelOptions): EncodingDecodingContext { - if (!context || !(context as EncodingDecodingContext).channelOptions) { +function normaliseContext(context: CipherOptions | EncodingDecodingContext | ChannelOptions): EncodingDecodingContext { + function isEncodingDecodingContext(ctx: any): ctx is EncodingDecodingContext { + return ctx && 'channelOptions' in ctx; + } + + if (!context || !isEncodingDecodingContext(context)) { return { channelOptions: context as ChannelOptions, plugins: {}, baseEncodedPreviousPayload: undefined, }; } - return context as EncodingDecodingContext; + return context; }
47-61
: Use optional chaining for better code readability.The conditional check on line 52 could benefit from optional chaining to make the code more concise.
Apply this diff to use optional chaining:
export function normalizeCipherOptions( Crypto: IUntypedCryptoStatic | null, logger: Logger, options: API.ChannelOptions | null, ): ChannelOptions { - if (options && options.cipher) { + if (options?.cipher) { if (!Crypto) Utils.throwMissingPluginError('Crypto'); const cipher = Crypto.getCipher(options.cipher, logger); return { cipher: cipher.cipherParams, channelCipher: cipher.cipher, }; } return options ?? {}; }🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
93-107
: Inconsistency in async/await pattern in encode function.The
encode
function awaitsencodeData
result implicitly through destructuring assignment but doesn't useawait
. This could lead to unexpected behavior ifencodeData
is modified to be asynchronous in the future.Apply this diff for consistency:
export async function encode<T extends BaseMessage>(msg: T, options: unknown): Promise<T> { - const { data, encoding } = encodeData(msg.data, msg.encoding); + const { data, encoding } = await Promise.resolve(encodeData(msg.data, msg.encoding)); msg.data = data; msg.encoding = encoding; if (options != null && (options as CipherOptions).cipher) { return encrypt(msg, options as CipherOptions); } else { return msg; } }
133-135
: Missing consistent error code pattern.The error thrown on line 134 uses a hardcoded error code (40013). Consider extracting common error codes to constants for better maintainability.
Consider creating an error code enumeration or constants:
+// At the top of the file with other imports +export enum ErrorCodes { + UNSUPPORTED_DATA_TYPE = 40013, + VCDIFF_DECODER_MISSING = 40019, + DELTA_DECODING_NOT_SUPPORTED = 40020, + VCDIFF_DELTA_DECODE_FAILED = 40018, + UNEXPECTED_ACTION = 40000, +} // Then in the code - throw new ErrorInfo('Data type is unsupported', 40013, 400); + throw new ErrorInfo('Data type is unsupported', ErrorCodes.UNSUPPORTED_DATA_TYPE, 400);
152-271
: Optimize conditional chain in decodeData function for readability.The decoding logic in the
decodeData
function contains deeply nested conditions and switch cases that could be simplified for better readability.Consider extracting the individual decoder implementations into separate functions to reduce nesting and improve maintainability:
export async function decodeData( data: any, encoding: string | null | undefined, inputContext: CipherOptions | EncodingDecodingContext | ChannelOptions, ): Promise<{ error?: ErrorInfo; data: any; encoding: string | null | undefined; }> { const context = normaliseContext(inputContext); let lastPayload = data; let decodedData = data; let finalEncoding = encoding; let decodingError: ErrorInfo | undefined; if (encoding) { const xforms = encoding.split('/'); let lastProcessedEncodingIndex; let encodingsToProcess = xforms.length; let xform = ''; try { while ((lastProcessedEncodingIndex = encodingsToProcess) > 0) { // eslint-disable-next-line security/detect-unsafe-regex const match = xforms[--encodingsToProcess].match(/([-\w]+)(\+([\w-]+))?/); if (!match) break; xform = match[1]; + const decoderResult = await processTransform(xform, match, decodedData, context, lastProcessedEncodingIndex, xforms.length); + decodedData = decoderResult.data; + if (decoderResult.lastPayload) { + lastPayload = decoderResult.lastPayload; + } - switch (xform) { - case 'base64': - decodedData = Platform.BufferUtils.base64Decode(String(decodedData)); - if (lastProcessedEncodingIndex == xforms.length) { - lastPayload = decodedData; - } - continue; - // ... rest of the switch cases - } } } catch (e) { // ... error handling } } // ... rest of the function } +async function processTransform( + xform: string, + match: RegExpMatchArray, + data: any, + context: EncodingDecodingContext, + lastProcessedEncodingIndex: number, + totalEncodings: number +): Promise<{ data: any; lastPayload?: any }> { + switch (xform) { + case 'base64': { + const decoded = Platform.BufferUtils.base64Decode(String(data)); + return { + data: decoded, + lastPayload: lastProcessedEncodingIndex === totalEncodings ? decoded : undefined + }; + } + // ... other cases extracted from the original switch + } + throw new Error('Unknown encoding'); +}🧰 Tools
🪛 Biome (1.9.4)
[error] 197-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
196-201
: Use optional chaining for nested condition checks.The multiple conditions checking for non-null properties could be simplified using optional chaining.
Apply this diff to use optional chaining:
case 'cipher': if ( - context.channelOptions != null && - context.channelOptions.cipher && - context.channelOptions.channelCipher + context.channelOptions?.cipher && + context.channelOptions?.channelCipher ) { const xformAlgorithm = match[3], cipher = context.channelOptions.channelCipher;🧰 Tools
🪛 Biome (1.9.4)
[error] 197-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
213-214
: Add more informative guidance for missing Vcdiff decoder.The error message for a missing Vcdiff decoder could be enhanced with more context about how to install or enable it.
Enhance the error message with installation instructions:
case 'vcdiff': if (!context.plugins || !context.plugins.vcdiff) { - throw new ErrorInfo('Missing Vcdiff decoder (https://github.com/ably-forks/vcdiff-decoder)', 40019, 400); + throw new ErrorInfo( + 'Missing Vcdiff decoder. Please install the vcdiff-decoder plugin from https://github.com/ably-forks/vcdiff-decoder or use "npm install @ably/vcdiff-decoder"', + 40019, + 400 + ); }
326-363
: Improve type safety in populateFieldsFromParent function.The
populateFieldsFromParent
function uses non-null assertions (!
) when accessing potentially undefined properties of the parent protocol message.Add type guards to improve safety:
export function populateFieldsFromParent(parent: ProtocolMessage) { const { id, connectionId, timestamp } = parent; let msgs: BaseMessage[]; switch (parent.action) { case actions.MESSAGE: { + if (!parent.messages) { + throw new ErrorInfo('Protocol message with MESSAGE action must have messages', 40000, 400); + } msgs = parent.messages!; break; } case actions.PRESENCE: case actions.SYNC: + if (!parent.presence) { + throw new ErrorInfo('Protocol message with PRESENCE/SYNC action must have presence', 40000, 400); + } msgs = parent.presence!; break; case actions.ANNOTATION: + if (!parent.annotations) { + throw new ErrorInfo('Protocol message with ANNOTATION action must have annotations', 40000, 400); + } msgs = parent.annotations!; break; case actions.OBJECT: case actions.OBJECT_SYNC: + if (!parent.state) { + throw new ErrorInfo('Protocol message with OBJECT/OBJECT_SYNC action must have state', 40000, 400); + } msgs = parent.state!; break; default: throw new ErrorInfo('Unexpected action ' + parent.action, 40000, 400); }
386-395
: Add JSDoc comments to the BaseMessage class.The
BaseMessage
abstract class lacks documentation that would help developers understand its purpose and usage.Add JSDoc comments to describe the class and its properties:
+/** + * Abstract base class representing a message in the Ably protocol. + * Provides common properties and functionality for all message types. + */ export abstract class BaseMessage { + /** Unique identifier for the message */ id?: string; + /** Timestamp when the message was sent or received (in milliseconds since epoch) */ timestamp?: number; + /** ID of the client that generated the message */ clientId?: string; + /** ID of the connection that the message was sent on */ connectionId?: string; + /** Message payload */ data?: any; + /** String describing the encoding applied to the data */ encoding?: string | null; + /** Additional metadata for the message */ extras?: any; + /** Size of the message in bytes */ size?: number; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
src/common/lib/types/basemessage.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/common/lib/types/basemessage.ts
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 197-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 15-15: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 20-20: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test-node (20.x)
- GitHub Check: test-browser (webkit)
- GitHub Check: test-node (18.x)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-node (16.x)
- GitHub Check: test-browser (chromium)
This commit merges changes from the next PRs: - #2001 - #2002 - #2000 - #1953 - #1989 - #1988 - #1984 - #1949 - #1980 - #1979 - #1978 - #1976 - #1962 - #1959 - #1955 - #1957 - #1940 - #1925 - #1947 - #1946 - #1945 - #1944 into the integration/liveobjects branch. No significant changes made during merge conflict resolution.
Encode function refactoring in commit f4073b9 made it so `data` and `encoding` fields will be set on a message object even if they are undefined. This is not a problem in itself, but current `BaseMessage.strMsg` implementation does not check for undefined values and adds them to the result string. It should not do that
74088f0
to
4a51ccd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/platform/react-hooks/src/hooks/useChannelAttach.ts (1)
34-42
:⚠️ Potential issueAdd missing dependency in useEffect.
The effect uses the
ably
variable but doesn't include it in the dependency array, which could lead to stale references if the Ably client changes.useEffect(() => { if (shouldAttachToTheChannel) { channel.attach().catch((reason) => { // we use a fire-and-forget approach for attaching, but calling detach during the attaching process or while // suspending can cause errors that will be automatically resolved logError(ably, reason.toString()); }); } - }, [shouldAttachToTheChannel, channel]); + }, [shouldAttachToTheChannel, channel, ably]);🧰 Tools
🪛 GitHub Check: lint
[warning] 42-42:
React Hook useEffect has a missing dependency: 'ably'. Either include it or remove the dependency arraysrc/common/lib/client/realtimechannel.ts (1)
244-287
: Improve type safety for publish arguments.The
publish
method has been refactored to properly handle different input formats, but it still acceptsany[]
as arguments which makes the API less clear to consumers.Consider using method overloads to provide better type information:
- async publish(...args: any[]): Promise<void> { + async publish(message: Message | Message[]): Promise<void>; + async publish(name: string, data: unknown): Promise<void>; + async publish(...args: any[]): Promise<void> { let messages: Message[]; let argCount = args.length; if (argCount == 1) { if (Utils.isObject(args[0])) { messages = [Message.fromValues(args[0])]; } else if (Array.isArray(args[0])) { messages = Message.fromValuesArray(args[0]); } else { throw new ErrorInfo( 'The single-argument form of publish() expects a message object or an array of message objects', 40013, 400, ); } } else { messages = [Message.fromValues({ name: args[0], data: args[1] })]; } // Rest of the implementation...src/common/lib/types/basemessage.ts (1)
13-24
: 🛠️ Refactor suggestionAvoid using the
Function
type inCipherOptions
.Using
Function
can mask specific function signatures. Consider defining a more explicit function type to improve type safety and self-documentation.export type CipherOptions = { channelCipher: { - encrypt: Function; + encrypt: (data: ArrayBuffer | Uint8Array) => Promise<ArrayBuffer | Uint8Array>; algorithm: 'aes'; }; cipher?: { channelCipher: { - encrypt: Function; + encrypt: (data: ArrayBuffer | Uint8Array) => Promise<ArrayBuffer | Uint8Array>; algorithm: 'aes'; }; }; };🧰 Tools
🪛 Biome (1.9.4)
[error] 15-15: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 20-20: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🧹 Nitpick comments (6)
test/realtime/annotations.test.js (1)
1-1
: Consider removing redundant 'use strict' directive.JavaScript modules are automatically in strict mode, making this directive redundant. However, if this file isn't using ES modules, you can safely ignore this suggestion.
-'use strict'; define(['shared_helper', 'chai'], function (Helper, chai) {
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
src/common/lib/client/realtimepresence.ts (1)
60-60
: Optional interface extraction.
Storing pending presence items as{ presence: WirePresenceMessage; callback: ErrCallback }[]
is valid. For clarity, you could create a small interface or type alias, liketype PendingPresenceItem = { presence: WirePresenceMessage; callback: ErrCallback };
, then use an array of that type.src/common/lib/types/annotation.ts (1)
14-33
:fromEncoded
andfromEncodedArray
functions.export async function fromEncoded(…): Promise<Annotation> { … } export async function fromEncodedArray(…): Promise<Annotation[]> { … }These mirror patterns from
message.ts
andpresencemessage.ts
. The code is straightforward. Consider adding tests for edge cases, such as invalid wire data or missing fields, if not already covered.src/common/lib/types/message.ts (1)
91-101
:getMessagesSize()
usesgetMessageSize()
.export function getMessagesSize(messages: WireMessage[]): number { … total += msg.size || (msg.size = getMessageSize(msg)); }Caching the size in
msg.size
is a small optimization. Make sure to reset if the message’s data changes.src/common/lib/types/basemessage.ts (2)
52-53
: Use optional chaining for clearer code.The nested conditions could be simplified using optional chaining for better readability.
- if (options && options.cipher) { + if (options?.cipher) { if (!Crypto) Utils.throwMissingPluginError('Crypto'); const cipher = Crypto.getCipher(options.cipher, logger);🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
196-212
: Use optional chaining for nested conditions.The nested condition checks could be simplified using optional chaining for better readability.
- if ( - context.channelOptions != null && - context.channelOptions.cipher && - context.channelOptions.channelCipher - ) { + if (context.channelOptions?.cipher && context.channelOptions?.channelCipher) { const xformAlgorithm = match[3], cipher = context.channelOptions.channelCipher; /* don't attempt to decrypt unless the cipher params are compatible */ if (xformAlgorithm != cipher.algorithm) { throw new Error('Unable to decrypt message with given cipher; incompatible cipher params'); } decodedData = await cipher.decrypt(decodedData); continue; } else { throw new Error('Unable to decrypt message; not an encrypted channel'); }🧰 Tools
🪛 Biome (1.9.4)
[error] 197-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (53)
.eslintrc.js
(1 hunks).github/workflows/test-browser.yml
(1 hunks)CHANGELOG.md
(1 hunks)README.md
(1 hunks)ably.d.ts
(19 hunks)modular.d.ts
(5 hunks)package.json
(1 hunks)src/common/lib/client/baseclient.ts
(3 hunks)src/common/lib/client/defaultrealtime.ts
(3 hunks)src/common/lib/client/defaultrest.ts
(3 hunks)src/common/lib/client/modularplugins.ts
(1 hunks)src/common/lib/client/realtimeannotations.ts
(1 hunks)src/common/lib/client/realtimechannel.ts
(19 hunks)src/common/lib/client/realtimepresence.ts
(9 hunks)src/common/lib/client/restannotations.ts
(1 hunks)src/common/lib/client/restchannel.ts
(7 hunks)src/common/lib/transport/comettransport.ts
(2 hunks)src/common/lib/transport/connectionmanager.ts
(2 hunks)src/common/lib/transport/protocol.ts
(3 hunks)src/common/lib/transport/transport.ts
(2 hunks)src/common/lib/transport/websockettransport.ts
(1 hunks)src/common/lib/types/annotation.ts
(1 hunks)src/common/lib/types/basemessage.ts
(1 hunks)src/common/lib/types/defaultannotation.ts
(1 hunks)src/common/lib/types/message.ts
(3 hunks)src/common/lib/types/protocolmessage.ts
(5 hunks)src/common/lib/types/protocolmessagecommon.ts
(1 hunks)src/platform/react-hooks/src/AblyReactHooks.ts
(1 hunks)src/platform/react-hooks/src/ChannelProvider.tsx
(2 hunks)src/platform/react-hooks/src/fakes/ably.ts
(3 hunks)src/platform/react-hooks/src/hooks/constants.ts
(1 hunks)src/platform/react-hooks/src/hooks/useChannel.ts
(2 hunks)src/platform/react-hooks/src/hooks/useChannelAttach.test.tsx
(1 hunks)src/platform/react-hooks/src/hooks/useChannelAttach.ts
(1 hunks)src/platform/react-hooks/src/hooks/usePresence.ts
(2 hunks)src/platform/react-hooks/src/hooks/usePresenceListener.ts
(2 hunks)src/platform/react-hooks/src/utils.ts
(1 hunks)src/platform/web/lib/http/http.ts
(1 hunks)src/platform/web/modular.ts
(1 hunks)src/platform/web/modular/annotations.ts
(1 hunks)src/plugins/objects/objects.ts
(1 hunks)src/plugins/push/pushactivation.ts
(1 hunks)test/browser/connection.test.js
(8 hunks)test/browser/modular.test.js
(20 hunks)test/common/ably-common
(1 hunks)test/common/modules/shared_helper.js
(1 hunks)test/realtime/annotations.test.js
(1 hunks)test/realtime/channel.test.js
(7 hunks)test/realtime/message.test.js
(3 hunks)test/realtime/objects.test.js
(18 hunks)test/realtime/presence.test.js
(6 hunks)test/rest/message.test.js
(1 hunks)test/support/browser_file_list.js
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- package.json
- .github/workflows/test-browser.yml
- test/common/ably-common
- test/support/browser_file_list.js
🚧 Files skipped from review as they are similar to previous changes (38)
- src/platform/react-hooks/src/ChannelProvider.tsx
- src/common/lib/transport/transport.ts
- src/platform/web/modular.ts
- .eslintrc.js
- src/platform/react-hooks/src/hooks/usePresenceListener.ts
- src/common/lib/client/defaultrest.ts
- src/platform/react-hooks/src/fakes/ably.ts
- src/plugins/objects/objects.ts
- src/common/lib/transport/protocol.ts
- src/common/lib/client/baseclient.ts
- test/realtime/channel.test.js
- src/platform/web/modular/annotations.ts
- src/common/lib/client/defaultrealtime.ts
- src/plugins/push/pushactivation.ts
- src/platform/react-hooks/src/utils.ts
- src/platform/web/lib/http/http.ts
- test/common/modules/shared_helper.js
- src/platform/react-hooks/src/hooks/useChannelAttach.test.tsx
- modular.d.ts
- test/rest/message.test.js
- src/platform/react-hooks/src/hooks/constants.ts
- src/platform/react-hooks/src/AblyReactHooks.ts
- src/common/lib/transport/connectionmanager.ts
- src/common/lib/types/defaultannotation.ts
- src/platform/react-hooks/src/hooks/useChannel.ts
- test/browser/modular.test.js
- src/platform/react-hooks/src/hooks/usePresence.ts
- src/common/lib/transport/comettransport.ts
- test/browser/connection.test.js
- test/realtime/presence.test.js
- test/realtime/objects.test.js
- src/common/lib/client/restchannel.ts
- src/common/lib/client/restannotations.ts
- src/common/lib/client/realtimeannotations.ts
- src/common/lib/transport/websockettransport.ts
- src/common/lib/types/protocolmessagecommon.ts
- src/common/lib/client/modularplugins.ts
- CHANGELOG.md
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/common/lib/types/annotation.ts (5)
src/common/lib/types/message.ts (8)
fromEncoded
(46-55)fromEncodedArray
(57-68)_fromEncoded
(72-75)_fromEncodedArray
(77-83)fromValues
(135-137)fromValues
(165-167)encode
(128-133)decode
(193-199)src/common/lib/client/restchannel.ts (1)
logger
(65-67)ably.d.ts (3)
ChannelOptions
(959-979)Annotation
(3218-3273)AnnotationAction
(3364-3364)src/common/lib/util/utils.ts (1)
Properties
(471-471)src/common/lib/types/basemessage.ts (4)
encode
(97-107)strMsg
(365-384)wireToJSON
(273-281)decode
(137-150)
src/common/lib/client/realtimechannel.ts (4)
src/common/lib/types/basemessage.ts (3)
CipherOptions
(13-24)populateFieldsFromParent
(327-363)MessageEncoding
(319-324)src/common/lib/types/message.ts (1)
getMessagesSize
(93-101)src/common/lib/types/protocolmessagecommon.ts (1)
actions
(4-27)src/common/lib/types/presencemessage.ts (1)
WirePresenceMessage
(119-150)
src/common/lib/types/message.ts (10)
ably.d.ts (5)
MessageAction
(3340-3345)ChannelOptions
(959-979)Message
(3131-3213)Crypto
(3513-3528)Operation
(3290-3303)src/common/lib/util/utils.ts (1)
Properties
(471-471)src/common/types/channel.d.ts (1)
ChannelOptions
(3-10)src/common/lib/types/basemessage.ts (7)
normalizeCipherOptions
(47-61)CipherOptions
(13-24)encode
(97-107)strMsg
(365-384)wireToJSON
(273-281)EncodingDecodingContext
(26-34)decode
(137-150)src/common/lib/client/restchannel.ts (1)
logger
(65-67)src/platform/web/lib/http/http.ts (1)
logger
(149-151)src/common/types/http.ts (1)
logger
(134-136)src/common/lib/types/annotation.ts (5)
fromEncodedArray
(23-33)_fromEncoded
(37-39)_fromEncodedArray
(41-50)encode
(63-70)decode
(103-114)src/common/lib/types/presencemessage.ts (5)
fromEncodedArray
(26-37)_fromEncoded
(41-46)_fromEncodedArray
(48-57)encode
(90-95)decode
(134-145)src/common/types/ICryptoStatic.ts (1)
IUntypedCryptoStatic
(23-25)
src/platform/react-hooks/src/hooks/useChannelAttach.ts (5)
ably.d.ts (1)
ConnectionState
(149-157)src/platform/react-hooks/src/hooks/useAbly.ts (1)
useAbly
(5-15)src/platform/react-hooks/src/hooks/useConnectionStateListener.ts (1)
useConnectionStateListener
(15-28)src/platform/react-hooks/src/hooks/constants.ts (1)
INACTIVE_CONNECTION_STATES
(3-3)src/platform/react-hooks/src/utils.ts (1)
logError
(7-14)
test/realtime/annotations.test.js (5)
test/realtime/presence.test.js (5)
chai
(4-4)realtime
(1619-1619)realtime
(1702-1702)channel
(1620-1620)channel
(1703-1703)src/common/lib/client/baseclient.ts (1)
rest
(108-113)test/realtime/channel.test.js (23)
helper
(155-155)helper
(175-175)helper
(213-213)helper
(240-240)helper
(270-270)helper
(308-308)helper
(343-343)helper
(384-384)helper
(414-414)helper
(448-448)helper
(488-488)helper
(510-510)helper
(543-543)helper
(606-606)helper
(661-661)realtime
(1829-1829)realtime
(1848-1848)realtime
(1872-1872)realtime
(1912-1912)channel
(1830-1830)channel
(1851-1851)channel
(1875-1875)channel
(1915-1915)src/common/lib/client/realtimechannel.ts (1)
annotations
(73-78)src/common/lib/client/restchannel.ts (1)
annotations
(37-42)
🪛 Biome (1.9.4)
src/common/lib/types/basemessage.ts
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 197-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 15-15: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
[error] 20-20: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
src/common/lib/client/realtimepresence.ts
[error] 132-133: This case is falling through to the next case.
Add a break
or return
statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
test/realtime/annotations.test.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🪛 LanguageTool
README.md
[grammar] ~77-~77: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...m/lib/ably.min-2.js"></script> ``` The Ably client library follows [Semantic Versioning](h...
(A_RB_NN)
🪛 markdownlint-cli2 (0.17.2)
README.md
77-77: Bare URL used
null
(MD034, no-bare-urls)
77-77: Bare URL used
null
(MD034, no-bare-urls)
77-77: Bare URL used
null
(MD034, no-bare-urls)
77-77: Bare URL used
null
(MD034, no-bare-urls)
77-77: Bare URL used
null
(MD034, no-bare-urls)
77-77: Spaces inside emphasis markers
null
(MD037, no-space-in-emphasis)
🪛 GitHub Check: lint
src/platform/react-hooks/src/hooks/useChannelAttach.ts
[warning] 42-42:
React Hook useEffect has a missing dependency: 'ably'. Either include it or remove the dependency array
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test-browser (webkit)
- GitHub Check: test-node (20.x)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-node (18.x)
- GitHub Check: test-browser (chromium)
- GitHub Check: test-node (16.x)
🔇 Additional comments (83)
README.md (2)
73-74
: Library version upgrade in CDN link.The CDN link has been updated to reference version 2 of the Ably JavaScript client library, which aligns with the broader version 2.6.x release mentioned in the PR summary.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
73-73: Code block style
Expected: indented; Actual: fenced(MD046, code-block-style)
77-77
: Version references updated to reflect version 2.The documentation about semantic versioning has been appropriately updated to show examples for the v2 series (2.js, 2.6.js, 2.6.3.js).
Note: The static analysis flagged bare URLs in this line, but this is acceptable in this context as these are examples of CDN links.
🧰 Tools
🪛 LanguageTool
[grammar] ~77-~77: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...m/lib/ably.min-2.js"></script> ``` The Ably client library follows [Semantic Versioning](h...(A_RB_NN)
🪛 markdownlint-cli2 (0.17.2)
77-77: Bare URL used
null(MD034, no-bare-urls)
77-77: Bare URL used
null(MD034, no-bare-urls)
77-77: Bare URL used
null(MD034, no-bare-urls)
77-77: Bare URL used
null(MD034, no-bare-urls)
77-77: Bare URL used
null(MD034, no-bare-urls)
77-77: Spaces inside emphasis markers
null(MD037, no-space-in-emphasis)
src/platform/react-hooks/src/hooks/useChannelAttach.ts (1)
12-47
: Well-structured hook for channel attachment management.This new hook provides a clean way to manage the attachment lifecycle of an Ably Realtime channel based on connection state. It tracks connection state changes and attaches the channel asynchronously when appropriate, returning the current connection state for reuse in related hooks.
🧰 Tools
🪛 GitHub Check: lint
[warning] 42-42:
React Hook useEffect has a missing dependency: 'ably'. Either include it or remove the dependency arraytest/realtime/annotations.test.js (2)
30-70
: Comprehensive test for annotation publishing and subscribing.The test thoroughly validates the annotation lifecycle, including publishing from realtime and REST clients, verifying message fields and expected behavior when annotations are created.
72-124
: Well-structured pagination test for REST annotations retrieval.This test thoroughly verifies the REST API for retrieving message annotations, including pagination capabilities. The test publishes multiple annotations and then verifies that they can be retrieved with correct pagination handling and properly ordered results.
test/realtime/message.test.js (4)
1278-1314
: Updated message action handling tests.The test suite has been renamed and refactored to use async/await for calls to
Message.fromEncoded
andmessage.encode()
. The action codes have been standardized (e.g., 0 for create actions), improving consistency with the implementation.
1315-1332
: Added test for message metadata population.Good addition of tests to verify that messages with action
create
(0) properly populateserial
andcreatedAt
fields fromversion
andtimestamp
, while other actions do not. This ensures correct metadata handling across different message types.
1443-1479
: Improved test reliability with setTimeout.The timeout ensures that all pending I/O operations and message processing complete before assertions are run, avoiding potential race conditions in the test suite. This is a good practice for asynchronous testing.
1491-1529
: Added robustness test for unrecognized message actions.This new test verifies that receiving protocol messages with unrecognized actions does not close or fail the realtime connection, enhancing robustness. This is important for ensuring backward compatibility and graceful handling of unexpected messages.
src/common/lib/client/realtimepresence.ts (8)
4-5
: Imports look correct.
Bringing inPresenceMessage
andWirePresenceMessage
frompresencemessage.ts
andCipherOptions
frombasemessage.ts
is consistent with the new architecture.
118-126
: Consider error handling on encode.const presence = PresenceMessage.fromData(data); presence.action = action; … const wirePresMsg = await presence.encode(channel.channelOptions as CipherOptions);This block looks fine, but if
presence.encode
throws (e.g., due to invalid data), the error will bubble up. Verify that upstream logic can handle this gracefully.
130-138
: Intentional switch fallthrough.
Lines 132-133 fall through to theattaching
case. There is already an// eslint-disable-next-line no-fallthrough
comment, so it appears intentional. If that’s desired, you can keep it; otherwise, insert a break or refactor to handle each case distinctly to silence linters.🧰 Tools
🪛 Biome (1.9.4)
[error] 132-133: This case is falling through to the next case.
Add a
break
orreturn
statement to the end of this case to prevent fallthrough.(lint/suspicious/noFallthroughSwitchClause)
172-197
: Check consistency for leaving presence in various states.switch (channel.state) { case 'attached': … case 'attaching': … case 'initialized': case 'failed': throw new PartialErrorInfo('…'); default: throw channel.invalidStateError(); }Consider whether states like
'detached'
or'suspended'
need handling. Ensure this logic stays consistent with_enterOrUpdateClient
to avoid unexpected errors.
282-282
: For-of loop is fine.
Using a for-of onpresenceSet
enhances clarity and avoids index-handling overhead.
313-313
: Action non-null assertion.this.subscriptions.emit(presence.action!, presence);Ensure
presence.action!
is always defined at runtime. If the code or upstream logic might ever produce undefined, this could throw an error.
354-357
: Promise chain for sending presence.this.channel .sendPresence(presenceArray) .then(() => multicaster()) .catch((err: ErrorInfo) => multicaster(err));Looks good; straightforward chain. Consider adding a
.finally()
if there is any shared cleanup logic needed, but otherwise this is fine.
454-458
: Conditional attach on subscribe.if (channel.channelOptions.attachOnSubscribe !== false) { await channel.attach(); }This is a helpful convenience feature. If a client wants manual attaching, they can override with
attachOnSubscribe = false
.src/common/lib/types/annotation.ts (6)
1-11
: Initial imports and setup.import Logger from '../util/logger'; import { BaseMessage, encode, decode, wireToJSON, strMsg } from './basemessage'; … import type { Properties } from '../util/utils'; import type RestChannel from '../client/restchannel'; import type RealtimeChannel from '../client/realtimechannel'; import type { ChannelOptions } from '../../types/channel';All relevant utilities and types are pulled in. This file depends on
basemessage.ts
for core encode/decode logic, consistent with the new approach.
12-13
:actions
array for annotation.const actions = ['annotation.create', 'annotation.delete'];Defining an array for annotation actions is consistent with the approach in other message types.
35-55
: Internal_fromEncoded
variants.export async function _fromEncoded(…) export async function _fromEncodedArray(…)These are for decoding with an existing channel, bypassing cipher normalization. This is consistent with the approach in other message types.
57-83
:Annotation
class basics.class Annotation extends BaseMessage { … async encode(): Promise<WireAnnotation> { … } static fromValues(…): Annotation { … } … }
- The
encode()
method does not pass cipher options, aligning with the comment that annotation data must be server-parseable.- The class extends
BaseMessage
for consistent encoding and decoding flow.
85-119
:WireAnnotation
class.class WireAnnotation extends BaseMessage { … async decode(channelOptions: ChannelOptions, logger: Logger): Promise<Annotation> { … await decode(res, channelOptions); … } }
- This mirrors
WireMessage
andWirePresenceMessage
.- The catch block logs errors but does not rethrow; presumably the caller handles partial results.
121-121
: Default export.export default Annotation;Makes sense for the primary class. Keep an eye on naming clarity if exporting both
Annotation
andWireAnnotation
from the same file, but it seems consistent with other patterns.src/common/lib/types/protocolmessage.ts (9)
4-4
: Annotations plugin import.import { AnnotationsPlugin } from '../client/modularplugins';This addition aligns with the new annotation functionality.
7-9
: Switch toWireMessage
,WirePresenceMessage
,WireAnnotation
.import { WireMessage } from './message'; import PresenceMessage, { WirePresenceMessage } from './presencemessage'; import Annotation, { WireAnnotation } from './annotation';This reflects the central theme of wire vs. high-level message objects.
29-39
:deserialize()
withannotationsPlugin
.export function deserialize( serialized: unknown, … annotationsPlugin: AnnotationsPlugin | null, … ) { … return fromDeserialized(deserialized, presenceMessagePlugin, annotationsPlugin, objectsPlugin); }Now includes the optional
annotationsPlugin
argument. This is consistent with the presence approach.
41-81
:fromDeserialized()
extended for annotations.let annotations: WireAnnotation[] | undefined; if (annotationsPlugin && deserialized.annotations) { annotations = annotationsPlugin.WireAnnotation.fromValuesArray(…); } … return Object.assign(new ProtocolMessage(), { … annotations, … });
- Good: We only decode annotations if the plugin is present.
- The rest uses the standard approach for presence, messages, and states.
89-100
:makeFromDeserializedWithDependencies()
test utility.return fromDeserialized( deserialized, { PresenceMessage, WirePresenceMessage, }, { Annotation, WireAnnotation, RealtimeAnnotations, RestAnnotations }, dependencies?.ObjectsPlugin ?? null, );This factory approach is consistent with prior patterns, letting tests supply a plugin environment.
103-103
:fromValues()
helper.export function fromValues(values: Properties<ProtocolMessage>): ProtocolMessage { return Object.assign(new ProtocolMessage(), values); }No concerns here — matches the pattern for converting plain objects to typed instances.
107-150
:stringify()
extended for annotations.if (msg.annotations && annotationsPlugin) { result += '; annotations=' + toStringArray(…); }Ensures annotation data is included for debugging and logs. Straightforward extension of existing approach for messages/presence.
🧰 Tools
🪛 Biome (1.9.4)
[error] 134-134: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
153-176
: Additional fields inProtocolMessage
.annotations?: WireAnnotation[]; params?: Record<string, string>; …
annotations?: WireAnnotation[];
is consistent with the plugin-based approach. The newparams
field is also introduced. Ensure no sensitive data is stored inparams
inadvertently.
181-187
:setFlag
and fallthrough logic.setFlag(flag: keyof typeof flags): number { return (this.flags = (this.flags as number) | flags[flag]); }This is standard bitwise logic for flags. No issues.
src/common/lib/types/message.ts (13)
2-11
: Refactor to import shared utilities frombasemessage.ts
.import { BaseMessage, encode, decode, wireToJSON, normalizeCipherOptions, EncodingDecodingContext, CipherOptions, strMsg, } from './basemessage';Centralizing encode/decode logic in
basemessage.ts
is a major improvement, removing duplication.
15-21
: Crypto and channel definitions.import type { IUntypedCryptoStatic } from 'common/types/ICryptoStatic'; import type { ChannelOptions } from '../../types/channel'; import type { Properties } from '../util/utils'; … type Channel = RestChannel | RealtimeChannel;Matches the approach in other message-related files, ensuring consistent references.
23-24
: Message actions array.const actions: API.MessageAction[] = [ … ];This mirrors the presence/annotation pattern of an array-based action definition.
25-27
:stringifyAction()
helper.function stringifyAction(action: number | undefined): string { … }Offers a quick fallback to
'unknown'
for out-of-range action indices. Fine approach.
29-29
:getMessageSize()
introduced.
This function calculates approximate message size in bytes. Make sure that for complexextras
ordata
, JSON size is measured accurately.
46-55
:fromEncoded()
logic.export async function fromEncoded(logger, Crypto, encoded, inputOptions?) { const options = normalizeCipherOptions(Crypto, logger, inputOptions ?? null); const wm = WireMessage.fromValues(encoded); return wm.decode(options, logger); }Ensures a uniform decode flow with optional crypto. Nicely done.
70-75
: Internal_fromEncoded
variants.export async function _fromEncoded(encoded: Properties<WireMessage>, channel: Channel): Promise<Message> { const wm = WireMessage.fromValues(encoded); return wm.decode(channel.channelOptions, channel.logger); }Identical logic except skipping cipher normalization. This pattern is consistent with presence and annotation code.
85-87
:encodeArray()
function.export async function encodeArray(messages: Array<Message>, options: CipherOptions): Promise<Array<WireMessage>> { return Promise.all(messages.map((message) => message.encode(options))); }A straightforward batch-encoding approach.
103-134
:Message
class.class Message extends BaseMessage { … }
- Uses
expandFields()
forversion → serial
ortimestamp → createdAt
conversions.encode()
calls the sharedencode
utility.- Great example of removing duplication and centralizing the logic in
basemessage.ts
.
135-141
:Message.fromValues()
variants.static fromValues(values: Properties<Message>): Message { … } static fromValuesArray(values: Properties<Message>[]): Message[] { … }This remains consistent with the other wire-based classes.
143-145
:toString()
override.toString() { return strMsg(this, 'Message'); }Straightforward debugging helper, consistent with other message types.
148-203
:WireMessage
class refactoring.class WireMessage extends BaseMessage { … async decode(…) { const { decoded } = await this.decodeWithErr(…); return decoded; } … toString() { return strMsg(this, 'WireMessage'); } }
decodeWithErr
pattern is flexible if partial decode must be handled.- The rest matches presence/annotation wire classes. This is a good design to unify message handling.
206-206
: No changes visible at line 206
No content is shown in the snippet for line 206, so no relevant comment.src/common/lib/client/realtimechannel.ts (12)
66-66
: LGTM: Proper initialization of _annotations property.Adding the
_annotations
property initialized to null is consistent with how the_presence
property is handled, maintaining a clear pattern for plugin-dependent features.
73-78
: LGTM: Well-implemented annotations getter.The getter implementation correctly follows the established pattern for plugin-dependent features, throwing an appropriate error when the plugin is missing.
89-89
: LGTM: Simplified initialization of _mode property.Initializing
_mode
directly to 0 simplifies the code while maintaining the same functionality.
113-115
: LGTM: Proper initialization of annotations in constructor.This code correctly instantiates the
RealtimeAnnotations
class when the plugin is available, following the same pattern used for other plugins.
289-296
: LGTM: Good extraction of validation logic.Creating a separate
throwIfUnpublishableState
method improves code organization and readability.
429-433
: LGTM: Modernized Promise-based implementation.Refactoring to use Promise API with async/await instead of callbacks makes the code more maintainable and easier to reason about.
449-454
: LGTM: Added support for conditional channel attachment.The
attachOnSubscribe
option provides flexibility in how channel subscription works, which is a nice improvement to the API.
493-503
: LGTM: Modernized API with Promise-based implementation.Converting callback-based methods to return Promises improves code quality and aligns with modern JavaScript practices.
607-617
: LGTM: Improved presence message handling.Using
populateFieldsFromParent
andPromise.all
for presence messages provides a more organized and efficient processing approach.
686-708
: LGTM: Improved message decoding with better error handling.The refactored message decoding with async/await and clearer error handling improves code maintainability and reliability.
716-729
: LGTM: Well-implemented annotation message handling.The new handling for
ANNOTATION
action follows the same pattern as other message types, with proper decoding and processing.
744-750
: LGTM: Improved handling of unrecognized message actions.Changing from aborting the connection to logging a major-level message for unrecognized actions improves forward compatibility and resilience.
src/common/lib/types/basemessage.ts (6)
36-45
: LGTM: Well-implemented context normalization.The
normaliseContext
function ensures consistent handling of different input types, providing good defaults and type safety.
97-107
: LGTM: Clear and focused encode function.The
encode
function has a well-defined responsibility, properly separating encoding and encryption concerns.
155-271
: LGTM: Comprehensive message decoding implementation.The
decodeData
function handles multiple encoding formats with careful error handling, providing a robust implementation for the complex decoding process.🧰 Tools
🪛 Biome (1.9.4)
[error] 197-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
319-325
: LGTM: Clean export of encoding utilities.The
MessageEncoding
object provides a well-organized export of related encoding functions, making the API more discoverable.
327-363
: LGTM: Well-implemented field population from parent message.The
populateFieldsFromParent
function handles different message types appropriately, ensuring consistent message metadata.
386-395
: LGTM: Clean base message definition.The
BaseMessage
abstract class provides a solid foundation for all message types with appropriate properties.ably.d.ts (20)
553-553
: Transport parameters now support boolean valuesThe
transportParams
field inClientOptions
has been updated to accept boolean values in addition to string and number values. This enhances flexibility for configuring connection parameters.
715-722
: Added new message modification capabilitiesNew capabilities have been added to support message modification operations:
message-update-any
: Update any messagesmessage-update-own
: Update own messages onlymessage-delete-any
: Delete any messagesmessage-delete-own
: Delete own messages onlyThese new capabilities provide more granular permission control for message operations.
865-893
: Updated channel modes to support both case variantsChannel modes now accept both uppercase and lowercase variants (
'PUBLISH'
and'publish'
) for backward compatibility. New annotation-related modes have been added:
ANNOTATION_PUBLISH
/annotation_publish
: Allows publishing annotationsANNOTATION_SUBSCRIBE
/annotation_subscribe
: Allows receiving annotations
908-909
: Extended ChannelMode type with annotation supportThe
ChannelMode
type has been updated to include the new annotation-related channel modes, enabling proper type checking when configuring channels for annotation capabilities.
911-954
: Added ResolvedChannelModes namespace and typeNew
ResolvedChannelModes
namespace andResolvedChannelMode
type have been introduced to represent the lowercase-only variants of channel modes. This is part of a planned migration path for standardizing on lowercase values in a future major release (as noted in GitHub issue #1954).
972-978
: Added attachOnSubscribe option to ChannelOptionsA new
attachOnSubscribe
boolean option has been added to control whether a channel should be implicitly attached when subscribing to it. This provides more control over channel attachment behavior, defaulting totrue
to maintain backward compatibility.
1022-1032
: Added GetAnnotationsParams interface for annotation retrievalNew interface for specifying parameters when retrieving message annotations, including a
limit
parameter to control the maximum number of annotations returned (default: 100, maximum: 1000).
2096-2194
: Added RealtimeAnnotations interfaceComprehensive interface for managing message annotations in realtime:
- Subscribe/unsubscribe to receive annotations
- Publish new annotations to messages
- Delete annotations from messages
- Retrieve annotations with pagination
The interface follows Ably's consistent pattern for resource management and includes clear documentation about the need to request the
annotation_subscribe
channel mode to receive individual annotations.
2736-2738
: Added annotations property to Channel interfaceThe
Channel
interface now includes anannotations
property that provides access toRestAnnotations
functionality, enabling annotation operations on REST channels.
2783-2820
: Added RestAnnotations interfaceNew interface for managing message annotations using the REST API:
- Publish annotations to messages
- Retrieve annotations with pagination
Similar to
RealtimeAnnotations
but with functionality appropriate for REST clients.
2898-2901
: Added annotations property to RealtimeChannel interfaceThe
RealtimeChannel
interface now includes anannotations
property that provides access toRealtimeAnnotations
functionality, enabling realtime annotation operations.
3003-3007
: Extended PublishOptions to support additional propertiesThe
PublishOptions
type now supports arbitrary additional string/number/boolean properties through an index signature. This allows for forward compatibility with new server-side publish options without requiring type definition updates.
3070-3126
: Added annotation summary type definitionsComprehensive type definitions for different annotation summary formats:
SummaryDistinctValues
: For distinct.v1 aggregationSummaryUniqueValues
: For unique.v1 aggregationSummaryMultipleValues
: For multiple.v1 aggregationSummaryClientIdList
: For flag.v1 aggregationSummaryClientIdCounts
: For counting aggregationsSummaryTotal
: For total.v1 aggregationThese types provide strongly-typed access to different annotation summary formats in the message summary.
3198-3212
: Added new Message properties for annotations supportTwo new properties added to the
Message
interface:
connectionKey
: Allows REST clients to publish on behalf of Realtime clientssummary
: Contains aggregated annotation data for the messageThese properties enable annotation functionality and cross-client publishing capabilities.
3215-3285
: Added Annotation and OutboundAnnotation interfacesNew interfaces define the structure of annotations:
Annotation
: Complete annotation object received from AblyOutboundAnnotation
: Simplified structure for creating/publishing annotationsThese interfaces provide type safety when working with message annotations.
3347-3364
: Added AnnotationActions namespace and typeNew namespace and type defining the possible annotation actions:
ANNOTATION_CREATE
: For creating a new annotationANNOTATION_DELETE
: For deleting an existing annotationThis provides type safety when working with annotation actions.
3369-3370
: Updated InboundMessage typeThe
InboundMessage
type has been updated to:
- Omit the
connectionKey
property (which is only used for outbound messages)- Require additional fields for type safety
This ensures type correctness when handling received messages.
3459-3477
: Added AnnotationStatic interface for decoding annotationsNew utility interface that provides methods for decoding annotation objects:
fromEncoded
: Decodes a single annotationfromEncodedArray
: Decodes an array of annotationsSimilar to existing Message and PresenceMessage static utilities, this provides consistent encoding/decoding capabilities for annotations.
3839-3842
: Added Annotation static property to Rest classThe
Rest
class now exposes theAnnotation
static utilities, providing access to annotation decoding capabilities for REST clients.
3894-3897
: Added Annotation static property to Realtime classThe
Realtime
class now exposes theAnnotation
static utilities, providing access to annotation decoding capabilities for Realtime clients.
Great idea to split it into multiple merge commits to isolate the interesting changes! |
It looks like the browser tests are failing on LiveObjects-related functionality, could you take a look please? |
I've had a brief look at 2a920d2, but it's still quite hard to understand what's going on there (not your fault!). I think that instead of me putting much time into understanding that, it probably makes more sense to get this PR merged (once tests are fixed) and then I do a deeper review on the into- |
Yea, totally understanable 😞 Can't really think what other dev process will help avoid these situations. Can't rebase
Makes sense. You can view the gist of the changes re encoding/decoding refactoring on a compare page: main...liveobjects/merge-main#diff-cea067adf28dca08e1effd8b2bbbc703aeb421c68529b8d9d4bee68331a657e6 (may need to switch to "Files changed" tab and scroll to |
Seems like browser tests can't resolve the reference to the |
Yes, this is definitely a good idea. |
Cool, I'll wait until the into- |
Fixed in #2011 - opened a separate PR as it is not related to the merge. That is PR is based on this merge PR so should expect a green CI there |
deep-equal [1] does not provide UMD-compatible bundle which causes our playwright tests to fail as they cannot resolve the module. Replace it with dequal [2] which has UMD-compatible bundle. (also apparently it should be faster than deep-equal) [1] https://www.npmjs.com/package/deep-equal [2] https://www.npmjs.com/package/dequal
Works for me. Please review those changes and approve this PR when ready, thanks |
…ser-tests Replace `deep-equal` with `dequal` package
This PR merges latest changes from the
main
branch into theintegration/liveobjects
branch. Most of the changes were trivial to resolve, as LiveObjects changes are mostly contained in a separate plugin and do not interfere with the core library.The merge was split into three separate merge steps to isolate a single PR (message types refactoring) that had the most conflicting changes. Most notable changes when resolving conflicts for that PR:
basemessage.ts
to use them in Objects pluginbasemessage.ts
to use them in Objects pluginSummary by CodeRabbit
Summary by CodeRabbit
New Features
useChannelAttach
) and improved channel/presence management in React Hooks.Bug Fixes
disableConnectivityCheck
option in browser environments.Documentation
Tests
Refactor
Chores