Renaming types for clarity #478
Conversation
This prevents clashes with the ably-js ClientOptions type.
AndyTWF
requested review from
a team,
Copilot,
vladvelici and
splindsay-92
and removed request for
a team
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update the chat configuration across several modules by renaming types and constants to better reflect a chat-specific context. In the UI layer, the default room options have been replaced with a configuration that enables all features. Throughout the core and test modules, the generic client option types have been renamed from ClientOptions to ChatClientOptions (and their normalized versions accordingly) with corresponding updates to function signatures and member types. These modifications ensure a consistent and clear naming convention for chat functionality without altering core business logic. Changes
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 (
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
PR Overview
This PR renames types and constants for improved clarity and to avoid naming conflicts.
- Renames ClientOptions to ChatClientOptions across the codebase.
- Renames DefaultRoomOptions to AllFeaturesEnabled, updating both the constant and corresponding JSDoc.
- Updates all related type definitions, imports, and usages accordingly.
Reviewed Changes
| File | Description |
|---|---|
| src/core/room-options.ts | Renamed constant and updated documentation to reflect new naming. |
| test/helper/chat.ts | Updated type import and usage from ClientOptions to ChatClientOptions. |
| src/core/index.ts | Adjusted exports for renamed types and constants. |
| src/core/rooms.ts | Updated type annotations and constructor parameter types. |
| test/helper/options.ts | Updated type and helper function signatures to use new types. |
| src/core/logger.ts | Changed import and function parameter types for consistency. |
| src/core/config.ts | Renamed interface and normalized type definitions accordingly. |
| src/core/chat.ts | Updated constructor and getter type signatures for the client options. |
| demo/src/App.tsx | Updated import and usage of room options from DefaultRoomOptions to AllFeaturesEnabled. |
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
AndyTWF
force-pushed
the
renaming-types-for-clarity
branch
from
3be56ee to
cd2eff7
Compare
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This avoids any ambiguity as to what they are and therefore clarifies that they're not a "recommendation" but a starting point.
AndyTWF
force-pushed
the
renaming-types-for-clarity
branch
from
cd2eff7 to
aad6d81
Compare
There was a problem hiding this comment.
PR Overview
This pull request refactors type and constant names for improved clarity and consistency. The changes include renaming ClientOptions to ChatClientOptions and DefaultRoomOptions to AllFeaturesEnabled, as well as updating all corresponding imports, type definitions, and documentation.
Reviewed Changes
| File | Description |
|---|---|
| src/core/room-options.ts | Renames DefaultRoomOptions to AllFeaturesEnabled and updates docs |
| src/core/index.ts | Updates exported types and constants to reflect renaming |
| src/core/rooms.ts | Changes configuration type references from ClientOptions to ChatClientOptions |
| src/core/config.ts | Renames ClientOptions to ChatClientOptions and NormalizedClientOptions to NormalizedChatClientOptions |
| src/core/chat.ts | Applies renaming to client options and updates related documentation |
| Various test files | Updates import and usage of the renamed types and constants |
| src/react/providers/chat-room-provider.tsx | Updates documentation and code references to use AllFeaturesEnabled |
Copilot reviewed 30 out of 30 changed files in this pull request and generated no comments.
Context
CHA-851
Docs PR: ably/docs#2455
Description
ClientOptionstoChatClientOptionsto avoid conflicts with ably-js.DefaultRoomOptionstoAllFeaturesEnabledso it doesn't imply that they're a recommendation.Checklist
Testing Instructions (Optional)
N/A
Summary by CodeRabbit
New Features
Refactor