Skip to content

Conversation

@VeskeR
Copy link
Contributor

@VeskeR VeskeR commented Nov 19, 2025

Resolves PUB-3439

Summary by CodeRabbit

  • Breaking Changes
    • Import paths simplified: migrate all package imports from the ably/objects submodule to direct imports from the main ably package
    • Generic type parameters now require explicit specification where automatic type defaults were previously available
    • Global type augmentation interface removed; custom object types must now be declared explicitly

… to be provided explicitly

Resolves PUB-3439
@coderabbitai
Copy link

coderabbitai bot commented Nov 19, 2025

Walkthrough

The changes remove the global AblyObjectsTypes interface and AblyDefaultObject type, making the generic type parameter of RealtimeObject.get() non-optional across TypeScript declarations, implementation, and tests. This requires consumers to explicitly specify type parameters instead of relying on global type augmentation defaults.

Changes

Cohort / File(s) Summary
Type Declaration Updates
ably.d.ts, src/plugins/objects/realtimeobject.ts
Removed default generic parameter from get<T extends Record<string, Value>>() method signature. Eliminated global AblyObjectsTypes interface and AblyDefaultObject type alias. Updated JSDoc example imports.
Test Updates
test/package/browser/template/src/index-objects.ts
Removed global augmentation for AblyObjectsTypes and ExplicitObjectType typedef. Updated channel.object.get() call sites to use explicit type parameter syntax: get<MyCustomObject>().
Configuration
typedoc.json
Removed intentionallyNotExported field from configuration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Consistent pattern: straightforward removal of default generic parameter across multiple files
  • Changes are repetitive in nature (same type signature modification replicated)
  • Test updates follow a clear pattern of making explicit what was implicit
  • Extra attention areas:
    • Verify that removing the default type parameter doesn't break downstream consumers who relied on global type augmentation
    • Confirm JSDoc examples accurately reflect the new required explicit typing approach

Poem

🐰 The global defaults have hopped away,
No more AblyObjectsTypes to play,
Now types must be explicit and clear,
A rabbit's delight—no ambiguity here!
Explicit parameters bring order and light,
TypeScript's strict path shines bright! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main objective: removing AblyObjectsTypes and requiring explicit type parameters for object structures.
Linked Issues check ✅ Passed The PR successfully removes the global AblyObjectsTypes interface and updates code to use explicit type parameters instead, aligning with PUB-3439 requirements.
Out of Scope Changes check ✅ Passed All changes are directly related to removing AblyObjectsTypes: typedoc config cleanup, type signature updates, and test file modifications are all in scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch PUB-3439/remove-AblyObjectsTypes

📜 Recent review details

Configuration used: CodeRabbit 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 9c9d510 and e70e1f7.

📒 Files selected for processing (4)
  • ably.d.ts (1 hunks)
  • src/plugins/objects/realtimeobject.ts (1 hunks)
  • test/package/browser/template/src/index-objects.ts (1 hunks)
  • typedoc.json (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/plugins/objects/realtimeobject.ts (2)
ably.d.ts (3)
  • Value (2432-2432)
  • PathObject (2773-2779)
  • LiveMap (2409-2412)
src/plugins/objects/livemap.ts (1)
  • LiveMap (48-983)
test/package/browser/template/src/index-objects.ts (2)
ably.d.ts (2)
  • PathObject (2773-2779)
  • LiveMap (2409-2412)
objects.d.ts (1)
  • LiveMap (16-28)
ably.d.ts (2)
src/plugins/objects/livemap.ts (1)
  • LiveMap (48-983)
objects.d.ts (1)
  • LiveMap (16-28)
⏰ 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). (6)
  • GitHub Check: test-node (16.x)
  • GitHub Check: test-node (20.x)
  • GitHub Check: test-node (18.x)
  • GitHub Check: test-browser (firefox)
  • GitHub Check: test-browser (chromium)
  • GitHub Check: test-browser (webkit)
🔇 Additional comments (4)
src/plugins/objects/realtimeobject.ts (1)

82-92: RealtimeObject.get implementation correctly matches new type contract

The new get<T extends Record<string, API.Value>>() signature aligns with the updated declaration (PathObject<LiveMap<T>>), keeps runtime behavior unchanged, and enforces that object shapes are composed from API.Value entries. Looks good to me; just ensure the TS type-check/build pipeline passes with the updated generics.

test/package/browser/template/src/index-objects.ts (1)

33-41: Test update correctly exercises explicit generic typing for Objects

Using channel.object.get<MyCustomObject>() and typing myObject as Ably.PathObject<LiveMap<MyCustomObject>> is consistent with the new RealtimeObject.get<T extends Record<string, Value>>() signature and ensures the custom object shape is driven by the explicit type parameter rather than any global default. This is a good regression test for the new typing model.

Also applies to: 46-48

ably.d.ts (1)

2317-2335: RealtimeObject.get declaration and docs are consistent with explicit type-parameter usage

The updated JSDoc (showing get<MyObject>()) and the new signature get<T extends Record<string, Value>>(): Promise<PathObject<LiveMap<T>>>; correctly reflect the removal of AblyObjectsTypes/AblyDefaultObject and ensure that object shapes are provided explicitly via T. This stays in sync with the implementation and the new test usage.

typedoc.json (1)

9-23: typedoc configuration cleanup looks safe

The requiredToBeDocumented list is unchanged and JSON remains valid; dropping the extra configuration around this block is a straightforward cleanup and should only affect how Typedoc treats previously “intentionally not exported” symbols. Please just confirm the docs pipeline/Typedoc job still runs cleanly.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants