Change the name for the Objects class to RealtimeObjects
#2043
Conversation
WalkthroughRenames the public Objects symbol to RealtimeObjects across types, imports, exports, constructor parameters, docs, private API tracking, and tests; updates module-report allowed file. No runtime logic, control flow, or behavior changed. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RealtimeChannel
participant RealtimeObjects
Client->>RealtimeChannel: create channel
RealtimeChannel->>RealtimeObjects: new RealtimeObjects(this)
RealtimeObjects-->>RealtimeChannel: constructed instance
RealtimeChannel-->>Client: expose channel.objects (RealtimeObjects)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.38.6)test/realtime/objects.test.jsTip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
temporarily deployed
to
staging/pull/2043/bundle-report
Per [1] and [2]. [1] ably/specification#332 [2] ably/ably-js#2043
Per [1] and [2]. Resolves #15. [1] ably/specification#332 [2] ably/ably-js#2043
Per [1] and [2]. Resolves #15. [1] ably/specification#332 [2] ably/ably-js#2043
VeskeR
force-pushed
the
rename-realtime-objects
branch
from
f787719 to
587b545
Compare
… 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
VeskeR
force-pushed
the
rename-realtime-objects
branch
from
587b545 to
9b9ca8a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/realtime/objects.test.js (1)
1019-1021: Fix invalid chained comparisons in time-bound assertions (JS evaluates them unexpectedly).JavaScript doesn’t support Python-style chained comparisons. Expressions like a <= b <= c are evaluated left-to-right and coerce to booleans (leading to false positives/negatives). Assert bounds with two comparisons or use chai’s range helpers.
Apply this diff to each occurrence:
@@ - expect( - tsBeforeMsg <= obj.tombstonedAt() <= tsAfterMsg, - `Check object's "tombstonedAt" value is set using local clock if no "serialTimestamp" provided`, - ).to.be.true; + const ts = obj.tombstonedAt(); + expect(ts).to.be.at.least(tsBeforeMsg); + expect(ts).to.be.at.most(tsAfterMsg); + // Check object's "tombstonedAt" value is set using local clock if no "serialTimestamp" provided@@ - expect( - tsBeforeMsg <= mapEntry.tombstonedAt <= tsAfterMsg, - `Check map entry's "tombstonedAt" value is set using local clock if no "serialTimestamp" provided`, - ).to.be.true; + const ts = mapEntry.tombstonedAt; + expect(ts).to.be.at.least(tsBeforeMsg); + expect(ts).to.be.at.most(tsAfterMsg); + // Check map entry's "tombstonedAt" value is set using local clock if no "serialTimestamp" provided@@ - expect( - tsBeforeMsg <= obj.tombstonedAt() <= tsAfterMsg, - `Check object's "tombstonedAt" value is set using local clock if no "serialTimestamp" provided`, - ).to.be.true; + const ts = obj.tombstonedAt(); + expect(ts).to.be.at.least(tsBeforeMsg); + expect(ts).to.be.at.most(tsAfterMsg); + // Check object's "tombstonedAt" value is set using local clock if no "serialTimestamp" provided@@ - expect( - tsBeforeMsg <= mapEntry.tombstonedAt <= tsAfterMsg, - `Check map entry's "tombstonedAt" value is set using local clock if no "serialTimestamp" provided`, - ).to.be.true; + const ts = mapEntry.tombstonedAt; + expect(ts).to.be.at.least(tsBeforeMsg); + expect(ts).to.be.at.most(tsAfterMsg); + // Check map entry's "tombstonedAt" value is set using local clock if no "serialTimestamp" providedAlso applies to: 1098-1100, 1672-1674, 2201-2203
🧹 Nitpick comments (5)
src/plugins/objects/batchcontextlivecounter.ts (1)
4-4: Use a type-only import for RealtimeObjects
RealtimeObjectsis only used as a type in this file; prefer a type-only import to avoid creating a runtime dependency and to align with existing patterns in this codebase.-import { RealtimeObjects } from './realtimeobjects'; +import type { RealtimeObjects } from './realtimeobjects';ably.d.ts (1)
2289-2289: Public API rename to RealtimeObjectsThe interface is now
RealtimeObjects, matching the runtime class andRealtimeChannel.objectsproperty type.If you want to ease migration for SDK consumers, consider a temporary deprecated alias. Only do this if it doesn’t conflict with the “breaking changes” plan for the integration branch.
/** @deprecated Use RealtimeObjects */ export type Objects = RealtimeObjects;test/realtime/objects.test.js (3)
4577-4581: Use the matching default constant when priming gcGracePeriod.You’re priming objects.gcGracePeriod with the gcInterval default. This works but is misleading and risks future coupling. Use RealtimeObjects._DEFAULTS.gcGracePeriod for clarity.
- objects.gcGracePeriod = ObjectsPlugin.RealtimeObjects._DEFAULTS.gcInterval + 1; + objects.gcGracePeriod = ObjectsPlugin.RealtimeObjects._DEFAULTS.gcGracePeriod + 1;Also applies to: 4600-4604
4711-4715: Global default mutation for GC interval can cause inter-test coupling.Overriding RealtimeObjects._DEFAULTS.gcInterval affects all instances. You restore it in finally (good), but if the suite runs tests in parallel this can flake other specs.
Consider stubbing the timer per-instance instead of changing a global:
- Inject a test-only GC interval via constructor/config if available, or
- Temporarily stub objects._objectsPool._onGCInterval scheduling to use a shorter delay for this client only.
Would you like a PR-ready helper to stub the pool timer locally for these scenarios?
Also applies to: 4766-4768
23-28: Confirm intended casing for channel object modes.Here you pass modes: ['OBJECT_SUBSCRIBE', 'OBJECT_PUBLISH'], while other tests and error messages use lowercase ('object_subscribe', 'object_publish'). If both are accepted, fine; otherwise align to one form to avoid confusion.
Possible tweak:
- modes: ['OBJECT_SUBSCRIBE', 'OBJECT_PUBLISH'], + modes: ['object_subscribe', 'object_publish'],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
ably.d.ts(6 hunks)scripts/moduleReport.ts(1 hunks)src/common/lib/client/realtimechannel.ts(3 hunks)src/plugins/objects/batchcontext.ts(2 hunks)src/plugins/objects/batchcontextlivecounter.ts(1 hunks)src/plugins/objects/batchcontextlivemap.ts(1 hunks)src/plugins/objects/index.ts(1 hunks)src/plugins/objects/livecounter.ts(6 hunks)src/plugins/objects/livemap.ts(9 hunks)src/plugins/objects/liveobject.ts(2 hunks)src/plugins/objects/objectspool.ts(2 hunks)src/plugins/objects/realtimeobjects.ts(4 hunks)src/plugins/objects/syncobjectsdatapool.ts(2 hunks)test/common/modules/private_api_recorder.js(4 hunks)test/realtime/objects.test.js(30 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- src/plugins/objects/syncobjectsdatapool.ts
- src/plugins/objects/livecounter.ts
- src/plugins/objects/objectspool.ts
- src/plugins/objects/realtimeobjects.ts
- src/plugins/objects/batchcontextlivemap.ts
- src/plugins/objects/liveobject.ts
- test/common/modules/private_api_recorder.js
- src/plugins/objects/index.ts
- src/plugins/objects/livemap.ts
- src/common/lib/client/realtimechannel.ts
- src/plugins/objects/batchcontext.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-08T05:28:30.955Z
Learnt from: VeskeR
PR: ably/ably-js#1917
File: src/plugins/liveobjects/liveobject.ts:2-2
Timestamp: 2024-11-08T05:28:30.955Z
Learning: In `src/plugins/liveobjects/liveobject.ts`, `EventEmitter` is accessed via the client object, so importing it with the `type` modifier is acceptable.
Applied to files:
scripts/moduleReport.tssrc/plugins/objects/batchcontextlivecounter.tsably.d.ts
📚 Learning: 2024-10-22T13:26:59.680Z
Learnt from: VeskeR
PR: ably/ably-js#1897
File: src/plugins/liveobjects/livecounter.ts:93-93
Timestamp: 2024-10-22T13:26:59.680Z
Learning: In the `LiveCounter` class's `_applyCounterCreate` method, it's intentional to increment the counter's value using `+=` instead of initializing it with `=` because the counter may have a pre-existing non-zero value.
Applied to files:
test/realtime/objects.test.js
🧬 Code Graph Analysis (3)
src/plugins/objects/batchcontextlivecounter.ts (2)
src/plugins/objects/batchcontext.ts (1)
BatchContext(11-107)src/plugins/objects/realtimeobjects.ts (1)
RealtimeObjects(39-529)
test/realtime/objects.test.js (1)
src/common/lib/client/realtimechannel.ts (1)
objects(153-158)
ably.d.ts (2)
src/plugins/objects/index.ts (1)
RealtimeObjects(4-4)src/plugins/objects/realtimeobjects.ts (1)
RealtimeObjects(39-529)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test-browser (webkit)
- GitHub Check: test-browser (firefox)
- GitHub Check: test-browser (chromium)
- GitHub Check: test-node (16.x)
- GitHub Check: test-node (20.x)
🔇 Additional comments (8)
src/plugins/objects/batchcontextlivecounter.ts (1)
11-11: Rename wiring looks correctConstructor param type updated to
RealtimeObjects; usage (getClient, guards, pool access) remains unchanged. No behavioral changes; compile-time safety improved.scripts/moduleReport.ts (1)
340-341: No stale references toobjects.tsremainAll verification checks passed:
- Confirmed there is no file named
objects.tsinsrc/plugins/objects(onlyrealtimeobjects.ts, which is expected).- No imports or exports of a type/value named
Objects(excluding event-related symbols).- No occurrences of the old path
src/plugins/objects/objects.ts.ably.d.ts (5)
1656-1659: Doc rename to RealtimeObjects — OKThe callback doc now references
RealtimeObjects. Name and linkage read correctly.
1666-1672: Batch callback doc updated to RealtimeObjects — OKReference to
RealtimeObjects.batchis consistent with the class rename; constraints (“Must not be async”) preserved.
2267-2270: ObjectsEvent doc updated to mention RealtimeObjects — OKEvent typing remains under the
Objects*naming while the class isRealtimeObjects. This is acceptable given “Objects” is the feature name.
2427-2433: BatchContext doc cross-link updated — OK
Mirrors the {@link RealtimeObjects.getRoot}matches the new interface name and keeps generics intact.
2946-2949: No lingeringObjectsreferences in ably.d.ts
All sanity checks passed—there are no remaining declarations or links still referring to the oldObjectsname:• Ran ripgrep across
ably.d.tsfor
–export declare interface Objects
–objects: Objects
–{@link Objects}and found zero matches. The public API surface is fully aligned to
RealtimeObjects. Changes approved.test/realtime/objects.test.js (1)
275-280: Rename to RealtimeObjects looks consistent across tests and private API taps.Representative assertions and private API recorder paths have been updated to RealtimeObjects, and the tests still import the plugin as Objects (as intended). This aligns with the public API rename without breaking plugin wiring.
Also applies to: 3078-3096, 3100-3124, 3204-3237, 3241-3266, 3295-3333, 3336-3362, 3366-3377, 3381-3413, 3417-3453, 3457-3490, 4530-4536, 4560-4562, 4577-4581, 4600-4604, 4625-4653, 4713-4749
VeskeR
merged commit f40b6bb
into
integration/objects-breaking-api
Change the name for the `Objects` class to `RealtimeObjects`
Change the name for the `Objects` class to `RealtimeObjects`
Change the name for the `Objects` class to `RealtimeObjects`
Change the name for the `Objects` class to `RealtimeObjects`
Change the name for the `Objects` class to `RealtimeObjects`
This aligns
Objectswith the naming convention for other realtime features in the SDK and matches the spec ably/specification#332.This is a public API breaking change, so PR will be merged into the integration branch for Objects API improvements that may introduce other API changes
Summary by CodeRabbit
Refactor
Documentation
Tests
Chores