-
Notifications
You must be signed in to change notification settings - Fork 56
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
Update type declarations in ably.d.ts
for LiveObjects
#1967
Conversation
Warning Rate limit exceeded@VeskeR has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis change introduces new types, interfaces, and namespaces to enhance the LiveObjects functionality in the Ably client library. New event callbacks and lifecycle event types are provided alongside methods for creating maps and counters, handling batch operations, and managing event listeners. Updates to documentation clarify the behavior of these methods, particularly regarding ACK messages and local object pooling. Additionally, minor comment adjustments improve clarity in the LiveCounter class. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant LiveObjects
participant Server
Client->>LiveObjects: createMap(entries) / createCounter(count)
LiveObjects->>Server: Send creation request with provided data
alt ACK Received
Server-->>LiveObjects: ACK (confirmation)
LiveObjects-->>Client: Return object from local pool
else No ACK yet
LiveObjects-->>Client: Return newly created local object using provided data
end
sequenceDiagram
participant Client
participant LiveObjects
participant Server
Client->>LiveObjects: batch(callback)
LiveObjects->>Client: Execute BatchCallback with BatchContext
Client->>LiveObjects: Perform batched set/increment/decrement operations
LiveObjects->>Server: Send batched operations message
Server-->>LiveObjects: ACK for batched update
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (1)
src/plugins/liveobjects/liveobjects.ts (1)
287-287
: TODO comments need to be addressed.There are TODO comments indicating missing implementation for handling 'detached', 'failed', and 'suspended' channel states. These states should be properly handled to ensure robust error handling and state management.
Would you like me to help implement the missing channel state handlers or create an issue to track this task?
Also applies to: 291-291
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ably.d.ts
(8 hunks)src/plugins/liveobjects/livecounter.ts
(1 hunks)src/plugins/liveobjects/liveobjects.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/plugins/liveobjects/livecounter.ts
🧰 Additional context used
🪛 GitHub Actions: Test NPM package
ably.d.ts
[error] 2223-2223: Type 'BatchContextLiveMap' is not generic.
🪛 GitHub Actions: Lint
ably.d.ts
[error] 2223-2223: Type 'BatchContextLiveMap' is not generic.
⏰ 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-node (16.x)
- GitHub Check: test-browser (chromium)
🔇 Additional comments (15)
ably.d.ts (13)
1569-1587
: Nice addition of callback types.Everything looks good here. The doc is clear, and the approach is consistent with existing type definitions.
2054-2072
: New LiveObjects events look good.No issues spotted with the usage or naming for
'syncing'
and'synced'
.
2073-2087
: New LiveObject lifecycle events are consistent.The
'deleted'
state is well-defined to capture when an object is removed from the pool.
2119-2134
: MethodcreateMap
is clearly documented.No issues found. The type parameterization is correct, ensuring typed usage of the newly created
LiveMap
.
2135-2149
: MethodcreateCounter
is straightforward.No issues found. The doc block is clear on the initial count behavior.
2150-2171
:batch
method is well-defined.Synchronous callback usage is clearly highlighted to avoid confusion about concurrency. Good job clarifying that it must not be async.
2205-2212
:OnLiveObjectsEventResponse
interface is fine.No issues found. The usage of an
off()
method is consistent with other patterns in this file.
2220-2227
: Documentation block forBatchContextLiveMap
.Minor note: The main declarations follow below. We'll watch for any type parameter consistency issues in the code body as well.
🧰 Tools
🪛 GitHub Actions: Test NPM package
[error] 2223-2223: Type 'BatchContextLiveMap' is not generic.
🪛 GitHub Actions: Lint
[error] 2223-2223: Type 'BatchContextLiveMap' is not generic.
2270-2293
:BatchContextLiveCounter
usage is consistent.No issues found. The definition is straightforward, and no generics conflict arises here.
2317-2341
: Setting/removing keys fromLiveMap
is well-documented.No further concerns. The approach to applying operations when echoed back is clear.
2371-2390
: Increment/Decrement methods are logically consistent.No issues. The documentation is clear on how the final value is applied upon message echo.
2431-2452
: Lifecycle event handling forLiveObject
is consistent.No issues or suggestions to add.
2475-2482
:OnLiveObjectLifecycleEventResponse
is properly structured.No issues found. The approach is consistent with the rest of the file’s event handling.
src/plugins/liveobjects/liveobjects.ts (2)
104-107
: LGTM! Documentation improvements enhance clarity.The updated documentation now provides a clearer explanation of the method's behavior regarding ACK messages and object creation, which better aligns with the actual implementation.
136-139
: LGTM! Documentation improvements enhance clarity.The updated documentation now provides a clearer explanation of the method's behavior regarding ACK messages and object creation, which better aligns with the actual implementation.
2635a66
to
ba1c24a
Compare
…d lifecycle events APIs
ba1c24a
to
f2dcf8a
Compare
This adds public write, batch and lifecycle events APIs type declarations. All liveobjects public API should be defined in
ably.d.ts
at this point.Summary by CodeRabbit
New Features
Documentation