Introduce Path-based LiveObjects API - merge integration/objects-breaking-api into main
#2134
Conversation
WalkthroughRenames and replaces the legacy "Objects" plugin with a new "LiveObjects" implementation across build config, exports, types, runtime modules, tests and docs; removes legacy batch-context code and adds path/instance/batch primitives (RealtimeObject, DefaultPathObject, DefaultInstance, RootBatchContext, new typings and runtime wiring). Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Client as RealtimeClient
participant RO as RealtimeObject
participant RBC as RootBatchContext
participant Transport as Transport
participant Server as Ably Server
Dev->>Client: call instance.set / increment / batch(fn)
Client->>RO: ensureAttached() / get() (attach + wait sync)
Client->>RBC: batch(fn) -> RBC.queueMessages(msgCtor)
RBC-->>RBC: collect message constructors
Client->>RBC: flush()
RBC->>RBC: run msgCtors -> ObjectMessage[]
RBC->>RO: RO.publish(ObjectMessage[])
RO->>Transport: serialize & enqueue protocol message
Transport->>Server: send message
Server-->>Transport: broadcast / ack
Transport-->>RO: incoming updates
RO->>Client: notify instance/path subscriptions (objectMessage, _type, tombstone)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas to focus during review:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ably.d.ts (1)
2305-2361: AlignRealtimeObject.on()return type between d.ts and implementationThe
on()method is typed as returningStatusSubscriptionin ably.d.ts, but the implementation returnsOnObjectsEventResponsefrom src/plugins/objects/realtimeobject.ts. While both types have functionally equivalentoff()methods, they should use the same type for consistency. Either update the d.ts to referenceOnObjectsEventResponseor ensure the implementation usesStatusSubscription.
🧹 Nitpick comments (14)
src/plugins/objects/instance.ts (1)
216-230: Consider guarding against async batch callbacks
batch()relies onfnbeing synchronous (as per theBatchFunctioncontract and docs), but there’s no runtime guard. If a JS consumer accidentally passes anasyncfunction, operations queued after anawaitwill either be dropped or run afterflush()/close().You could defensively detect this and fail fast:
Suggested runtime guard
async batch<T extends LiveObjectType = LiveObjectType>(fn: BatchFunction<T>): Promise<void> { this._realtimeObject.throwIfInvalidWriteApiConfiguration(); if (!(this._value instanceof LiveObject)) { throw new this._client.ErrorInfo('Cannot batch operations on a non-LiveObject instance', 92007, 400); } const ctx = new RootBatchContext(this._realtimeObject, this); try { - fn(ctx as unknown as BatchContext<T>); + const result = fn(ctx as unknown as BatchContext<T>); + if (result && typeof (result as any).then === 'function') { + // Optional: choose a more specific code if you have one for API misuse + throw new this._client.ErrorInfo('BatchFunction must be synchronous; async functions are not supported', 40003, 400); + } await ctx.flush(); } finally { ctx.close(); } }ably.d.ts (1)
2363-4083: HavePrimitiveBatchContextextendBatchContextBasefor consistency
BatchContextBasedocumentsidas "present regardless of the underlying type", butPrimitiveBatchContextdoesn't extend it, whileLiveMapBatchContext,LiveCounterBatchContext, andAnyBatchContextdo. This causes generic code written againstBatchContextBaseto fail type-checking whenTincludes primitives. Sinceidwould beundefinedfor primitives at runtime anyway,PrimitiveBatchContext<T>should extendBatchContextBaseto match the documented behavior and simplify generic usage patterns.objects.d.ts (1)
86-86: Consider adding a proper type for the Objects export.The
Objectsexport is typed asany, which loses type safety for consumers. Consider defining a proper interface for the plugin object structure.🔎 Suggested improvement
-export declare const Objects: any; +export declare const Objects: { + LiveCounter: typeof LiveCounter; + LiveMap: typeof LiveMap; + ObjectMessage: typeof import('./ably').ObjectMessage; + RealtimeObject: typeof import('./ably').RealtimeObject; + WireObjectMessage: typeof import('./ably').WireObjectMessage; +};Alternatively, if there's a specific reason for using
any(e.g., internal plugin registration mechanics), document it with a comment.src/plugins/objects/rootbatchcontext.ts (1)
19-24: Constructor usesnull as anyfor self-referential root context.The pattern of passing
null as anyto the superclass and then immediately overwriting_rootContextworks but is unconventional. The superclassDefaultBatchContextexpects a validRootBatchContextand may access it during construction or in methods before line 23 executes.Consider whether making
_rootContexta protected setter or using a factory pattern would be cleaner, though this current approach is functional if the superclass constructor doesn't use_rootContext.src/plugins/objects/livecountervaluetype.ts (2)
30-37: Consider adding basic validation increate()for better developer experience.The comment explains why
ErrorInfocan't be used here, but you could still throw a plainErrororTypeErrorfor obviously invalid inputs (likeNaNor non-number types). This would give developers immediate feedback rather than deferring all errors to mutation time.static create(initialCount: number = 0): API.LiveCounter { if (typeof initialCount !== 'number' || !Number.isFinite(initialCount)) { throw new TypeError('initialCount must be a finite number'); } return new LiveCounterValueType(initialCount); }
56-58: Validation may be redundant given TypeScript and constructor constraints.The validation checks if
countis not a number or not finite, but:
- TypeScript enforces
initialCountasnumberincreate()- The constructor stores it directly
The only way to reach this with an invalid value is via JavaScript without TypeScript, or if someone casts maliciously. If this validation is intentional for JavaScript users, the code is correct. Otherwise, it's defensive but potentially unreachable.
src/plugins/objects/pathobjectsubscriptionregister.ts (1)
76-76: Subscription ID counter could theoretically overflow.Using an incrementing integer for subscription IDs will overflow after
Number.MAX_SAFE_INTEGERsubscriptions. While practically unlikely, consider using a string-based approach (e.g.,cheapRandStr) for robustness, especially in long-running applications.src/plugins/objects/objectmessage.ts (1)
8-17: Consider type-safe alternatives for operation action string mapping.The
operationActionsarray currently maps enum indices to strings, which works but lacks compile-time guarantees. If enum values are reordered or new operations added without updating the array, the mapping breaks silently. Consider using aMap<ObjectOperationAction, string>or a computed object to ensure type safety and prevent drift between the enum and lookup table.src/plugins/objects/realtimeobject.ts (1)
452-475: Consider extracting parent reference logic to LiveMap.The
_rebuildAllParentReferences()method has detailed knowledge ofLiveMapinternals (iterating entries, callingaddParentReference). This creates coupling betweenRealtimeObjectandLiveMap.Consider moving the parent reference establishment logic into
LiveMapitself, e.g., arebuildChildParentReferences()method thatLiveMapimplements, whichRealtimeObjectcan call polymorphically on all objects.src/plugins/objects/livemapvaluetype.ts (1)
128-143: Non-null assertion onoperation?.objectIdmay cause runtime issues if message creation fails silently.Lines 132 and 137 use non-null assertions (
operation?.objectId!) which could lead toundefinedbeing assigned if the message creation somehow produces a message without anobjectId. While unlikely given the current implementation, this pattern masks potential issues.Consider adding explicit validation or restructuring to avoid the non-null assertions.
🔎 Suggested approach
if (LiveMapValueType.instanceof(value)) { const { mapCreateMsg, nestedObjectsCreateMsgs: childNestedObjs } = await LiveMapValueType.createMapCreateMessage(realtimeObject, value); nestedObjectsCreateMsgs.push(...childNestedObjs, mapCreateMsg); - const typedObjectData: ObjectIdObjectData = { objectId: mapCreateMsg.operation?.objectId! }; + const objectId = mapCreateMsg.operation?.objectId; + if (!objectId) { + throw new realtimeObject.getClient().ErrorInfo('Failed to create objectId for nested map', 92000, 500); + } + const typedObjectData: ObjectIdObjectData = { objectId }; objectData = typedObjectData; } else if (LiveCounterValueType.instanceof(value)) { const counterCreateMsg = await LiveCounterValueType.createCounterCreateMessage(realtimeObject, value); nestedObjectsCreateMsgs.push(counterCreateMsg); - const typedObjectData: ObjectIdObjectData = { objectId: counterCreateMsg.operation?.objectId! }; + const objectId = counterCreateMsg.operation?.objectId; + if (!objectId) { + throw new realtimeObject.getClient().ErrorInfo('Failed to create objectId for nested counter', 92000, 500); + } + const typedObjectData: ObjectIdObjectData = { objectId }; objectData = typedObjectData;src/plugins/objects/batchcontext.ts (1)
62-83: Generator functions validate state once at start but mutations could occur during iteration.The generator functions (
entries,keys,values) checkthrowIfInvalidAccessApiConfiguration()and_throwIfClosed()only at the start of iteration. If the batch is closed or the channel state changes during iteration, subsequent yields would still proceed.This is likely acceptable for the expected use case (immediate iteration), but worth noting. If this is intentional, a brief comment would help future maintainers.
src/plugins/objects/liveobject.ts (1)
241-249: Consider potential performance concern with path construction.The
[key, ...currentPath]spread operator on line 245 creates a new array for each parent-key combination. For deeply nested objects with many parents, this could create significant garbage. Consider using a mutable approach and only copying when reaching the root.🔎 Optional optimization using reverse at the end:
- for (const [parent, keys] of obj._parentReferences) { - for (const key of keys) { - stack.push({ - obj: parent, - currentPath: [key, ...currentPath], - visited: newVisited, - }); - } - } + for (const [parent, keys] of obj._parentReferences) { + for (const key of keys) { + stack.push({ + obj: parent, + currentPath: [...currentPath, key], // append instead of prepend + visited: newVisited, + }); + } + } } - return paths; + // Reverse each path since we built them bottom-up + return paths.map(p => p.reverse());src/plugins/objects/pathobject.ts (1)
395-412: Potential resource leak iffnthrows beforeflush()is called.The
batch()method has propertry-finallyto callctx.close(), but iffn()is synchronous and throws, the pending operations in the batch context may be lost. Consider whetherflush()should be called in thefinallyblock as well (with error handling), or document that the batch is discarded on error.async batch<T extends LiveObjectType = LiveObjectType>(fn: BatchFunction<T>): Promise<void> { const instance = this._resolveInstance(); if (!instance) { throw new this._client.ErrorInfo( `Cannot batch operations on a non-LiveObject at path: ${this._escapePath(this._path).join('.')}`, 92007, 400, ); } const ctx = new RootBatchContext(this._realtimeObject, instance); try { fn(ctx as unknown as BatchContext<T>); await ctx.flush(); } finally { ctx.close(); } }If discarding on error is intentional, a comment would help clarify the design decision.
src/plugins/objects/livemap.ts (1)
764-797: Parent reference updates in _applyMapSet are correct but have duplication.The parent reference handling in
_applyMapSet()is correct, but there's some overlap with_updateParentReferencesFromUpdate(). The inline handling in_applyMapSetand_applyMapRemoveis necessary for operation-by-operation updates, while_updateParentReferencesFromUpdateis used for bulk state sync inoverrideWithObjectState().Consider adding a brief comment explaining why both are needed to prevent future confusion.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (34)
.gitignore(1 hunks)Gruntfile.js(2 hunks)ably.d.ts(5 hunks)grunt/esbuild/build.js(2 hunks)objects.d.ts(1 hunks)package.json(1 hunks)scripts/moduleReport.ts(1 hunks)src/common/lib/client/presencemap.ts(2 hunks)src/common/lib/client/realtimechannel.ts(9 hunks)src/common/lib/client/realtimepresence.ts(1 hunks)src/common/lib/util/utils.ts(1 hunks)src/platform/web/lib/util/bufferutils.ts(1 hunks)src/plugins/objects/batchcontext.ts(1 hunks)src/plugins/objects/batchcontextlivecounter.ts(0 hunks)src/plugins/objects/batchcontextlivemap.ts(0 hunks)src/plugins/objects/constants.ts(1 hunks)src/plugins/objects/index.ts(1 hunks)src/plugins/objects/instance.ts(1 hunks)src/plugins/objects/livecounter.ts(9 hunks)src/plugins/objects/livecountervaluetype.ts(1 hunks)src/plugins/objects/livemap.ts(23 hunks)src/plugins/objects/livemapvaluetype.ts(1 hunks)src/plugins/objects/liveobject.ts(7 hunks)src/plugins/objects/objectmessage.ts(8 hunks)src/plugins/objects/objectspool.ts(7 hunks)src/plugins/objects/pathobject.ts(1 hunks)src/plugins/objects/pathobjectsubscriptionregister.ts(1 hunks)src/plugins/objects/realtimeobject.ts(12 hunks)src/plugins/objects/rootbatchcontext.ts(1 hunks)src/plugins/objects/syncobjectsdatapool.ts(2 hunks)test/common/modules/private_api_recorder.js(5 hunks)test/package/browser/template/README.md(1 hunks)test/package/browser/template/src/index-objects.ts(3 hunks)typedoc.json(1 hunks)
💤 Files with no reviewable changes (2)
- src/plugins/objects/batchcontextlivecounter.ts
- src/plugins/objects/batchcontextlivemap.ts
🧰 Additional context used
🧬 Code graph analysis (12)
src/common/lib/util/utils.ts (1)
src/common/lib/types/errorinfo.ts (1)
ErrorInfo(35-66)
src/plugins/objects/rootbatchcontext.ts (2)
src/plugins/objects/batchcontext.ts (1)
DefaultBatchContext(17-138)ably.d.ts (3)
ObjectMessage(3942-3989)Instance(3840-3846)Value(2432-2432)
src/plugins/objects/livecountervaluetype.ts (6)
src/plugins/objects/index.ts (3)
LiveCounterValueType(7-7)RealtimeObject(10-10)ObjectMessage(9-9)objects.d.ts (1)
LiveCounter(49-59)ably.d.ts (4)
LiveCounter(2417-2420)ObjectMessage(3942-3989)ObjectOperation(3994-4013)ObjectData(4068-4073)src/common/lib/transport/messagequeue.ts (1)
count(14-16)src/plugins/objects/realtimeobject.ts (1)
RealtimeObject(38-492)src/plugins/objects/objectmessage.ts (1)
createInitialValueJSONString(261-286)
src/platform/web/lib/util/bufferutils.ts (1)
src/platform/nodejs/lib/util/bufferutils.ts (1)
Output(5-5)
src/common/lib/client/realtimepresence.ts (2)
src/common/lib/client/presencemap.ts (1)
PresenceMap(33-205)src/common/lib/types/errorinfo.ts (1)
ErrorInfo(35-66)
src/common/lib/client/realtimechannel.ts (2)
src/plugins/objects/index.ts (1)
RealtimeObject(10-10)src/plugins/objects/realtimeobject.ts (1)
RealtimeObject(38-492)
src/plugins/objects/objectmessage.ts (1)
ably.d.ts (5)
ObjectOperationAction(3916-3922)ObjectsMapSemantics(3937-3937)Primitive(2367-2375)ObjectOperation(3994-4013)ObjectData(4068-4073)
src/plugins/objects/index.ts (2)
src/plugins/objects/livecountervaluetype.ts (1)
LiveCounterValueType(20-97)src/plugins/objects/livemapvaluetype.ts (1)
LiveMapValueType(33-162)
src/plugins/objects/objectspool.ts (3)
src/plugins/objects/index.ts (1)
RealtimeObject(10-10)src/plugins/objects/realtimeobject.ts (1)
RealtimeObject(38-492)src/plugins/objects/constants.ts (1)
ROOT_OBJECT_ID(1-1)
src/plugins/objects/liveobject.ts (4)
src/plugins/objects/objectmessage.ts (1)
ObjectMessage(358-462)src/plugins/objects/instance.ts (1)
InstanceEvent(23-26)src/plugins/objects/constants.ts (1)
ROOT_OBJECT_ID(1-1)src/plugins/objects/pathobjectsubscriptionregister.ts (1)
PathEvent(27-34)
src/plugins/objects/instance.ts (8)
ably.d.ts (10)
ObjectMessage(3942-3989)Value(2432-2432)AnyInstance(3771-3831)LiveObject(2425-2425)LiveMap(2409-2412)Instance(3840-3846)Primitive(2367-2375)LiveCounter(2417-2420)Subscription(1680-1689)BatchContext(3172-3178)src/plugins/objects/objectmessage.ts (1)
ObjectMessage(358-462)src/plugins/objects/realtimeobject.ts (1)
RealtimeObject(38-492)src/plugins/objects/livemap.ts (1)
LiveMap(48-1037)src/plugins/objects/batchcontext.ts (1)
value(38-42)src/plugins/objects/pathobject.ts (2)
value(174-213)instance(215-226)src/plugins/objects/livecounter.ts (2)
value(64-67)LiveCounter(15-301)src/plugins/objects/rootbatchcontext.ts (1)
RootBatchContext(6-73)
src/plugins/objects/livecounter.ts (3)
src/plugins/objects/liveobject.ts (2)
LiveObjectData(14-16)LiveObjectUpdate(18-26)src/plugins/objects/realtimeobject.ts (1)
RealtimeObject(38-492)src/plugins/objects/objectmessage.ts (1)
ObjectMessage(358-462)
🔇 Additional comments (89)
.gitignore (1)
7-7: LGTM!Correctly ignoring the generated
objects.d.mtsfile, which aligns with the newbuild:objects:typestask that generates it fromobjects.d.ts.typedoc.json (1)
22-23: LGTM!The removal of the
intentionallyNotExportedproperty for__global.AblyObjectsTypesaligns with the broader API refactor that replaces the legacy Objects surface with the new RealtimeObject/PathObject-based model.src/common/lib/util/utils.ts (1)
493-531: Well-implemented async iterator pattern.The implementation correctly:
- Queues events when no consumer is awaiting
- Resolves immediately when a consumer is waiting
- Detects concurrent
next()calls- Cleans up the listener in the
finallyblockOne consideration: the
eventQueuecan grow unboundedly if the consumer processes events slower than they arrive. This is acceptable for typical event subscription patterns, but worth noting if high-frequency events are expected.grunt/esbuild/build.js (1)
88-95: LGTM!The ESM configuration correctly:
- Overrides
formatto'esm'for ES module output- Clears the
pluginsarray since the UMD wrapper isn't needed for ESM- Uses the
.mjsextension for the output file- Maintains
dequalas an external dependencyThis follows the same pattern as
modularConfigon lines 33-39.Gruntfile.js (3)
141-156: LGTM!The bundle task correctly adds the new ESM build alongside the existing UMD and CDN builds using
Promise.allfor parallel execution.
169-169: LGTM!The composite task properly sequences the bundle and types tasks, ensuring types are generated after the bundle is complete.
158-167: The regex handles all import patterns in the file correctly. Verification ofobjects.d.tsconfirms it only uses single-quoted./imports without existing.jsextensions, so the current regex pattern is sufficient for this specific use case. No changes needed.src/common/lib/client/realtimepresence.ts (1)
182-205: Clean async/await refactoring.The changes simplify the presence retrieval logic by:
- Using the new
channel.ensureAttached()instead of manual state management- Replacing callback/promise mixture with straightforward async/await
- Encapsulating result transformation in the local
toMessageshelperThe suspended state handling correctly differentiates between:
waitForSync: true→ throws an error (presence state is stale)waitForSync: false→ returns stale data (explicit opt-in by caller)src/plugins/objects/instance.ts (1)
46-79: Compaction and JSON-compaction behaviour looks consistent
compact()andcompactJson()correctly delegate toLiveMap’s compaction for map instances and fall back tovalue()plus base64 encoding for primitive/binary values. This matches theCompactedValue/CompactedJsonValuecontracts and the PathObject semantics, so no changes needed here.src/plugins/objects/constants.ts (1)
1-1: Canonical root object ID constant looks good
ROOT_OBJECT_ID = 'root'provides a clear single source of truth for the root object identifier; no issues here.src/platform/web/lib/util/bufferutils.ts (1)
8-11: Output alias now correctly reflects web runtime return typeSwitching
OutputtoArrayBuffermatches what allBufferUtilsmethods on web already return (toArrayBuffer(...)orUint8Array.buffer). This tightens the typing without changing behaviour and stays compatible with the node implementation via the genericIBufferUtils<Bufferlike, Output, ToBufferOutput>.test/package/browser/template/README.md (1)
10-12: README correctly updated for named Objects exportThe README now reflects the breaking change from a default to a named export (
import { Objects } from 'ably/objects'), keeping the browser template instructions aligned with the new packaging.ably.d.ts (1)
1649-1711: New callback and subscription abstractions are clear and cohesiveThe introduction of
EventCallback<T>,BatchFunction<T>, and theSubscription/StatusSubscriptioninterfaces gives a clean, reusable shape to event APIs and one-off lifecycle subscriptions. The JSDoc examples match the shapes, and the separation betweenunsubscribe()andoff()reads well. No naming conflicts with legacy definitions exist elsewhere in the codebase.src/common/lib/client/presencemap.ts (1)
179-192: LGTM! Clean migration to Promise-based API.The conversion from callback-based
waitSync(callback)to asyncwaitSync(): Promise<void>is well-implemented. The early return when no sync is in progress prevents unnecessary event listener registration, andawait this.once('sync')correctly waits for the sync event.This is a breaking change for any existing callers that used the callback pattern. Ensure all call sites have been updated accordingly.
package.json (2)
33-42: Well-structured conditional exports for dual ESM/CJS support.The exports configuration correctly provides separate entry points for ESM (
import) and CJS (require) with corresponding type declarations. Thetypesfield is properly placed beforedefaultin each condition block, which is required for correct TypeScript resolution.This is a breaking change as consumers will need to update from
import Objects from 'ably/objects'toimport { Objects } from 'ably/objects'.
48-48: LGTM!Adding
objects.d.mtsto the files array ensures the ESM type declarations are included in the published package.scripts/moduleReport.ts (1)
329-346: LGTM! Updated allowedFiles reflects the new module structure.The additions to
checkObjectsPluginFilescorrectly enumerate the new RealtimeObject-centric modules:
instance.ts,realtimeobject.ts,rootbatchcontext.tsfor core object handlingpathobject.ts,pathobjectsubscriptionregister.tsfor path-based accesslivecountervaluetype.ts,livemapvaluetype.tsfor value type factoriesThis aligns with the broader API migration from Objects to RealtimeObject.
src/plugins/objects/syncobjectsdatapool.ts (2)
4-4: LGTM!Import updated to align with the RealtimeObject-centric model.
30-34: LGTM! Constructor updated to accept RealtimeObject.The constructor signature change from
ObjectstoRealtimeObjectis consistent with the broader API migration. The initialization pattern correctly extracts_clientand_channelfrom the providedRealtimeObject.Note: The TODO comment on line 21 suggests this class may need further review regarding its necessity after the createOp changes.
src/plugins/objects/index.ts (2)
1-12: LGTM! Clean export structure with aliasing.The named exports with aliasing (
LiveCounterValueType as LiveCounter) provide a cleaner public API while maintaining internal naming conventions. This supports the dual-use pattern where consumers can import individual utilities or the consolidated plugin object.
14-23: LGTM! Objects plugin export consolidates the public API.The
Objectsconst provides a single object for plugin registration with the Ably client while the named exports above allow granular imports. The JSDoc clearly explains the export's purpose.test/common/modules/private_api_recorder.js (5)
17-17: LGTM!Adding
call.EventEmitter.listenersto track the new EventEmitter usage.
29-31: LGTM! Updated private API identifiers for RealtimeObject.The identifiers correctly reflect the Objects → RealtimeObject rename and include the new
getPathObjectSubscriptionRegisterfor path-based subscriptions.
81-91: LGTM! New read identifiers for the updated object model.The additions cover the new internal structures:
DefaultInstance._valuefor instance value accessLiveMap._dataRef.dataandLiveObject._subscriptionsfor internal data accessPathObjectSubscriptionRegister._subscriptionsfor path subscriptionsRealtimeObject._DEFAULTS.gcGracePeriodandgcGracePeriodfor GC configuration
131-132: LGTM!Replace identifiers updated for RealtimeObject namespace.
151-152: LGTM!Write identifiers updated for RealtimeObject namespace.
src/plugins/objects/objectspool.ts (7)
2-2: LGTM!Importing
ROOT_OBJECT_IDfrom constants centralizes the magic string.
19-21: LGTM! Constructor updated to accept RealtimeObject.The constructor correctly extracts the client from the RealtimeObject and initializes the pool.
33-43: LGTM! New accessor methods for root and all objects.
getRoot()provides type-safe access to the root LiveMap, andgetAll()returns an efficient iterator for processing all objects in the pool. The type assertion ingetRoot()is safe since the root is always initialized as a LiveMap per spec RTO3b.
65-65: LGTM!Using the new
getRoot()accessor instead of direct pool access.
96-102: LGTM!
zeroValuefactory calls updated to pass_realtimeObjectinstead of the old_objectsreference.
112-112: LGTM!Root initialization updated to use
_realtimeObject.
117-132: LGTM!GC logic correctly references
this._realtimeObject.gcGracePeriodfor the tombstone grace period check.objects.d.ts (5)
3-9: LGTM!The imports correctly alias types to avoid naming conflicts with the classes defined in this file.
13-25: Well-documented NoInfer polyfill.The
NoInfer<T>utility type correctly implements the polyfill pattern for TypeScript versions prior to 5.4. The JSDoc thoroughly explains both the purpose and the implementation mechanism (deferred conditional types preventing inference).
27-44: LGTM! LiveMap.create with NoInfer for improved type inference.Using
NoInfer<T>oninitialEntriesforces TypeScript to infer the type from the contextual usage (e.g.,LiveMap.setcall) rather than from the argument itself. This improves the developer experience by providing better type inference in mutation contexts.
46-59: LGTM!
LiveCounter.createprovides a clean factory method for creating counter value types with an optional initial count.
61-85: LGTM! Comprehensive documentation for the Objects plugin.The JSDoc provides clear examples for:
- Plugin registration with
RealtimeandBaseRealtime- Named imports for individual utilities (
LiveCounter,LiveMap)The
@experimentaltags on the factory methods appropriately signal the API stability.src/plugins/objects/rootbatchcontext.ts (2)
27-40: Verify error handling during flush.The
flush()method runs all message constructors in parallel and publishes results. If any constructor throws,Promise.allwill reject and thefinallyblock will still clean up state. However, the caller won't know which messages succeeded vs failed.Consider whether partial failures should be handled differently (e.g., collecting results with
Promise.allSettledand reporting which operations failed).
52-67: LGTM on wrapInstance memoization logic.The memoization by
objectIdcorrectly ensures consistent wrappers for the same object within a batch. Objects without IDs (e.g., value types not yet persisted) correctly get fresh wrappers each time.src/common/lib/client/realtimechannel.ts (4)
20-20: LGTM on import update.The import correctly reflects the rename from
ObjectstoRealtimeObject.
140-142: LGTM on plugin initialization.The constructor correctly instantiates
RealtimeObjectvia the plugin system, consistent with how other plugins (Push, Annotations) are initialized.
152-158: LGTM on getter rename.The accessor is correctly renamed from
objectstoobject, maintaining the error handling pattern for missing plugins.
553-555: LGTM on _object usage updates.All references to the object plugin are correctly updated from
_objectsto_object, maintaining consistent behavior for attachment and state change notifications.Also applies to: 801-802
src/plugins/objects/livecountervaluetype.ts (2)
20-28: LGTM on immutable value type pattern.Using
Object.freeze()combined with private constructor ensures immutability. The dual type identification approach (compile-time symbol + runtime_livetype) handles cross-bundle scenarios well.
49-88: LGTM on message creation logic.The async message creation correctly:
- Retrieves client timestamp
- Generates nonce
- Computes deterministic objectId from initial value
- Constructs the full ObjectMessage with COUNTER_CREATE action
src/plugins/objects/pathobjectsubscriptionregister.ts (2)
112-128: LGTM on error isolation pattern.The try-catch around listener invocation correctly prevents one failing subscription from affecting others. Logging at
LOG_MINORlevel is appropriate for user callback errors.
152-178: Well-documented depth filtering logic.The docstring clearly explains the depth calculation with examples. The implementation correctly handles:
- Non-bubbling events (exact match only)
- Path prefix matching
- Infinite depth when
undefined- Relative depth calculation
src/plugins/objects/objectmessage.ts (3)
340-351: LGTM on stringifyOperation helper.Correctly converts internal enum-based operation to user-facing string-based format. Falls back to
'unknown'for unrecognized values, which is defensive.
776-780: LGTM on early return optimization.Short-circuiting when
objectIdis present avoids unnecessary processing for object references. The return structure correctly includes onlyobjectId.
438-445: LGTM on type predicate methods.
isOperationMessage()andisSyncMessage()provide clean boolean checks for message type discrimination.src/plugins/objects/realtimeobject.ts (6)
10-12: LGTM!The new imports for
DefaultPathObjectandPathObjectSubscriptionRegisteralign with the path-based API migration. The ObjectsPool import change is consistent with the refactoring.
38-68: LGTM!The class rename to
RealtimeObjectand the addition of_pathObjectSubscriptionRegisterare well-structured. The initialization order is correct, with the subscription register created after the objects pool is available.
77-95: LGTM!The
get()method properly:
- Validates channel mode before proceeding
- Ensures channel attachment via
ensureAttached()- Waits for sync completion if not yet synced
- Returns a new
DefaultPathObjectwrapping the root with an empty path arrayThe API transition from
getRoot()returningLiveMap<T>toget()returningPathObject<LiveMap<T>>is cleanly implemented.
145-150: LGTM!The new internal accessor
getPathObjectSubscriptionRegister()is properly marked with@internalJSDoc and follows the existing pattern of internal accessors in this class.
326-329: LGTM!The explicit typing for
existingObjectUpdatesis clearer and more maintainable than an inline anonymous type.
368-374: LGTM!Rebuilding parent references after sync is necessary since objects may reference other objects that weren't yet in the pool when initially created. The invocation point after all objects are synced is correct.
src/plugins/objects/livemapvaluetype.ts (3)
33-43: LGTM!The branded type pattern with both a type-only symbol (
[API.__livetype]) and a runtime field (_livetype) is a solid approach for cross-bundle type identification. UsingObject.freeze()enforces immutability as documented.
45-54: LGTM!The decision to defer validation to mutation methods rather than in this static factory is reasonable given the constraints around importing
ErrorInfointo plugin code. The comment adequately explains this design choice.
56-61: LGTM!The
instanceoftype guard using the runtime_livetypefield is a reliable pattern for cross-bundle detection where the nativeinstanceofoperator would fail.test/package/browser/template/src/index-objects.ts (4)
1-4: LGTM!The import changes correctly reflect the new API structure:
- Named exports from
'ably'for type-related exportsObjectsplugin from'ably/objects'as a named export (aligned with the breaking change noted in PR objectives)
12-26: LGTM!The
MyCustomObjecttype definition comprehensively covers all supported value types:
- Primitives:
number,string,boolean- Optional fields:
couldBeUndefined- Nested LiveMap with further nesting
- LiveCounter
- Binary types:
ArrayBuffer,BufferThis provides good test coverage for the type system.
35-65: LGTM!The type assertions verify:
channel.object.get<T>()returnsAbly.PathObject<LiveMap<T>>- PathObject methods (
size(),get(),value()) work with correct return types- Nested access via
map.get('nestedMap')?.get('baz')preserves literal types- Subscription callback receives correctly typed
objectandmessageThe use of nullish coalescing and non-null assertions is appropriate for a type-checking-only test.
67-112: LGTM!The
compact()andcompactJson()type assertions correctly verify:
CompactedValueunwrapsLiveCountertonumberand preservesArrayBuffer/BufferCompactedJsonValueconverts binary types tostring(base64) and includes the{ objectId: string }union for cyclic reference handlingThis thoroughly validates the compaction type transformations.
src/plugins/objects/batchcontext.ts (4)
17-26: LGTM!The class refactoring to
DefaultBatchContextimplementingAnyBatchContextis well-structured. The constructor correctly obtains the client fromRealtimeObjectand stores the required dependencies.
28-36: LGTM!The
get()method properly validates access configuration and closed state before delegating to the instance and wrapping the result viaRootBatchContext.
91-131: LGTM!The mutation methods (
set,remove,increment,decrement) properly:
- Validate write API configuration
- Check closed state
- Verify the instance type before operating
- Queue messages via
RootBatchContextusing async callbacksThe
decrementcorrectly delegates toincrementwith a negated value after its own type check.
133-137: LGTM!The
_throwIfClosed()helper centralizes the closed-batch check with a clear error message and appropriate error code.src/plugins/objects/livecounter.ts (7)
9-12: LGTM!Adding
_type: 'LiveCounterUpdate'to the interface enables discriminated union patterns for type-safe update handling across different LiveObject types.
22-24: LGTM!The static factory methods now correctly take
RealtimeObjectas the first parameter, aligning with the migration from theObjectsabstraction. The implementation properly extracts the channel and client fromRealtimeObject.Also applies to: 32-36
41-61: LGTM!The
createCounterIncMessagestatic method is well-structured:
- Validates the amount is a finite number
- Constructs the message with proper action and payload
- Uses client utilities correctly
This enables reuse from both instance methods and batch contexts.
64-96: LGTM!The
value(),increment(), anddecrement()methods properly use_realtimeObjectfor:
- API configuration validation
- Message creation
- Publishing
The
decrement()method appropriately validates the amount type before negation to avoid silent type coercion bugs.
207-226: LGTM!The
overrideWithObjectStatemethod now correctly:
- Propagates
objectMessageon the update object for tombstone and normal paths- Returns
LiveCounterUpdatewith consistent structure including the_typediscriminatorThe flow is clear: tombstone path sets update from
tombstone(), normal path computes diff and attachesobjectMessage.
242-263: LGTM!Both
_updateFromDataDiffand_mergeInitialDataFromCreateOperationnow consistently returnLiveCounterUpdateobjects with the_type: 'LiveCounterUpdate'discriminator, ensuring type safety in update handlers.
293-300: LGTM!The
_applyCounterIncmethod correctly constructs a completeLiveCounterUpdatewith:
- The amount delta in
update- The originating
objectMessagefor subscriber context- The
_typediscriminatorThis aligns with the consistent update structure across all counter operations.
src/plugins/objects/liveobject.ts (4)
50-68: LGTM! Parent reference tracking initialization is well-structured.The
_parentReferencesMap withSet<string>values efficiently supports the multi-parent, multi-key relationship model needed for path traversal. Constructor properly initializes all fields including the new parent references.
215-253: Well-implemented iterative DFS with proper cycle detection.The
getFullPaths()implementation correctly:
- Uses an explicit stack to avoid call-stack overflow for deep hierarchies
- Creates new
visitedsets per path branch to handle diamond-shaped graphs- Detects cycles and gracefully skips them
One minor observation: creating a new
Seton each iteration (line 231) could be optimized for very large graphs, but this is acceptable for typical use cases.
286-322: Path subscription notification logic is correct.The implementation properly:
- Filters out sync messages from user-facing events
- Creates bubbling events for the object's paths
- Creates non-bubbling events for each updated key in LiveMapUpdate
96-109: No action needed. The_subscriptions.off()call correctly cleans up all listeners. The Ably EventEmitter implementation explicitly supports calling.off()with no arguments to remove all listeners across all events. When invoked this way, it clears all four listener storage arrays (any,events,anyOnce, andeventsOnce), which is the intended behavior for the tombstone operation.src/plugins/objects/pathobject.ts (4)
139-165: Excellent manual path parsing to avoid negative lookbehind compatibility issues.The manual character-by-character parsing of escaped dots is well-implemented and correctly handles:
- Escaped dots (
\.) being preserved as literal dots- Unescaped backslashes at the end of string
- Split on unescaped dots only
The comment explaining the Safari compatibility concern is helpful.
239-243: Unused loop variable_is acceptable but could use conventional naming.The underscore prefix is a common convention for unused variables. This is fine.
308-322: Missing try-catch for error code 92005 inset()method.Unlike
value(),entries(), and other read methods that catch error code 92005 and returnundefined, theset()method directly throws if_resolvePath()fails. This is intentional for mutation operations (user should know if the path doesn't exist), but it creates an asymmetry.Consider documenting this behavior or ensuring it's intentional that mutations throw while reads return undefined for unresolved paths.
414-442: Path resolution correctly throws for intermediate non-collection types and missing keys.The
_resolvePath()implementation correctly validates each segment and throws with error code 92005, which callers can catch and handle appropriately.src/plugins/objects/livemap.ts (6)
42-52: Well-designed branded type pattern for LiveMap.The use of
declare readonly [API.__livetype]: 'LiveMap'provides compile-time type discrimination without runtime overhead. This is a clean approach for TypeScript type branding.
501-531: Compact implementation correctly handles cyclic references using memoization.The
compact()method properly:
- Memoizes results before recursing to handle cycles
- Returns in-memory object references for already-visited objects
- Uses
entries()to skip tombstoned entries
543-579: CompactJson correctly differentiates from compact() for JSON serialization.Good distinction between:
compact(): Returns in-memory pointers for cyclescompactJson(): Returns{ objectId: string }for cyclesThis ensures
compactJson()output is JSON-serializable.
472-490: Verify entry iteration during clearData doesn't cause issues.The
clearData()method iterates overthis._dataRef.data.entries()to clean up parent references, then callssuper.clearData()which resets_dataRefto a zero-value. This is correct since the iteration happens before the reset.
997-1036: Duplicate parent reference updates in _updateParentReferencesFromUpdate.The
_updateParentReferencesFromUpdatemethod handles parent reference updates foroverrideWithObjectState(). However, whenchangeType === 'updated', it removes the old reference and adds the new one. This is the same logic as in_applyMapSet().This duplication is acceptable since they serve different code paths, but be aware that any bug fix needs to be applied in both places.
976-977: Type assertion is correct and properly justified.The cast from internal
LiveObjecttoAPI.LiveObjectusing the double assertion is necessary and appropriate. The abstractLiveObjectbase class cannot be directly cast to the public API type union (LiveMap | LiveCounter), soas unknown asis the correct pattern. However, at runtime,refObjectis guaranteed to be an instance of eitherLiveMaporLiveCounter—both of which implement their respective public API interfaces. This assertion is safe and properly handles the type system's limitation in tracking that abstract base class instances are always concrete implementations.
… with the naming convention for other realtime features in the SDK This updates ably-js implementation to match the spec [1] [1] ably/specification#332
…RealtimeObjects` to `RealtimeObject` Resolves https://ably.atlassian.net/browse/PUB-2059
Change the name for the `Objects` class to `RealtimeObjects`
Implements: - LiveObject parent tracking - Path-based LiveObject event emission - PathObject subscriptions path matching with deep subscriptions - Path event emission for LiveMap key updates - Full ObjectMessage (user-facing type) argument for subscription callbacks Resolves PUB-2061
Stricter typing removes `ArrayBufferView` from the union type to align with the publicly exposed API, which only uses a union of `Buffer` and `ArrayBuffer`. This change enables the internal `ObjectMessage.operation` object to be exposed as the public `ObjectOperation` type in the following commit without buffer type mismatches for underlying values.
…iveObject subscriptions LiveObject subscriptions (via PathObject and Instance APIs) now expose the full operation object that caused the change in the `event.message.operation` field.
Resolves PUB-2063
Exposes LiveMap.create() and LiveCounter.create() static methods at ably/objects export. Updates all objects tests to use these new methods to create LiveMap and LiveCounter instead of realtimeObject.createMap()/.createCounter() calls Resolves PUB-2060
This was replaced by PathObject/Instance subscription API from previous commits
Adds new path-based types to ably.d.ts. The previous LiveObject types are temporarily marked as *Deprecated in ably.d.ts until they are fully converted to the new type system in the following PRs. The `RealtimeObject` entrypoint temporarily adds a `getPathObject` method to get a PathObject instance for a root and preserves the existing `.get()` method which returns a LiveMap instance directly so that corresponding tests do not fail yet. Resolves PUB-2057
This makes LiveObjects Instance methods to be in line with PathObject methods handling of wrong underlying type, as discussed in [1]. Type/behavior overview: - id(): may return `undefined` (when Instance is from .get() call that returned a primitive instance) - compact(): is defined for all Instance types so can be non-nullable. See method implementation in [2] - get(): may return `undefined` when not a map - entries()/keys()/values()/size(): may return `undefined` when not a map - value(): may return `undefined` when not a counter or a primitive [1] #2091 (comment) [2] #2098
They are replaced by object creation via value types from previous commit
[PUB-2059] Change LiveObjects entry point to `channel.object.get()`
Update Objects docstrings to not use "root" terminology
[PUB-2057] Implement base PathObject class with access and mutation methods
[PUB-2063] Implement Instance API for LiveObjects PathObject
…eObjects Resolves PUB-2064
[PUB-2060] Implement LiveMap and LiveCounter creation via value types
…s for binary values
[PUB-2061] Add path based subscriptions support for LiveObjects
Resolves PUB-2062
…ject creation This also removes the now-obsolete RealtimeObject.batch() API. Resolves PUB-2065
Just like .unsubscribeAll, .offAll doesn't play nice with composable SDKs - the end user by indiscriminately calling .offAll to remove all listeners may remove a listener registered by another SDK wrapper via the public subscription API. It is safer and more reliable to require developers to keep track of their listeners and remove them individually, which realistically they should be doing anyway.
…ead of LiveMap/LiveCounter
PathObject API couple of minor fixes
github-actions
bot
temporarily deployed
to
staging/pull/2134/bundle-report
github-actions
bot
temporarily deployed
to
staging/pull/2134/features
github-actions
bot
temporarily deployed
to
staging/pull/2134/typedoc
VeskeR
changed the title
Integration/objects-breaking-api into mainintegration/objects-breaking-api into main
ttypic
left a comment
ttypic
left a comment
There was a problem hiding this comment.
Double-checked public api changes, and build process, every change has already been checked in the individual PRs. LGTM
Part of AIT-31 - docs for LiveObjects path-based API
- Add section documenting plugin import changes: path renamed from 'ably/objects' to 'ably/liveobjects', default export changed to named export, plugin name changed from Objects to LiveObjects - Add TypeScript section for types moving from 'ably' to 'ably/liveobjects' - Add section for RealtimeObject.offAll() removal with individual listener management alternatives - Update existing code examples to use 'ably/liveobjects' import path - Mention UMD bundle global is now AblyLiveObjectsPlugin
…ventResponse Remove unnecessary OnObjectsEventResponse
github-actions
bot
temporarily deployed
to
staging/pull/2134/features
github-actions
bot
temporarily deployed
to
staging/pull/2134/bundle-report
github-actions
bot
temporarily deployed
to
staging/pull/2134/typedoc
[AIT-31] LiveObjects Path-based API migration guide
github-actions
bot
temporarily deployed
to
staging/pull/2134/bundle-report
github-actions
bot
temporarily deployed
to
staging/pull/2134/features
github-actions
bot
temporarily deployed
to
staging/pull/2134/typedoc
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/plugins/liveobjects/pathobject.ts (1)
177-200: AlignPathObject.value()return type with possiblenullvalue
value()currently returnsresolvedwhen it isnull(Line 196), but the signature isvalue<U extends number | Primitive = number | Primitive>(): U | undefined, andPrimitive/Valuedon’t includenull. This recreates the earlier mismatch: TypeScript callers don’t seenullin the type, but can still receive it at runtime.Consider updating either:
- the return type to include
null:- value<U extends number | Primitive = number | Primitive>(): U | undefined { + value<U extends number | Primitive = number | Primitive>(): U | null | undefined {or
- the
Primitive/Valuealiases to explicitly admitnullifnullis intended as a valid stored value.src/plugins/liveobjects/livemap.ts (1)
108-126: Address null safety concern from previous review.The non-null assertions on
counterCreateMsg.operation?.objectId!(line 112) andmapCreateMsg.operation?.objectId!(line 121) assume these create messages always have a defined operation and objectId. If eithercreateCounterCreateMessageorcreateMapCreateMessagecan return messages with undefined operation or objectId fields, this will cause runtime issues.🔎 Recommended validation approach
if (LiveCounterValueType.instanceof(value)) { const counterCreateMsg = await LiveCounterValueType.createCounterCreateMessage(realtimeObject, value); createValueTypesMessages = [counterCreateMsg]; + if (!counterCreateMsg.operation?.objectId) { + throw new client.ErrorInfo('Counter create message missing objectId', 92000, 500); + } - const typedObjectData: ObjectIdObjectData = { objectId: counterCreateMsg.operation?.objectId! }; + const typedObjectData: ObjectIdObjectData = { objectId: counterCreateMsg.operation.objectId }; objectData = typedObjectData; } else if (LiveMapValueType.instanceof(value)) { const { mapCreateMsg, nestedObjectsCreateMsgs } = await LiveMapValueType.createMapCreateMessage( realtimeObject, value, ); createValueTypesMessages = [...nestedObjectsCreateMsgs, mapCreateMsg]; + if (!mapCreateMsg.operation?.objectId) { + throw new client.ErrorInfo('Map create message missing objectId', 92000, 500); + } - const typedObjectData: ObjectIdObjectData = { objectId: mapCreateMsg.operation?.objectId! }; + const typedObjectData: ObjectIdObjectData = { objectId: mapCreateMsg.operation.objectId }; objectData = typedObjectData; }
🧹 Nitpick comments (3)
src/plugins/liveobjects/realtimeobject.ts (1)
308-351: Post-sync parent-reference rebuild is correct; just be aware of its cost on large graphsRunning
_rebuildAllParentReferences()after_applySync()ensures allLiveMap→LiveObjectrelationships are re-established beforenotifyUpdatedfires, which is important for features that rely on parent links (e.g. compaction and path-based subscriptions). The implementation is straightforward but walks the entire pool twice and iterates everyLiveMap.entries(); on very large object graphs this is O(N)–O(N·avgDegree) per sync.Given it only runs at the end of a sync sequence, that trade-off is probably fine, but if you later see sync-time hotspots it might be worth narrowing the rebuild to objects actually touched by the sync.
src/plugins/liveobjects/batchcontext.ts (1)
90-130: Batch context write guards and queuing strategy look solid (minor coupling toDefaultInstance)The synchronous
set/remove/increment/decrementmethods correctly:
- enforce write preconditions,
- guard against use-after-close,
- narrow to the appropriate underlying type (
LiveMap/LiveCounter),- and delegate to
RootBatchContext.queueMessages(...)so batch semantics stay centralized.The only minor coupling is the cast to
DefaultInstance<Value>for the type checks; ifInstanceever gains a public discriminator API, you could switch to that to decouple from the concrete implementation, but it’s not a blocker.liveobjects.d.ts (1)
1683-1683: Consider cleaner pattern for branded type utilities.Static analysis warns about merging the
LiveCounterclass (line 1683) with theLiveCounterinterface (lines 161-164). While this works because__livetypeis type-only, it's technically an unsafe declaration merge pattern.The same pattern is used for
LiveMap(line 1665).Consider alternatives:
- Use different names:
LiveCounterValueclass andLiveCounterinterface- Make the class abstract and use separate concrete implementations
- Accept the pattern as-is since
__livetypeis type-onlySince the brand is type-only (no JS emission), the current implementation is functionally correct, but may trigger linting warnings in user projects.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
ably.d.ts(4 hunks)docs/migration-guides/v2/liveobjects.md(1 hunks)liveobjects.d.ts(1 hunks)src/plugins/liveobjects/batchcontext.ts(1 hunks)src/plugins/liveobjects/instance.ts(1 hunks)src/plugins/liveobjects/livecounter.ts(7 hunks)src/plugins/liveobjects/livemap.ts(22 hunks)src/plugins/liveobjects/liveobject.ts(7 hunks)src/plugins/liveobjects/pathobject.ts(1 hunks)src/plugins/liveobjects/realtimeobject.ts(13 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/plugins/liveobjects/pathobject.ts (3)
src/plugins/liveobjects/livemap.ts (1)
LiveMap(56-1037)src/plugins/liveobjects/livecounter.ts (2)
value(68-70)LiveCounter(17-302)src/plugins/liveobjects/objectid.ts (1)
LiveObjectType(4-4)
src/plugins/liveobjects/instance.ts (8)
liveobjects.d.ts (11)
ObjectMessage(1529-1576)Value(176-176)AnyInstance(1367-1420)LiveObject(169-169)LiveMap(153-156)LiveMap(1665-1678)Instance(1427-1433)Primitive(117-125)LiveCounter(161-164)LiveCounter(1683-1692)BatchContext(823-829)src/plugins/liveobjects/objectmessage.ts (1)
ObjectMessage(357-461)src/plugins/liveobjects/realtimeobject.ts (1)
RealtimeObject(35-468)src/plugins/liveobjects/livemap.ts (1)
LiveMap(56-1037)src/plugins/liveobjects/batchcontext.ts (1)
value(43-47)src/plugins/liveobjects/livecounter.ts (2)
value(68-70)LiveCounter(17-302)src/plugins/liveobjects/pathobject.ts (2)
value(177-218)instance(220-233)src/plugins/liveobjects/objectid.ts (1)
LiveObjectType(4-4)
src/plugins/liveobjects/livemap.ts (5)
src/plugins/liveobjects/liveobject.ts (1)
LiveObjectUpdate(18-26)src/plugins/liveobjects/realtimeobject.ts (1)
RealtimeObject(35-468)src/plugins/liveobjects/objectmessage.ts (1)
ObjectMessage(357-461)src/plugins/liveobjects/livecountervaluetype.ts (1)
LiveCounterValueType(21-98)src/plugins/liveobjects/livemapvaluetype.ts (1)
LiveMapValueType(34-161)
src/plugins/liveobjects/batchcontext.ts (4)
src/plugins/liveobjects/rootbatchcontext.ts (1)
RootBatchContext(6-73)src/plugins/liveobjects/instance.ts (2)
value(103-133)DefaultInstance(27-264)src/plugins/liveobjects/livecounter.ts (2)
value(68-70)LiveCounter(17-302)src/plugins/liveobjects/livemap.ts (1)
LiveMap(56-1037)
src/plugins/liveobjects/liveobject.ts (4)
src/plugins/liveobjects/objectmessage.ts (1)
ObjectMessage(357-461)src/plugins/liveobjects/instance.ts (1)
InstanceEvent(22-25)src/plugins/liveobjects/constants.ts (1)
ROOT_OBJECT_ID(1-1)src/plugins/liveobjects/pathobjectsubscriptionregister.ts (1)
PathEvent(23-30)
liveobjects.d.ts (7)
src/plugins/liveobjects/realtimeobject.ts (1)
RealtimeObject(35-468)src/plugins/liveobjects/livemap.ts (1)
LiveMap(56-1037)test/realtime/liveobjects.test.js (2)
LiveMap(18-18)LiveCounter(19-19)src/plugins/liveobjects/livecounter.ts (2)
LiveCounter(17-302)value(68-70)src/plugins/liveobjects/pathobject.ts (2)
path(48-51)value(177-218)scripts/cdn_deploy.js (1)
path(5-5)src/plugins/liveobjects/objectmessage.ts (7)
ObjectMessage(357-461)ObjectOperation(130-165)ObjectsMapOp(70-75)ObjectsCounterOp(81-84)ObjectsMap(110-115)ObjectsCounter(121-124)ObjectData(39-44)
🪛 Biome (2.1.2)
liveobjects.d.ts
[error] 1683-1683: This class is unsafely merged with an interface.
The interface is declared here.
The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.
(lint/suspicious/noUnsafeDeclarationMerging)
🪛 LanguageTool
docs/migration-guides/v2/liveobjects.md
[style] ~554-~554: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... been moved from the 'ably' export to 'ably/liveobjects'. This consolidates all Li...
(ADVERB_REPETITION_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: test-browser (webkit)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-browser (chromium)
- GitHub Check: test-npm-package
- GitHub Check: test-node (20.x)
- GitHub Check: test-node (18.x)
- GitHub Check: test-node (16.x)
🔇 Additional comments (8)
ably.d.ts (1)
1659-1685: Unified callback & subscription primitives look consistent with LiveObjects usageThe new
EventCallback<T>,Subscription, andStatusSubscriptionabstractions are simple and align well with the LiveObjects path/instance subscription APIs (e.g.,PathObject.subscribeandRealtimeObject.on). This should make the public surface easier to reason about while remaining structurally compatible with the existing event emitter model.src/plugins/liveobjects/livecounter.ts (1)
1-301: LGTM! Clean migration to RealtimeObject-centric design.The LiveCounter implementation has been successfully migrated to use RealtimeObject throughout. Key improvements include:
- Proper type branding with
__livetypesymbol- Consistent
_type: 'LiveCounterUpdate'discriminator in all update paths- ObjectMessage propagation in updates for subscription events
- Clean static method signatures using RealtimeObject context
The refactor maintains correctness while aligning with the new path-based API design.
src/plugins/liveobjects/livemap.ts (2)
501-579: Well-implemented cycle handling in compact methods.Both
compact()andcompactJson()correctly handle cyclic references:
compact()uses memoization to return shared object referencescompactJson()returns{ objectId: string }for cycles, making it JSON-serializable- Buffer encoding to base64 in
compactJson()is properly handledThe implementation follows the documented behavior and prevents infinite loops.
764-796: Excellent parent reference management implementation.The parent reference tracking across
_applyMapSet,_applyMapRemove, and_updateParentReferencesFromUpdateis well-designed:
- During operations: References are updated inline when processing MAP_SET/MAP_REMOVE
- During sync: References are batch-updated based on computed diff after
overrideWithObjectState- Cleanup: Properly removes references when entries are tombstoned or overridden
This ensures the parent reference graph stays consistent for path computation.
Also applies to: 843-850, 998-1036
src/plugins/liveobjects/liveobject.ts (3)
162-205: Clean parent reference management API.The parent reference tracking methods are well-designed:
- Efficient data structure:
Map<LiveObject, Set<string>>- Proper cleanup: Empty Sets are removed to avoid memory leaks
- Complete API: add, remove, removeAll, clear cover all use cases
213-251: Robust path computation with proper cycle handling.The
getFullPaths()implementation correctly uses iterative DFS with per-path cycle detection:
- Each path maintains its own
visitedSet to detect cycles within that path- Multiple paths through the same node are correctly found
- Stack-based iteration avoids recursion depth limits
This elegantly handles the complex graph traversal needed for the path-based API.
276-320: Well-designed dual notification system.The split between instance and path subscriptions is elegant:
- Instance subscriptions: Track specific object instances regardless of location
- Path subscriptions: Track changes at specific paths, computing all paths to root
- Per-key events: LiveMapUpdate creates non-bubbling events for each modified key
Filtering out object sync messages (line 279, 297) is correct—only operations represent single state changes.
src/plugins/liveobjects/instance.ts (1)
239-253: Batch error handling is intentional and working as designed.The batch method correctly prevents partial operations from being sent if the user function throws an error. If
fn(ctx)fails,ctx.flush()is skipped, so mutations are discarded. Iffn(ctx)succeeds,ctx.flush()executes and sends all queued operations. Thectx.close()call in the finally block is technically redundant (sinceflush()already callsclose()internally) but harmless. This behavior is confirmed by comprehensive tests that verify operations are canceled on error and the batch context becomes inaccessible afterward.
Make PathObjectSubscriptionRegister.subscribe options null check more robust
This is an aggregate PR combining all PRs that targeted
integration/objects-breaking-apibranch, implementing the path-based API changes for LiveObjects. All included PRs were reviewed individually, so a full review of all changed files isn't necessary (or realistic at this point).Instead, I'd ask reviewers to have a final look at the public type changes and build changes to spot any obvious errors.
This PR includes:
Objectsclass toRealtimeObjects#2043channel.object.get()#2087.compact()method for PathObject and Instance types for LiveObjects #2098string#2120Instance.id()toInstance.idgetter property to better align with other public interfaces #2128PathObject/Instance.compact()intocompact()andcompactJson()#2129ably.d.tstoobjects.d.ts#2130moduleResolution#2136Conflicts with the main branch will be resolved by rebasing
integration/objects-breaking-apion top of latestmainusing the--rebase-mergesoption (i.e.git rebase main --rebase-merges). Rebase is chosen over a merge commit to avoid a massive merge commit that would inevitably hinder traversing line history, interfere with git blame, and clutter the git history. As a result of the rebase, final commit hashes won't match those from the PRs mentioned above. However, thanks to--rebase-merges, the merge commits for those PRs will still be preserved in the final history, which should make finding the original context for relevant changes easier in the future.Other references:
Summary by CodeRabbit
New Features
Breaking Changes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.