refactor: To use @rocket.chat/rest-typings as the single source of truth for REST API type definitions.#6876
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces many local REST typing definitions with exports from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
deepak0x
force-pushed
the
refactor/rest-typings-setup_and_helpers_migration
branch
from
39693ae to
1c1e6dc
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: 3
🧹 Nitpick comments (1)
scripts/verify-migration-phase.sh (1)
1-52: Consider adding test execution to match documentation.MIGRATION_SUMMARY.md and MIGRATION_CHECKLIST.md both reference running
yarn testas part of the verification process, but this script only checks TypeScript compilation and linting. Consider adding test execution or updating the documentation to reflect the actual verification steps.🔎 Proposed addition of test execution
echo "✅ Linting passed with 0 errors" +# Run tests +echo "🧪 Running tests..." +if ! yarn test; then + echo "❌ Tests failed!" + exit 1 +fi +echo "✅ Tests passed" + echo "" echo "✅ Phase $PHASE verification complete!" echo " - TypeScript: ✅" echo " - Linting: ✅ (0 errors)" +echo " - Tests: ✅"
📜 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 ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.gitignoreMIGRATION_CHECKLIST.mdMIGRATION_PLAN.mdMIGRATION_SUMMARY.mdapp/definitions/rest/COMPATIBILITY_LAYER.mdapp/definitions/rest/helpers/PaginatedRequest.tsapp/definitions/rest/helpers/PaginatedResult.tsapp/definitions/rest/helpers/index.tspackage.jsonscripts/verify-migration-phase.sh
🔇 Additional comments (8)
app/definitions/rest/COMPATIBILITY_LAYER.md (1)
1-52: Documentation looks good, but note the .gitignore conflict.The compatibility layer documentation is well-structured and provides clear guidance with helpful examples. However, this file is listed in
.gitignore(line 92), which conflicts with it being committed. Please refer to the review comment on.gitignorefor resolution.MIGRATION_PLAN.md (1)
1-315: Comprehensive migration plan, but conflicts with .gitignore.The migration plan is well-structured with clear phases, time estimates, and quality gates. However, this file matches the
*MIGRATION*.mdpattern in.gitignore(line 88), which would prevent it from being tracked. Please refer to the review comment on.gitignorefor resolution.app/definitions/rest/helpers/PaginatedResult.ts (1)
1-2: No action needed—verification confirms the external type exists and all usages are type-safe.The
@rocket.chat/rest-typingspackage (v6.13.1) is properly declared in package.json, and the re-export is working correctly across all seven files that importPaginatedResult(directory.ts, settings.ts, teams.ts, videoConference.ts, omnichannel.ts, emojiCustom.ts, chat.ts). No type errors or import failures are present, confirming structure compatibility and type-safety.package.json (1)
50-53: The specified versions (6.13.1) exist, are published, and have no known security vulnerabilities. They are compatible by design, released together as matched versions in Rocket.Chat 6.13.1. The exact version pinning indicates intentional selection for stable compatibility with a specific Rocket.Chat server release, which is appropriate for a mobile client targeting that version. No action needed.app/definitions/rest/helpers/index.ts (1)
2-4: LGTM! Clear migration plan documented.The TODO comments effectively communicate the migration strategy and rationale for maintaining the current implementation during the transition phase.
MIGRATION_SUMMARY.md (1)
1-58: Well-structured migration documentation.The summary provides a clear overview of the migration process with helpful quick references, phase descriptions, quality gates, and important notes. The structure will guide developers effectively through the phased migration.
MIGRATION_CHECKLIST.md (1)
1-288: Excellent comprehensive migration checklist.This checklist is well-structured and provides clear, actionable steps for each migration phase. The inclusion of verification commands (
yarn lint,tsc --noEmit) after each category and standardized commit messages will help maintain quality and consistency throughout the migration process.app/definitions/rest/helpers/PaginatedRequest.ts (1)
1-2: No action needed. The re-export to@rocket.chat/rest-typingsis confirmed compatible.The migration refactored the local
PaginatedRequestdefinition to use the external package type, which correctly supports generic parametersT(request body) and an optionalS(sort field) as evidenced by usage patterns throughout the codebase (PaginatedRequest<{ query: string }, 'name'>,PaginatedRequest<{ text: string }>). The external type includes the expected fields:count,offset,sort, andquery. The codebase actively uses this re-export without type errors across multiple endpoint definitions.
deepak0x
force-pushed
the
refactor/rest-typings-setup_and_helpers_migration
branch
4 times, most recently
from
861a4fa to
d9b821b
Compare
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
Left a few comments.
| @@ -1,4 +1,7 @@ | |||
| /* eslint-disable @typescript-eslint/no-unused-vars */ | |||
| // TODO: Once we migrate all endpoints to use @rocket.chat/rest-typings, | |||
There was a problem hiding this comment.
Can you do it on this PR or start the work on this PR, so it serves as a POC?
There was a problem hiding this comment.
sure...will try my best...
deepak0x
force-pushed
the
refactor/rest-typings-setup_and_helpers_migration
branch
from
0aa5c81 to
b5e17a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/definitions/rest/helpers/index.ts (2)
1-1: Verify the necessity of the eslint-disable directive.The
no-unused-varsdisable is common in type-only files, but it can mask legitimate issues. Consider reviewing whether all type parameters are actually necessary.
2-4: Track the migration TODO for future completion.The POC comment indicates this is a temporary bridge implementation until full migration to
@rocket.chat/rest-typings. This technical debt should be tracked to ensure the migration is completed.Would you like me to create a tracking issue for the full migration to
@rocket.chat/rest-typingsendpoints (replacing the current Endpoints structure with/v1/prefixed paths)?
📜 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 ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
app/definitions/rest/helpers/PaginatedRequest.tsapp/definitions/rest/helpers/PaginatedResult.tsapp/definitions/rest/helpers/index.tsapp/definitions/rest/v1/channels.tsapp/definitions/rest/v1/chat.tsapp/definitions/rest/v1/directory.tsapp/definitions/rest/v1/emojiCustom.tsapp/definitions/rest/v1/groups.tsapp/definitions/rest/v1/im.tsapp/definitions/rest/v1/omnichannel.tsapp/definitions/rest/v1/settings.tsapp/definitions/rest/v1/teams.tsapp/definitions/rest/v1/videoConference.tsapp/lib/hooks/useEndpointData.tspackage.json
💤 Files with no reviewable changes (2)
- app/definitions/rest/helpers/PaginatedRequest.ts
- app/definitions/rest/helpers/PaginatedResult.ts
✅ Files skipped from review due to trivial changes (1)
- app/definitions/rest/v1/videoConference.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
app/definitions/rest/helpers/index.ts (1)
app/definitions/rest/v1/index.ts (1)
Endpoints(25-47)
app/lib/hooks/useEndpointData.ts (1)
app/definitions/rest/helpers/index.ts (4)
ErrorResult(182-182)Serialized(48-56)ResultFor(177-180)MatchPathPattern(59-61)
🔇 Additional comments (13)
app/lib/hooks/useEndpointData.ts (1)
45-51: The error-handling logic is correct and intentional.The SDK resolves promises in both success and error cases—it only rejects for network/connection errors. The logic correctly identifies errors when responses have
success: falseand treats all other resolved values as results. This matches the actual SDK behavior shown in the tests and is part of the@rocket.chat/rest-typingsmigration.The guards are appropriate:
e && typeof e === 'object' && 'success' in e && !e.successensures non-objects and objects without a success property pass through tosetResult, which is the correct behavior since the type system guarantees all SDK responses are objects with a success property.Likely an incorrect or invalid review comment.
app/definitions/rest/v1/im.ts (1)
1-1: LGTM! Import migration correctly applied.The migration from local
PaginatedRequestto@rocket.chat/rest-typingsfollows the same pattern as other files in this PR. Type-only import is appropriate.app/definitions/rest/v1/channels.ts (1)
1-1: LGTM! Consistent with the rest-typings migration.The migration from local
PaginatedRequestto@rocket.chat/rest-typingsis correctly implemented.app/definitions/rest/v1/teams.ts (1)
1-1: LGTM! Migration correctly applied.The migration from local
PaginatedResultto@rocket.chat/rest-typingsis consistent with the broader refactoring effort.app/definitions/rest/v1/settings.ts (1)
1-1: LGTM! Import migration correctly applied.The migration from local
PaginatedResultto@rocket.chat/rest-typingsis correctly implemented. The type is used in an intersection type at line 70, which should work seamlessly with the external definition.app/definitions/rest/v1/chat.ts (1)
1-1: LGTM! Migration to @rocket.chat/rest-typings is correctly implemented.The type-only import from
@rocket.chat/rest-typingsforPaginatedResultis properly used and compatible. The old local definition has been removed, and the genericPaginatedResult<T>type matches the usage patterns in chat.ts and other REST endpoint definitions. Package version 6.13.1 is stable and was released as part of the official Rocket.Chat 6.13.1 release.app/definitions/rest/v1/emojiCustom.ts (1)
1-1: LGTM! Clean migration to external types.The import consolidation from local helpers to
@rocket.chat/rest-typingsaligns with the PR objectives and maintains existing type signatures.app/definitions/rest/v1/directory.ts (1)
1-1: LGTM! Consistent with migration pattern.The import source change maintains the existing type contract while aligning with the centralized type definitions from
@rocket.chat/rest-typings.app/definitions/rest/v1/omnichannel.ts (1)
1-1: LGTM! Extensive migration completed successfully.The import migration to
@rocket.chat/rest-typingsis applied consistently across all OmnichannelEndpoints, maintaining type safety while reducing local type duplication.app/definitions/rest/helpers/index.ts (3)
29-29: Type intersection ensures string compatibility.The
TPathPattern & stringintersection correctly constrains the type forReplacePlaceholders, ensuring that only string-compatible path patterns are processed.
175-180: LGTM! Error handling wrapper aligns with SDK needs.The
ResultFortype correctly wraps operation results withSuccessResult,FailureResult, andUnauthorizedResultpatterns, providing comprehensive error handling for SDK consumers.
64-158: This is a POC adaptation—verify against actual rest-typings 6.13.1 when migrating endpoints.The type utilities are correctly documented as adaptations rather than direct implementations from
@rocket.chat/rest-typings. The file comment explicitly notes this is a temporary POC pattern awaiting full endpoint migration. The only direct import from rest-typings isReplacePlaceholders, which is properly re-exported; the remaining utilities (UrlParams,MethodOf,PathFor, etc.) are locally implemented to match the current Endpoints structure.Before merging, confirm:
- The local type implementations are compatible with rest-typings 6.13.1's type signatures when endpoints are eventually migrated to use
/v1/prefix paths- No breaking changes in future rest-typings versions (6.13.1+) will affect the mapped type patterns used here
The TypeScript strict mode configuration and tsc in the lint script will catch type errors during compilation, but real compatibility should be verified once these types are actively used.
app/definitions/rest/v1/groups.ts (1)
1-1: Clean refactor with confirmed package compatibility.The migration to
@rocket.chat/rest-typingsimproves type consistency across the codebase. The package version6.13.1is correctly specified inpackage.json, andPaginatedRequestis properly exported and already in use across multiple endpoint definitions.
deepak0x
changed the title
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)
app/lib/services/restApi.ts (1)
333-346: Fix inconsistent payload field naming intoggleReadStatus.The function uses different field names for the room identifier between endpoints:
subscriptions.unreadreceives{ roomId }subscriptions.readreceives{ rid: roomId }The
subscriptions.readendpoint pattern is confirmed byapp/lib/methods/readMessages.ts(line 37), which also uses{ rid }. To maintain consistency,subscriptions.unreadshould useridinstead ofroomId.Additionally, the return type
Promise<any>loses type safety. Consider defining the actual response type from rest-typings or adding a TODO comment to restore typing.
🤖 Fix all issues with AI agents
In `@app/lib/methods/getSingleMessage.ts`:
- Around line 8-9: getSingleMessage currently uses an unsafe type assertion "as
IMessage" hiding a structural mismatch (response has _id while IMessage expects
id); update getSingleMessage to either map the sdk.get('chat.getMessage') result
into the correct shape by copying _id to id before resolving (preserve all other
fields) or change the function return type to the raw response type from
rest-typings and remove the assertion so callers like getMessageInfo and
getThreadName explicitly handle the _id→id mapping; locate getSingleMessage and
the sdk.get('chat.getMessage') call to implement the mapping or adjust the
return type accordingly.
In `@app/lib/services/restApi.ts`:
- Around line 359-361: The getUserPreferences function is incorrectly passing a
userId to sdk.get for the 'users.getPreferences' endpoint which accepts no
parameters; update the function signature remove the userId argument, remove the
`{ userId } as any` payload, and call sdk.get('users.getPreferences') with no
second argument inside getUserPreferences; if you actually need another user's
preferences, replace the call with the correct endpoint/mechanism instead of
passing userId to 'users.getPreferences'.
In `@app/views/DiscussionsView/index.tsx`:
- Around line 67-73: The code casts result to any which removes type safety;
instead type the response returned by the API (avoid "as any") by
importing/using the appropriate type from `@rocket.chat/rest-typings` or declare a
local interface matching { messages?: IMessageFromServer[]; count?: number;
total?: number } and use a type guard/adapter to convert/validate result before
using it; then replace uses of || with nullish coalescing (use offset.current +=
resultData.count ?? 0 and total.current = resultData.total ?? 0) and keep the
existing logic around isSearching/setSearch/setDiscussions but operate on the
properly typed resultData.
In `@app/views/ReadReceiptView/index.tsx`:
- Line 90: The current cast "result.receipts as any" in ReadReceiptView
(index.tsx) bypasses TS safety; replace the cast by aligning the response type
from chat.getMessageReadReceipts with the expected receipts shape (or add a
proper adapter/mapper) and narrow the union with a type guard: inspect
result.success and then assign result.receipts to a strongly typed variable
(e.g., IReadReceipt[] or the appropriate interface from
`@rocket.chat/rest-typings`) or transform result.receipts into that interface;
update the rest-typings response type to include the receipts field if it’s
missing, or implement a local typed adapter function (e.g., mapReceiptsFromSdk)
and use that instead of casting to any.
In `@app/views/UserNotificationPreferencesView/index.tsx`:
- Line 49: The code is using an unsafe cast (result.preferences as any) which
bypasses typing; replace this by aligning types: import the preferences type
from `@rocket.chat/rest-typings` and use it for the local INotificationPreferences
(or update INotificationPreferences to match that imported type), change the
setPreferences call to accept the correctly typed value, and remove the other as
any in getUserPreferences in restApi.ts by updating that function to return the
proper typed response or add a small adapter function (e.g.,
transformUserPreferences) to convert the SDK shape into INotificationPreferences
before calling setPreferences.
♻️ Duplicate comments (2)
app/views/RoomView/RightButtons.tsx (1)
280-281: Duplicateas anycast pattern.This is the same type-unsafe pattern as in
Livechat.tsx. Once you extract a shared typed helper (as suggested in the other file), apply it here as well to maintain consistency and type safety across the codebase.app/views/RoomActionsView/index.tsx (1)
411-412: Duplicateas anycast pattern.Same type-unsafe workaround for department info. Consolidate with the shared helper suggested for
Livechat.tsx.
🧹 Nitpick comments (18)
app/views/RoomInfoView/index.tsx (1)
171-172: Avoidas anycasts; use proper type alignment instead.These
as anyassertions undermine the type safety benefits of migrating to@rocket.chat/rest-typings. If there's a mismatch between the REST response type and localIUser, consider:
- Using a more specific type assertion (e.g.,
as Pick<IUser, 'username' | 'roles'>)- Creating a type adapter function that maps the REST response to the expected local type
- Extending/aligning the local types with rest-typings
Note the inconsistency: the
elsebranch (lines 178-179) handlesroomUserwithout any casts, suggesting the types should be compatible.♻️ Suggested approach
if (result.success) { const { user } = result; - const r = handleRoles(user as any); - setRoomUser({ ...user, roles: r } as any); + const r = handleRoles(user as Pick<IUser, 'username' | 'roles'>); + setRoomUser({ ...user, roles: r } as IUser); }Or create a mapper function if the types diverge significantly:
const mapRestUserToRoomUser = (user: RestUserType): Partial<IUser> => ({ _id: user._id, username: user.username, name: user.name, roles: user.roles, // ... map other fields });app/views/CannedResponsesListView/index.tsx (2)
88-88: Double cast indicates type mismatch between rest-typings and local definition.The
as unknown as ILivechatDepartment[]pattern is a valid escape hatch for the migration, but it signals that the@rocket.chat/rest-typingsdepartment type differs from the localILivechatDepartmentinterface. Consider aligning the local interface with the external type definition in a follow-up to eliminate this cast.
130-130: Prefer double cast overas anyfor type safety.Casting to
anycompletely disables type checking on the state update. For consistency with line 88 and to retain some type safety, consider using the double cast pattern instead:♻️ Suggested improvement
- setCannedResponses(prevCanned => (debounced ? res.cannedResponses : [...prevCanned, ...res.cannedResponses]) as any); + setCannedResponses(prevCanned => + (debounced ? res.cannedResponses : [...prevCanned, ...res.cannedResponses]) as unknown as ICannedResponse[] + );This maintains the type escape hatch needed for the migration while preserving the declared state type.
app/lib/methods/getCustomEmojis.ts (1)
140-142: Unsafe type assertion masks a type mismatch.The
as unknown as TCustomEmojiModel[]cast bypasses type safety entirely. The API returns raw emoji objects (with_id), whileTCustomEmojiModelis a WatermelonDBModeltype (withidand Model methods). This mismatch is evident inupdateEmojiswhereupdateitems are accessed via._idbutallRecordsitems via.id.Consider updating the
IUpdateEmojisinterface to use the correct API response type (e.g., from@rocket.chat/rest-typings) forupdateandremoveinstead ofTCustomEmojiModel[]:♻️ Suggested approach
+import type { ICustomEmoji } from '@rocket.chat/core-typings'; // or appropriate type from rest-typings interface IUpdateEmojis { - update: TCustomEmojiModel[]; - remove?: TCustomEmojiModel[]; + update: ICustomEmoji[]; + remove?: ICustomEmoji[]; allRecords: TCustomEmojiModel[]; }Then the call site would not need the unsafe cast:
-const changedEmojis = await updateEmojis({ update: (update || []) as unknown as TCustomEmojiModel[], remove: (remove || []) as unknown as TCustomEmojiModel[], allRecords }); +const changedEmojis = await updateEmojis({ update: update || [], remove: remove || [], allRecords });app/views/ThreadMessagesView/index.tsx (2)
345-351: Prefer proper type adapters overas anycasts.The
as anycasts bypass TypeScript's type safety entirely, which undermines the goal of using@rocket.chat/rest-typingsas the source of truth. If the types from the external package don't match the internalIMessagetype expected byupdateThreads, consider:
- Creating a type adapter function to map the external thread type to
IMessage- Using a more specific type assertion if the types are compatible
- Updating
updateThreadsto accept the external type directlyThe fallback
resultData.count || result.threads.lengthsuggests the response shape is uncertain—ifcountexists on the actual API response but not in the types, consider extending the type definition or using a type guard.Example type adapter approach
// Define adapter if types differ structurally type ThreadsListResponse = Awaited<ReturnType<typeof getThreadsList>>; const mapThreadsToMessages = (threads: ThreadsListResponse['threads']): IMessage[] => { return threads.map(thread => buildMessage(thread) as IMessage); }; // Then use: this.updateThreads({ update: mapThreadsToMessages(result.threads), lastThreadSync });For the
countproperty, if it's a known part of the API response:// Use type assertion with a specific interface interface ThreadsListResultWithCount { count?: number; threads: { length: number }; } const resultData = result as ThreadsListResultWithCount;
370-370: Sameas anypattern as inload—apply consistent type handling.This cast has the same type safety concerns noted above. When refactoring the
loadmethod, apply the same adapter pattern here for consistency.app/ee/omnichannel/lib/subscriptions/inquiry.ts (1)
93-94: Consider fixing the type at the source instead of inline annotations.The inline type annotations on lines 93–94 suggest that
departmentsis loosely typed after the SDK call. The root cause is theas anycast on thesdk.get()URL ingetAgentDepartments, which prevents TypeScript from inferring the response shape. While the annotations work, they:
- Add verbosity that wouldn't be needed if
getAgentDepartmentsexplicitly returned a typed response- Could mask type mismatches if the actual API response differs from the assumed
{ departmentId: string }shapeInstead of adding inline type guards, either remove the
as anycast from the SDK call or add an explicit return type togetAgentDepartmentsusing the existingOmnichannelEndpointstype from@rocket.chat/rest-typings. This would allow TypeScript to automatically infer the correct types downstream.app/views/ForwardLivechatView.tsx (1)
79-88: Double cast bypasses type safety and masks API response structure mismatch.The
as unknown as IServerRoompattern silences the compiler but doesn't guarantee that the actualrooms.infoAPI response includes all properties required byIServerRoom(e.g.,u,uids,usersCount). If the API returns a partial room object, property accesses could fail at runtime.The root issue is that
sdk.get('rooms.info', { roomId })has no explicit return type. Consider:
- Type the
getRoomInfocall using the response type from@rocket.chat/rest-typings- Create a mapper function that extracts and validates only the properties actually needed (
servedBy,departmentId)- Replace the local
IServerRoomwith the canonical type from the external packageapp/views/MessagesView/index.tsx (1)
190-190: Doubleas anycast masks type incompatibility betweenrest-typingsandIServerAttachment[].The input and output casts to
anysuppress TypeScript checking for this entire expression. Sincesdk.get()is strongly typed viarest-typingsgenerics,result.fileshas a defined type that doesn't match whatEncryption.decryptFilesexpects. Rather than silencing the compiler:
- Check the actual type that
rest-typingsdefines for the files endpoint response- Update
IServerAttachment[]to match, or add a type adapter ifrest-typingshas a different file structure- Ensure
decryptFilesproperly types its return value instead of falling back toanyapp/views/RoomInfoView/Livechat.tsx (1)
35-36: Avoidas anycast; extract a typed helper instead.Casting
result as anydefeats the purpose of adopting typed REST definitions. This same pattern appears in at least three files (Livechat.tsx,RightButtons.tsx,RoomActionsView/index.tsx), which suggests creating a shared utility.Consider narrowing the type or adding a type guard:
♻️ Suggested approach
Create a helper in the REST API layer that properly types the department response:
// In app/lib/services/restApi.ts or a dedicated helper export const getDepartmentFromResult = ( result: Awaited<ReturnType<typeof getDepartmentInfo>> ): ILivechatDepartment | undefined => { if (result.success && 'department' in result) { return result.department as ILivechatDepartment; } return undefined; };Then usage becomes type-safe:
-const resultData = result as any; -setDepartment(resultData.department as ILivechatDepartment); +const dept = getDepartmentFromResult(result); +if (dept) setDepartment(dept);app/definitions/rest/v1/invites.ts (1)
1-9: Consider extractingRemoveV1Prefixto a shared utility.The
RemoveV1Prefixtype is duplicated across multiple endpoint definition files (channels.ts,chat.ts,dm.ts,groups.ts,im.ts,omnichannel.ts,rooms.ts,invites.ts, etc.). Extract it to a common location to reduce duplication.♻️ Suggested refactor
Create a shared utility file:
// app/definitions/rest/v1/helpers.ts export type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; export type AdaptEndpoints<T> = { [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; };Then each endpoint file becomes simpler:
-import type { InvitesEndpoints as RestTypingsInvitesEndpoints } from '@rocket.chat/rest-typings'; - -type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; - -type AdaptInvitesEndpoints<T> = { - [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; -}; - -export type InvitesEndpoints = AdaptInvitesEndpoints<RestTypingsInvitesEndpoints>; +import type { InvitesEndpoints as RestTypingsInvitesEndpoints } from '@rocket.chat/rest-typings'; +import type { AdaptEndpoints } from './helpers'; + +export type InvitesEndpoints = AdaptEndpoints<RestTypingsInvitesEndpoints>;app/definitions/rest/v1/channels.ts (1)
1-9: Transformation approach is correct; consider extracting shared utility types.The type transformation correctly strips the
/v1/prefix from endpoint keys. However,RemoveV1Prefix<T>and theAdapt*Endpoints<T>pattern are duplicated across multiple files (channels, chat, users, dm, im, etc.).Consider extracting these to a shared utility file to reduce duplication:
♻️ Suggested shared utility
Create a shared utility file (e.g.,
app/definitions/rest/v1/helpers.ts):export type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; export type AdaptEndpoints<T> = { [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; };Then simplify each endpoint file:
-import type { ChannelsEndpoints as RestTypingsChannelsEndpoints } from '@rocket.chat/rest-typings'; - -type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; - -type AdaptChannelsEndpoints<T> = { - [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; -}; - -export type ChannelsEndpoints = AdaptChannelsEndpoints<RestTypingsChannelsEndpoints>; +import type { ChannelsEndpoints as RestTypingsChannelsEndpoints } from '@rocket.chat/rest-typings'; +import type { AdaptEndpoints } from './helpers'; + +export type ChannelsEndpoints = AdaptEndpoints<RestTypingsChannelsEndpoints>;app/definitions/rest/v1/rooms.ts (1)
1-9: Consider extractingRemoveV1Prefixto a shared utility.The implementation is correct and the approach of deriving endpoint types from
@rocket.chat/rest-typingsis sound. However,RemoveV1Prefix<T>is duplicated across multiple files (rooms.ts, teams.ts, emojiCustom.ts, omnichannel.ts, and likely others such as channels.ts, chat.ts, groups.ts per the AI summary).Extract this helper to a shared location to reduce duplication and simplify future maintenance.
Example shared utility file
Create a file like
app/definitions/rest/v1/helpers.ts:export type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; export type AdaptEndpoints<T> = { [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; };Then in each endpoint file:
-type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; - -type AdaptRoomsEndpoints<T> = { - [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; -}; - -export type RoomsEndpoints = AdaptRoomsEndpoints<RestTypingsRoomsEndpoints>; +import type { AdaptEndpoints } from './helpers'; + +export type RoomsEndpoints = AdaptEndpoints<RestTypingsRoomsEndpoints>;app/lib/services/restApi.ts (4)
260-260: Consider documenting whyas anyis needed here.The conditional param structure (different shapes for channels vs groups, and version-dependent fields) likely doesn't align with the strict rest-typings definitions. This cast is acceptable for now, but consider adding a brief comment explaining the type mismatch or filing a follow-up to properly type this once rest-typings supports the variant shapes.
425-434: Document that this endpoint may not be in rest-typings yet.The
livechat/inquiries.returnAsInquiryendpoint (RC 7.12.0+) requiresas anycasts, likely because it's not yet defined in the installed@rocket.chat/rest-typingsversion. Consider adding a comment or TODO to remove the casts when rest-typings is updated.
488-490: Consider separating query params from the URL path.The query parameter
enabledDepartmentsOnly=trueis embedded in the URL string. For consistency with other SDK calls and better type support in the future, consider passing it as a params object if the SDK supports it:sdk.get(`livechat/agents/${uid}/departments`, { enabledDepartmentsOnly: true })
965-968: Clarify the endpoint version and return type.The
// RC ?comment indicates uncertainty about when this endpoint was introduced. The return typePromise<boolean | {}>is also unusual. Consider:
- Documenting the actual RC version if known
- Refining the return type to match the actual API response
app/lib/methods/getSingleMessage.ts (1)
4-15: Consider refactoring to avoid the Promise constructor anti-pattern.Wrapping
asyncinsidenew Promise()is redundant and can obscure errors. This is pre-existing code, but a future cleanup could simplify it:♻️ Suggested refactor
-const getSingleMessage = (messageId: string): Promise<IMessage> => - new Promise(async (resolve, reject) => { - try { - const result = await getSingleMessageService(messageId); - if (result.success && result.message) { - return resolve(result.message as IMessage); - } - return reject(); - } catch (e) { - return reject(e); - } - }); +const getSingleMessage = async (messageId: string): Promise<IMessage> => { + const result = await getSingleMessageService(messageId); + if (result.success && result.message) { + return result.message as IMessage; + } + throw new Error('Failed to fetch message'); +};
📜 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 (31)
app/definitions/rest/v1/channels.tsapp/definitions/rest/v1/chat.tsapp/definitions/rest/v1/dm.tsapp/definitions/rest/v1/emojiCustom.tsapp/definitions/rest/v1/groups.tsapp/definitions/rest/v1/im.tsapp/definitions/rest/v1/invites.tsapp/definitions/rest/v1/omnichannel.tsapp/definitions/rest/v1/rooms.tsapp/definitions/rest/v1/teams.tsapp/definitions/rest/v1/users.tsapp/ee/omnichannel/lib/subscriptions/inquiry.tsapp/lib/hooks/useUserData.tsapp/lib/methods/getCustomEmojis.tsapp/lib/methods/getRoomInfo.tsapp/lib/methods/getSingleMessage.tsapp/lib/services/restApi.tsapp/views/CannedResponsesListView/index.tsxapp/views/DiscussionsView/index.tsxapp/views/ForwardLivechatView.tsxapp/views/LivechatEditView.tsxapp/views/MessagesView/index.tsxapp/views/ReadReceiptView/index.tsxapp/views/RoomActionsView/index.tsxapp/views/RoomInfoView/Livechat.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/RightButtons.tsxapp/views/SearchMessagesView/index.tsxapp/views/TeamChannelsView.tsxapp/views/ThreadMessagesView/index.tsxapp/views/UserNotificationPreferencesView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/definitions/rest/v1/groups.ts
🧰 Additional context used
🧬 Code graph analysis (10)
app/lib/methods/getSingleMessage.ts (1)
app/definitions/IMessage.ts (1)
IMessage(147-178)
app/views/ForwardLivechatView.tsx (1)
app/definitions/IRoom.ts (1)
IServerRoom(152-242)
app/views/RoomInfoView/Livechat.tsx (1)
app/definitions/ILivechatDepartment.ts (1)
ILivechatDepartment(1-23)
app/views/SearchMessagesView/index.tsx (1)
app/definitions/IMessage.ts (2)
IMessageFromServer(109-137)TMessageModel(180-183)
app/views/CannedResponsesListView/index.tsx (1)
app/definitions/ILivechatDepartment.ts (1)
ILivechatDepartment(1-23)
app/lib/services/restApi.ts (1)
app/lib/notifications/index.ts (1)
getDeviceToken(83-83)
app/views/RoomActionsView/index.tsx (1)
app/definitions/ILivechatDepartment.ts (1)
ILivechatDepartment(1-23)
app/lib/methods/getRoomInfo.ts (1)
app/definitions/IRoom.ts (1)
RoomType(147-147)
app/views/RoomView/RightButtons.tsx (1)
app/definitions/ILivechatDepartment.ts (1)
ILivechatDepartment(1-23)
app/lib/methods/getCustomEmojis.ts (1)
app/definitions/IEmoji.ts (1)
TCustomEmojiModel(35-35)
🔇 Additional comments (29)
app/lib/methods/getRoomInfo.ts (2)
25-25: LGTM! Good defensive null check.Adding the explicit
result.roomguard is a sensible improvement that prevents potential runtime errors if the API response lacks theroomproperty despitesuccess: true.
30-30: Type cast is consistent with existing pattern.The
as RoomTypeassertion aligns with line 20 and is a reasonable adaptation for the rest-typings migration where the external package may define a broader type for thetproperty.app/views/SearchMessagesView/index.tsx (1)
182-182: LGTM!The
|| []fallback correctly handles the case whereresult.messagesis undefined (from the optional chaining on line 164). The type assertion to(IMessageFromServer | TMessageModel)[]is appropriate here since the method needs to return a union type that accommodates both server responses and local WatermelonDB models returned from the encrypted room branch.app/views/TeamChannelsView.tsx (1)
336-363: Good defensive check, but verify the empty params fallback behavior.The added check for
result.roomis a sound improvement that prevents accessing properties on a potentially undefined object. However, when the condition fails (either!result.successor!result.room),goRoomis still invoked with an emptyparamsobject, which could lead to unexpected navigation behavior or errors downstream.Consider whether an early return or user feedback should be provided when room info cannot be retrieved:
Suggested improvement
onPressItem = debounce( async (item: IItem) => { logEvent(events.TC_GO_ROOM); const { isMasterDetail } = this.props; try { - let params = {}; const result = await getRoomInfo(item._id); - if (result.success && result.room) { - params = { - rid: item._id, - name: getRoomTitle(result.room), - joinCodeRequired: result.room.joinCodeRequired, - t: result.room.t, - teamId: result.room.teamId - }; + if (!result.success || !result.room) { + showErrorAlert(I18n.t('Room_not_found')); + return; } + const params = { + rid: item._id, + name: getRoomTitle(result.room), + joinCodeRequired: result.room.joinCodeRequired, + t: result.room.t, + teamId: result.room.teamId + }; goRoom({ item: params, isMasterDetail }); } catch (e: any) {app/lib/methods/getCustomEmojis.ts (1)
127-130: LGTM on parameter shape change.The updated parameter shape
{ query: string; updatedSince?: string }aligns with theemoji-custom.listendpoint definition from@rocket.chat/rest-typings. Initializing with{ query: '' }correctly satisfies the required parameter.app/views/LivechatEditView.tsx (1)
124-124: The endpoint's type annotation should be improved before enforcing strict types on the callback.The use of
anyon line 124 is a consequence of the endpoint being cast toas anyinapp/lib/services/restApi.ts:490. While using a proper type from@rocket.chat/rest-typingswould be ideal, the SDK cannot infer correct types when the endpoint path itself is marked asany. Consider fixing the root cause by removing or refining theas anycast on the endpoint definition first, which would then allow the callback parameter to receive proper typing through the SDK's genericResultFor<'GET', MatchPathPattern<TPath>>mechanism.app/lib/hooks/useUserData.ts (1)
35-36: LGTM!Good defensive defaults. The fallbacks ensure consistency with the initial state shape and prevent potential
undefinedpropagation to child components.app/views/RoomActionsView/index.tsx (3)
203-208: LGTM!Good use of nullish coalescing to handle potentially undefined
counters.members. The distinction between?? undefined(for state) and?? 0(for the update function) correctly preserves the semantic difference between "unknown count" and "zero members".
246-247: Consistent nullish coalescing.Same defensive pattern applied correctly in the
componentDidMountpath.
612-612: Remove the unnecessaryanycast ifisLastOwneris available from the response type, or extend the type locally if it's missing.The
(r as any).isLastOwnercast suggestsisLastOwneris not typed in theteamListRoomsOfUserresponse from the SDK (which uses@rocket.chat/rest-typings). However,isLastOwneris already defined in the localIRoominterface as an optional boolean. Compare this withRoomMembersView/helpers.ts:156, which handles the same API response by casting the entire loop variable toanyinstead of just the property. If the endpoint's type definition in rest-typings lacksisLastOwner, consider either extending the response type locally or opening an issue upstream to add it to the type definitions.app/definitions/rest/v1/chat.ts (1)
1-9: LGTM!The transformation logic is correct and follows the same pattern established in other endpoint files. The duplication concern applies here as well—once the shared utility is extracted, this file can be simplified.
app/definitions/rest/v1/users.ts (1)
1-9: LGTM!Consistent with the transformation pattern used across other endpoint definition files. The type mapping correctly adapts
RestTypingsUsersEndpointsby stripping the/v1/prefix.app/definitions/rest/v1/dm.ts (1)
1-9: LGTM!Follows the established pattern for adapting REST typings. The transformation correctly maps
RestTypingsDmEndpointstoDmEndpointswith prefix stripping.app/definitions/rest/v1/im.ts (1)
1-9: LGTM!Consistent with the transformation approach across all endpoint definition files. The migration to
@rocket.chat/rest-typingsas the single source of truth is well-structured.app/definitions/rest/v1/rooms.ts (1)
11-13: LGTM!
TRoomsMediaResponseis unchanged and remains correctly defined as a local type.app/definitions/rest/v1/teams.ts (1)
1-9: LGTM — same refactor opportunity as noted in rooms.ts.The adapter pattern is correctly implemented. The
RemoveV1PrefixandAdaptTeamsEndpointshelpers duplicate the same logic from other files and would benefit from consolidation into a shared utility.app/definitions/rest/v1/emojiCustom.ts (1)
1-9: LGTM — same refactor opportunity applies.Correctly derives
EmojiCustomEndpointsfrom the external rest-typings package. The duplicated helper types should be consolidated per the suggestion in rooms.ts.app/definitions/rest/v1/omnichannel.ts (1)
1-9: LGTM — same refactor opportunity applies.Correctly derives
OmnichannelEndpointsfrom the external rest-typings package. Consider consolidating the duplicated helper types as suggested in the rooms.ts review.app/lib/services/restApi.ts (11)
271-277: LGTM!Converting
nulltoundefinedvia?? undefinedaligns with rest-typings expectations for optional parameters.
473-478: LGTM for the routing config endpoint cast.The
livechat/config/routingendpoint (RC 7.11.0+) cast is acceptable since it's likely not yet in rest-typings.
599-601: LGTM!Converting the boolean
ignoreto string via.toString()aligns with the API's expected parameter type.
646-651: LGTM with reservation.The
as anycasts ondataandcustomFieldsbypass rest-typings validation. The local types appear reasonable, but consider verifying alignment with rest-typings definitions when refining this migration.
657-659: LGTM!
696-704: LGTM!The sort object cast is acceptable - MongoDB-style sort objects often need casting due to strict typing constraints.
797-809: LGTM!
970-991: LGTM!The dual payload structure (content vs text) for encrypted and regular messages requires casting since rest-typings likely doesn't model both variants.
993-1011: LGTM!Explicit type annotation
'apn' | 'gcm'ensures the literal union type is preserved for rest-typings compatibility.
1083-1086: LGTM!
1095-1095: LGTM!
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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)
app/definitions/rest/v1/videoConference.ts (1)
9-13: Fix return type: endpoint returnsPromise<void>, notvoid.The
POSTmethod for this endpoint is typed incorrectly. The actual usage inapp/lib/methods/videoConfTimer.tscalls.catch()on the result ofupdateJitsiTimeout, which requires the return type to be a Promise:updateJitsiTimeout(rid).catch((e: unknown) => console.log(e));The return type should be
Promise<void>to match the actual behavior ofsdk.post():'video-conference/jitsi.update-timeout': { POST: (params: { roomId: string }) => Promise<void>; };
🤖 Fix all issues with AI agents
In `@app/definitions/rest/v1/channels.ts`:
- Around line 5-7: The closing brace in the type declaration for
AdaptChannelsEndpoints has inconsistent indentation (an extra tab before the
brace); align the closing brace with the start of the type declaration so it
matches the indentation of "type AdaptChannelsEndpoints<T> = {" and the opening
brace—edit the line containing the trailing brace for AdaptChannelsEndpoints to
remove the extra tab so formatting is consistent with RemoveV1Prefix and the
rest of the file.
In `@app/definitions/rest/v1/customUserStatus.ts`:
- Around line 11-13: The closing brace in the type declaration
AdaptCustomUserStatusEndpoints has extra indentation; adjust the closing brace
for that mapped type to match the opening indentation (remove the double tab so
the brace aligns with the type declaration), ensuring consistent indentation for
the entire type block.
In `@app/definitions/rest/v1/settings.ts`:
- Line 67: The isOauthCustomConfiguration type guard currently always returns
Boolean(config), which accepts any truthy value and doesn't validate required
fields; update isOauthCustomConfiguration to perform real runtime checks for the
OauthCustomConfiguration shape (e.g., verify presence and types of required
properties like clientId, clientSecret, redirectUri, or whichever fields the
OauthCustomConfiguration interface declares) or delete the function entirely if
runtime validation is unnecessary and you rely solely on compile-time types;
reference the isOauthCustomConfiguration function and the
OauthCustomConfiguration type when making the change.
- Around line 69-80: The local declaration of SettingsEndpoints redefines the
transformed endpoint 'settings/:_id' (from
AdaptSettingsEndpoints<RestTypingsSettingsEndpoints>) which can produce a union
if the local GET/POST signatures differ from the upstream; either add a concise
in-line comment above the 'settings/:_id' block stating that this is an
intentional override of the upstream type (mentioning AdaptSettingsEndpoints and
RestTypingsSettingsEndpoints) and why, or confirm and align the local signatures
(GET returning Pick<ISetting,'_id'|'value'> and POST accepting
SettingsUpdateProps) so they exactly match the upstream contract to avoid a
merged union type. Ensure the comment references the symbols SettingsEndpoints,
'settings/:_id', ISetting, and SettingsUpdateProps so future readers know the
override is deliberate and compatible.
In `@app/definitions/rest/v1/teams.ts`:
- Around line 5-7: The closing brace in the generic type declaration for
AdaptTeamsEndpoints has inconsistent indentation (an extra tab); fix the
formatting by aligning the closing brace with the opening "type
AdaptTeamsEndpoints<T> = {" line and the rest of the block so it matches the
style used in channels.ts—adjust the indentation of the line containing "};"
that closes the mapped type using RemoveV1Prefix and the mapped key signature.
♻️ Duplicate comments (1)
app/lib/services/restApi.ts (1)
359-361: Theusers.getPreferencesendpoint does not accept auserIdparameter.This issue was previously identified. The API returns preferences for the authenticated user based on headers only—the
userIdparameter will be ignored. Theas anycast masks an incorrect API call.
🧹 Nitpick comments (4)
app/lib/services/restApi.ts (2)
337-346: Return type loosened toPromise<any>reduces caller type safety.Changing from a typed response to
Promise<any>means callers lose compile-time guarantees on the response shape. If the rest-typings package provides proper response types forsubscriptions.readandsubscriptions.unread, consider using them instead ofany.
965-968: Trackas anycasts for post-migration cleanup.This file has accumulated many
as anycasts (e2e.resetRoomKey,chat.update,users.removeOtherTokens,rooms.invite, etc.) as workarounds during the rest-typings migration. Consider:
- Opening an issue to track these for cleanup
- Adding inline TODOs with issue references
- Contributing missing type definitions upstream to
@rocket.chat/rest-typingsapp/definitions/rest/v1/channels.ts (1)
1-9: Consider centralizingRemoveV1Prefix<T>into a shared utility type.This helper type is duplicated across multiple endpoint definition files (
channels.ts,moderation.ts,commands.ts,teams.ts,customUserStatus.ts, and likely others). Extract it to a shared location (e.g.,app/definitions/rest/v1/helpers.ts) and import where needed.♻️ Suggested shared helper
Create a new file
app/definitions/rest/v1/helpers.ts:export type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; export type AdaptEndpoints<T> = { [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; };Then in each endpoint file:
-type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; - -type AdaptChannelsEndpoints<T> = { - [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; - }; +import type { AdaptEndpoints } from './helpers'; -export type ChannelsEndpoints = AdaptChannelsEndpoints<RestTypingsChannelsEndpoints>; +export type ChannelsEndpoints = AdaptEndpoints<RestTypingsChannelsEndpoints>;app/definitions/rest/v1/e2e.ts (1)
3-7: ExtractRemoveV1Prefixto a shared utilities file to reduce duplication across 23 endpoint modules.
RemoveV1Prefixand the adapter type pattern are identically duplicated across all v1 endpoint files (channels, chat, e2e, rooms, users, etc.). Define these once inapp/definitions/rest/helpers.tsand import as needed.♻️ Suggested refactoring
Create shared helper file:
// app/definitions/rest/helpers.ts export type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; export type AdaptEndpoints<T> = { [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; };Then in each endpoint file:
import type { E2eEndpoints as RestTypingsE2eEndpoints } from '@rocket.chat/rest-typings'; +import type { AdaptEndpoints } from '../helpers'; -type RemoveV1Prefix<T> = T extends `/v1/${infer Rest}` ? Rest : T; - -type AdaptE2eEndpoints<T> = { - [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; -}; - -export type E2eEndpoints = AdaptE2eEndpoints<RestTypingsE2eEndpoints> & { +export type E2eEndpoints = AdaptEndpoints<RestTypingsE2eEndpoints> & {
📜 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 (12)
app/definitions/rest/v1/autotranslate.tsapp/definitions/rest/v1/channels.tsapp/definitions/rest/v1/chat.tsapp/definitions/rest/v1/commands.tsapp/definitions/rest/v1/customUserStatus.tsapp/definitions/rest/v1/e2e.tsapp/definitions/rest/v1/moderation.tsapp/definitions/rest/v1/settings.tsapp/definitions/rest/v1/teams.tsapp/definitions/rest/v1/videoConference.tsapp/lib/methods/getSlashCommands.tsapp/lib/services/restApi.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/definitions/rest/v1/chat.ts
🧰 Additional context used
🧬 Code graph analysis (6)
app/definitions/rest/v1/customUserStatus.ts (1)
app/definitions/rest/v1/index.ts (1)
Endpoints(25-47)
app/lib/methods/getSlashCommands.ts (1)
app/definitions/ISlashCommand.ts (1)
ISlashCommandResult(12-14)
app/definitions/rest/v1/commands.ts (1)
app/definitions/rest/v1/index.ts (1)
Endpoints(25-47)
app/definitions/rest/v1/autotranslate.ts (1)
app/definitions/rest/v1/index.ts (1)
Endpoints(25-47)
app/lib/services/restApi.ts (1)
app/lib/notifications/index.ts (1)
getDeviceToken(83-83)
app/definitions/rest/v1/settings.ts (1)
app/definitions/ISetting.ts (2)
ISettingColor(102-106)ISetting(20-20)
🔇 Additional comments (17)
app/lib/methods/getSlashCommands.ts (3)
27-28: Reasonable type assertion for improved internal type safety.The typed alias improves type safety within the function. Given that the null/length check on line 22 guards against undefined/empty cases, this is a safe approach.
However, the reliance on
@ts-ignoreat lines 14-15 and 20-21 suggests the SDK endpoint typings don't yet align with the actual API response shape. As this PR aims to use@rocket.chat/rest-typingsas the single source of truth, consider creating a follow-up task to properly type thecommands.listendpoint response so these@ts-ignorecomments can be removed.
30-34: LGTM!Consistent use of the typed
commandsTypedvariable in filter operations. The explicit type annotations on callback parameters (e.g.,i1: ISlashCommandResult) are now technically redundant since TypeScript can infer them from the array type, but they don't hurt readability.
54-54: LGTM!Correctly uses
commandsTypedfor the update lookup, ensuring consistent typing throughout the function.app/definitions/rest/v1/autotranslate.ts (1)
1-15: Type transformation syntax is correct, but endpoint extraction requires verification.The type transformation uses idiomatic TypeScript patterns—conditional mapped types and template literal string manipulation are syntactically sound. The
RemoveV1Prefixutility correctly strips the/v1/prefix from keys, and the overall pipeline structure is well-designed.However, the success of this approach depends on whether the
Endpointstype from@rocket.chat/rest-typingsactually contains keys matching the/v1/autotranslate.${string}pattern. Verify that the rest-typings package exports autotranslate endpoints with this naming convention to ensure the extraction filter produces the expected keys.app/definitions/rest/v1/videoConference.ts (1)
1-7: Clean utility type pattern for adapting upstream types.The
RemoveV1Prefixconditional type and the key remapping inAdaptVideoConferenceEndpointsare well-designed. This approach cleanly strips the/v1/prefix from the canonical rest-typings package while preserving type safety.app/lib/services/restApi.ts (6)
260-260: Acceptable migration workaround, but track for cleanup.The
as anycast here is a reasonable interim solution while integrating rest-typings. Consider adding a TODO comment or tracking issue to revisit these casts once the endpoint definitions are fully aligned.
473-478: LGTM with caveat.The endpoint path cast is a necessary workaround. The explicit return type annotation maintains type safety for callers despite the internal
as any.
599-601: Verifyignoreparameter type forchat.ignoreUserendpoint.The change to
ignore.toString()suggests the rest-typings definition expects a string. Confirm this matches the Rocket.Chat server API expectation—some REST APIs accept boolean query params directly.
997-1002: LGTM!The explicit type annotation
'apn' | 'gcm'improves clarity and ensures type safety for the push token type.
428-429: [Your rewritten review comment text here]
[Exactly ONE classification tag]
818-826: Breaking change:triggerIdis now required.All callers of
runSlashCommandin the codebase already provide this parameter—for example,MessageComposer.tsxgenerates it viagenerateTriggerId(appId)before calling the function. No changes needed.app/definitions/rest/v1/moderation.ts (1)
1-9: LGTM!The type transformation correctly adapts
RestTypingsModerationEndpointsby stripping the/v1/prefix from endpoint keys. The implementation aligns with the broader migration pattern in this PR.app/definitions/rest/v1/commands.ts (1)
1-15: LGTM - extraction approach is appropriate.Since
@rocket.chat/rest-typingsdoesn't exportCommandsEndpointsdirectly, extracting them from the mainEndpointstype using the key filter pattern is the correct approach. The transformation logic is sound.app/definitions/rest/v1/customUserStatus.ts (1)
1-15: LGTM - type logic is correct.The extraction and adaptation pattern correctly filters
custom-user-status.*endpoints and strips the/v1/prefix. Follows the same approach ascommands.tsfor endpoints not directly exported by@rocket.chat/rest-typings.app/definitions/rest/v1/e2e.ts (1)
9-16: LGTM!The adapter pattern correctly transforms the imported
RestTypingsE2eEndpointsby stripping the/v1/prefix, and the intersection type properly extends it with local endpoints that aren't yet in the package. The implementation aligns well with the PR objective of using@rocket.chat/rest-typingsas the source of truth.app/definitions/rest/v1/settings.ts (2)
1-10: Helper types and imports align with refactoring goal.The
RemoveV1PrefixandAdaptSettingsEndpointsutility types correctly strip the/v1/prefix from upstream endpoint keys, enabling seamless integration with the existing codebase's endpoint naming convention. The special case handling for/v1/settings/:_idon line 9 is a clean approach.
18-34: Type guards correctly discriminate settings update variants.The property-based checks (
'execute' in props,'editor' in props && 'value' in props,'value' in props) properly narrow the union type at runtime. The order of checks matters when used in conditionals—ensure callers checkisSettingsUpdatePropsActionsandisSettingsUpdatePropsColorbeforeisSettingsUpdatePropDefaultsince the latter's check is more general.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/lib/services/restApi.ts`:
- Around line 1084-1087: The function logoutOtherLocations assumes
reduxStore.getState().login.user.id exists; add a guard in logoutOtherLocations
to read const { id } = reduxStore.getState().login.user and if id is falsy
either throw a clear Error or return a rejected Promise (or no-op) before
calling sdk.post('users.removeOtherTokens'), and pass a correctly typed payload
(e.g., { userId: id }) only when id is present to avoid sending undefined to
users.removeOtherTokens.
♻️ Duplicate comments (2)
app/definitions/rest/v1/teams.ts (1)
5-9: Indentation issue on closing brace appears unresolved.The past review flagged inconsistent indentation on line 7 (extra tab before
};). This line was not modified in the current revision, so the fix may not have been applied.🔧 Fix
type AdaptTeamsEndpoints<T> = { [K in keyof T as RemoveV1Prefix<K & string>]: T[K]; - }; +};app/definitions/rest/v1/customUserStatus.ts (1)
9-13: Type transformation logic is correct.The
RemoveV1Prefixconditional type andAdaptCustomUserStatusEndpointsmapped type correctly strip the/v1/prefix from endpoint keys. TheK & stringintersection properly narrows the key type for template literal compatibility.Note: An indentation issue on the closing brace was flagged in a previous review. Please ensure it has been addressed.
🧹 Nitpick comments (6)
app/lib/methods/getSingleMessage.ts (1)
4-5: Consider simplifying the Promise wrapper (optional).The
new Promise(async ...)pattern is redundant since async functions already return Promises. A simpler approach:♻️ Suggested simplification
-const getSingleMessage = (messageId: string): Promise<IMessage> => - new Promise(async (resolve, reject) => { - try { - const result = await getSingleMessageService(messageId); - if (result.success && result.message) { - const { message } = result; - // Map _id to id to match IMessage interface structure - // The API returns messages with _id, but IMessage requires id field - return resolve({ - ...message, - id: message._id - } as unknown as IMessage); - } - return reject(); - } catch (e) { - return reject(e); - } - }); +const getSingleMessage = async (messageId: string): Promise<IMessage> => { + const result = await getSingleMessageService(messageId); + if (result.success && result.message) { + const { message } = result; + // Map _id to id to match IMessage interface structure + // The API returns messages with _id, but IMessage requires id field + return { + ...message, + id: message._id + } as unknown as IMessage; + } + throw new Error('Failed to retrieve message'); +};app/definitions/rest/v1/settings.ts (1)
36-65: Consider tighteningunknownfield types incrementally.The extensive use of
unknownfor many fields is safer thanany, but if the actual shapes of these OAuth configuration fields are known fromLoginServiceConfigurationor backend documentation, consider typing them more precisely in a follow-up. This isn't blocking, but would improve type safety for consumers of this configuration.app/lib/services/restApi.ts (4)
333-346: Consider preserving return type safety.The
Promise<any>return type discards type information. If the two endpoints return different response shapes, consider using a union type or generic to preserve type safety for callers.-): Promise<any> => { +): Promise<{ success: boolean }> => {
489-491: Consider separating query parameters from path.Embedding query parameters in the path string is unconventional. If the SDK supports it, consider passing
enabledDepartmentsOnlyas a params object for clarity and type safety:- sdk.get(`livechat/agents/${uid}/departments?enabledDepartmentsOnly=true` as any); + sdk.get(`livechat/agents/${uid}/departments` as any, { enabledDepartmentsOnly: true });
647-652: Type alignment opportunity.The
as anycasts for bothdataandcustomFieldssuggest the localIProfileParamsdefinition may differ from@rocket.chat/rest-typings. Consider aligning the local type with the package definition to eliminate these casts.
426-435: Consider tracking missing endpoint type definition in a consolidated issue.The
as anycasts on line 430 indicatelivechat/inquiries.returnAsInquirylacks type definitions from@rocket.chat/rest-typings. This pattern is consistent with other newer endpoints in the file (lines 652, 968, 1096). While the file already contains TODO comments for missing definitions elsewhere, consolidating these into a single tracking issue for batch-updating rest-typings when the package is updated would be more maintainable than scattered inline TODOs.
📜 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 (9)
app/definitions/rest/v1/channels.tsapp/definitions/rest/v1/customUserStatus.tsapp/definitions/rest/v1/settings.tsapp/definitions/rest/v1/teams.tsapp/lib/methods/getSingleMessage.tsapp/lib/services/restApi.tsapp/views/DiscussionsView/index.tsxapp/views/ReadReceiptView/index.tsxapp/views/UserNotificationPreferencesView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- app/views/ReadReceiptView/index.tsx
- app/views/DiscussionsView/index.tsx
- app/views/UserNotificationPreferencesView/index.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
app/lib/methods/getSingleMessage.ts (1)
app/definitions/IMessage.ts (1)
IMessage(147-178)
app/definitions/rest/v1/settings.ts (1)
app/definitions/ISetting.ts (2)
ISettingColor(102-106)ISetting(20-20)
app/definitions/rest/v1/customUserStatus.ts (1)
app/definitions/rest/v1/index.ts (1)
Endpoints(25-47)
🔇 Additional comments (21)
app/definitions/rest/v1/teams.ts (1)
1-4: LGTM!The type-only import and
RemoveV1Prefixutility type are correctly implemented. The conditional template literal type correctly strips the/v1/prefix when present.app/lib/methods/getSingleMessage.ts (1)
8-15: Explicit_id→idmapping addresses the prior type mismatch concern.The change correctly maps the API's
_idfield to theidfield required byIMessage. The spread pattern preserves all other fields while adding the required mapping. Theas unknown as IMessageassertion is acceptable given the external API type constraints during this migration.app/definitions/rest/v1/customUserStatus.ts (2)
1-7: LGTM! Type extraction logic is well-structured.The use of template literal types to filter endpoints matching
/v1/custom-user-status.${string}is an idiomatic approach for extracting a subset of the upstreamEndpointstype.
15-15: Export correctly derives the type from upstream package.This approach ensures
CustomUserStatusEndpointsstays in sync with@rocket.chat/rest-typingswhile adapting the key format for local usage. This aligns well with the PR objective of using the package as the single source of truth.app/definitions/rest/v1/channels.ts (1)
1-9: Clean implementation of the type adaptation pattern.The use of
RemoveV1PrefixandAdaptChannelsEndpointshelper types to derive endpoints from@rocket.chat/rest-typingsis consistent with the PR's goal and follows the same pattern used in other endpoint files.app/definitions/rest/v1/settings.ts (3)
18-34: AI summary is inconsistent with actual code.The AI summary states that runtime type guards (
isSettingsUpdatePropsActions,isSettingsUpdatePropsColor,isSettingsUpdatePropDefault) were removed, but they are clearly present and correctly implemented in the code.
67-72: Type guard now provides meaningful validation.This implementation properly checks for the presence and types of required fields (
_id,loginStyle) and validates theloginStylevalue. This addresses the previous concern about the type guard accepting any truthy value.
74-87: Override documentation added as requested.The comments on lines 81-82 now clearly explain why
settings/:_idis overridden from upstream types. This addresses the previous review concern about documenting the intentional override.Note that the intersection type semantics will merge both the upstream and local type definitions for
settings/:_id. As long as the local types (ISetting,SettingsUpdateProps) remain compatible with the actual API responses, this approach maintains type safety while preserving codebase compatibility.app/lib/services/restApi.ts (13)
260-260: Acceptable cast for conditional payload structure.The
as anycast here accommodates the varyingparamsshape based on server version and channel type. This is a reasonable workaround during the rest-typings migration.
271-277: LGTM!Explicitly converting
nulltoundefinedvia??ensures the payload conforms to expected typing wherejoinCodeshould bestring | undefinedrather thanstring | null.
359-362: LGTM - past review concern addressed.The
userIdparameter has been correctly removed, aligning with the Rocket.Chat API documentation whereusers.getPreferencesreturns preferences for the authenticated user based on headers only.
474-479: Acceptable cast for newer endpoint.The
livechat/config/routingendpoint (RC 7.11.0+) may not be in@rocket.chat/rest-typingsyet. The cast is appropriate for forward compatibility.
600-602: LGTM!Converting the boolean
ignoreto a string aligns with REST API query parameter conventions where boolean values are typically passed as'true'/'false'strings.
658-660: Acceptable migration cast.The
as anycast fornotificationsparameter follows the same pattern. Consider aligningIRoomNotificationswith the rest-typings definition in a follow-up.
697-704: LGTM!The
as anycast for the sort parameter accommodates the MongoDB-style sort format (-1for descending) which may not match the strict typing in rest-typings.
798-810: LGTM!The params object is well-typed locally with explicit optional
texthandling. Theas anycast bridges the gap with rest-typings definitions.
819-827: LGTM!The
triggerIdparameter is now explicitly typed as a requiredstring, improving type safety for slash command execution.
971-992: LGTM!The
editMessagefunction correctly handles both encrypted (content) and non-encrypted (text) message updates. Theas anycasts accommodate the different payload shapes required bychat.update.
994-1012: LGTM!Narrowing
typeto'apn' | 'gcm'union improves type safety by restricting to the only valid push notification types for iOS and Android respectively.
1096-1096: Acceptable cast for missing endpoint definition.The
rooms.inviteendpoint may not be defined in@rocket.chat/rest-typings. The action parameter is well-typed as a union. Consider tracking this for future type alignment.
966-969: Document the minimum server version for this endpoint.The
RC ?comment indicates uncertainty about whene2e.resetRoomKeywas introduced. Unlike other e2e endpoints in this file that document specific versions (RC 0.70.0, 2.2.0, 5.5), this one lacks version information. Investigate the Rocket.Chat server documentation or changelog to identify and document the minimum supported version.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Hello Maintainers |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/lib/methods/getUsersPresence.ts (1)
91-109:⚠️ Potential issue | 🟠 MajorReplace
forEach(async ...)with afor...ofloop to await completion of each database write.Line 91 uses
forEachwith an async callback for database operations, which launches all writes concurrently without awaiting them. This can result in unhandled promise rejections and non-deterministic write ordering.Suggested fix
- users.forEach(async (presenceUser) => { + for (const presenceUser of users) { const user = presenceUser as unknown as IUser; try { const userRecord = await userCollection.find(user._id); await db.write(async () => { await userRecord.update(u => { Object.assign(u, user); }); }); } catch (e) { // User not found await db.write(async () => { await userCollection.create(u => { u._raw = sanitizedRaw({ id: user._id }, userCollection.schema); Object.assign(u, user); }); }); } - }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/getUsersPresence.ts` around lines 91 - 109, The current users.forEach(async (presenceUser) => { ... }) launches async DB operations without awaiting them; replace that forEach with a sequential for...of loop over users (e.g., for (const presenceUser of users) { ... }) and await each async step (userCollection.find(user._id), db.write(...) and userCollection.create(...)) so each write completes before continuing; keep the existing try/catch logic, ensure you await the db.write calls and userRecord.update / userCollection.create invocations, and retain sanitizedRaw(...) for new records.
🧹 Nitpick comments (11)
app/containers/EmojiPicker/index.tsx (1)
42-42: Prefer a typed route shape over an inline cast.
category={route.key as TEmojiCategory}works, but the assertion can hide drift between route keys andTEmojiCategory. Consider typingroutes/renderScenewith a route subtype soroute.keyis alreadyTEmojiCategory.Suggested refactor
+type EmojiRoute = Route & { key: TEmojiCategory; title: TIconsName }; + -const routes = categories.tabs.map(tab => ({ +const routes: EmojiRoute[] = categories.tabs.map(tab => ({ key: tab.category, title: tab.tabLabel, accessibilityLabel: tab.accessibilityLabel })); -const renderScene = ({ route }: { route: Route }) => ( - <EmojiCategory parentWidth={parentWidth} onEmojiSelected={handleEmojiSelect} category={route.key as TEmojiCategory} /> +const renderScene = ({ route }: { route: EmojiRoute }) => ( + <EmojiCategory parentWidth={parentWidth} onEmojiSelected={handleEmojiSelect} category={route.key} /> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/EmojiPicker/index.tsx` at line 42, The code is using an inline cast category={route.key as TEmojiCategory} which can mask mismatches; update the route typing so route.key is already TEmojiCategory instead of casting. Concretely, change the routes/renderScene types (or the navigator route type) to a specific route subtype where key: TEmojiCategory, update the signature used where EmojiCategory is rendered (the renderScene/route type and the routes array), and then remove the inline cast in the EmojiCategory usage so it becomes category={route.key}.app/lib/methods/helpers/findSubscriptionsRooms.ts (1)
63-63: Avoidunknowndouble-cast; type the mapper output directly.Line 63 works, but
as unknown as IServerSubscription[]suppresses compile-time shape checks. Prefer declaringmappedExistingSubsasIServerSubscription[](or a typed mapper function) and concat without unsafe casting.Proposed refactor
- const mappedExistingSubs = existingSubs.map(s => ({ + const mappedExistingSubs: IServerSubscription[] = existingSubs.map(s => ({ _id: s._id, f: s.f, t: s.t, ts: s.ts, @@ avatarETag: s.avatarETag, roles: s.roles })); // Assign - sub = subscriptions.concat(mappedExistingSubs as unknown as IServerSubscription[]); + sub = subscriptions.concat(mappedExistingSubs);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/helpers/findSubscriptionsRooms.ts` at line 63, The concat uses an unsafe double-cast—change the mapper that produces mappedExistingSubs to return IServerSubscription[] (or create a typed mapper function) so you can concat directly into subscriptions without using "as unknown as IServerSubscription[]"; update the declaration/return type of mappedExistingSubs (or the mapper function) in findSubscriptionsRooms.ts and remove the double-cast on the concat line to restore compile-time shape checks.app/views/RoomInfoView/index.tsx (1)
171-173: Consider explicit state typing for stronger type safety.The
as typeof roomUsercast on line 173 relies on type inference from the initial statemember || {}(line 47), which can be overly permissive. While the comment documenting the API shape difference is helpful, an explicit type annotation on theroomUserstate would make the cast target more predictable and catch mismatches at compile time.♻️ Suggested improvement
Define an explicit type for the state:
// Define a type that represents the display-ready user shape type TRoomUser = Partial<IUser & { os?: string; browser?: string }>; const [roomUser, setRoomUser] = useState<TRoomUser>(member || {});Then the cast becomes more explicit and self-documenting:
-setRoomUser({ ...user, roles: r } as typeof roomUser); +setRoomUser({ ...user, roles: r } as TRoomUser);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/RoomInfoView/index.tsx` around lines 171 - 173, Introduce an explicit state type for the display-ready user and use it for roomUser instead of relying on the inferred type; define a type (e.g. TRoomUser) that represents the partial display shape (Partial<IUser & { os?: string; browser?: string }>) and change the useState declaration for roomUser to useState<TRoomUser>(member || {}), then update the setRoomUser call in the code that uses handleRoles to setRoomUser({ ...user, roles: r }) (removing or replacing the current cast to typeof roomUser) so the state and casts are explicit and type-safe.app/lib/methods/getSingleMessage.ts (1)
4-17: Consider simplifying by removing the Promise constructor anti-pattern.Wrapping an
asyncfunction insidenew Promise()is unnecessary sinceasyncfunctions already return a Promise. This can be simplified to a plain async function, improving readability and avoiding potential pitfalls with error handling in async executors.♻️ Proposed refactor
-const getSingleMessage = (messageId: string): Promise<IMessage> => - new Promise(async (resolve, reject) => { - try { - const result = await getSingleMessageService(messageId); - if (result.success && result.message) { - const { message } = result; - // API returns _id; app uses id. Map explicitly. API message may omit some IMessage fields (e.g. dml). - return resolve({ ...message, id: message._id } as IMessage); - } - return reject(); - } catch (e) { - return reject(e); - } - }); +const getSingleMessage = async (messageId: string): Promise<IMessage> => { + const result = await getSingleMessageService(messageId); + if (result.success && result.message) { + const { message } = result; + // API returns _id; app uses id. Map explicitly. API message may omit some IMessage fields (e.g. dml). + return { ...message, id: message._id } as IMessage; + } + throw new Error('Failed to retrieve message'); +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/getSingleMessage.ts` around lines 4 - 17, Replace the Promise-constructor anti-pattern in getSingleMessage by making getSingleMessage an async function that awaits getSingleMessageService(messageId), checks result.success and result.message, returns the mapped IMessage ({ ...message, id: message._id }) when present, and throws an error (or the caught error) when the service indicates failure or an exception occurs so callers receive the rejected promise naturally; keep the mapping of API _id to id and preserve error propagation rather than using new Promise, and reference getSingleMessage and getSingleMessageService when implementing the change.app/lib/services/connect.ts (1)
448-453: Use a stable provider id for the reducer key.Line 453 keys the stored map by
name, butnamecan fall back tobuttonLabelText, which is presentation text. Now that this reduced object is what gets dispatched, keying byserviceis safer and avoids accidental overwrites when labels are customized or duplicated.Suggested fix
const loginServicesReducer = loginServices.reduce<Record<string, LoginServiceEntry>>((ret, item) => { + const key = item.service || item.name || item.buttonLabelText; const name = item.name || item.buttonLabelText || item.service; const authType = determineAuthType(item); if (authType !== 'not_supported') { - ret[name] = { ...item, name, authType }; + ret[key] = { ...item, name, authType }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/services/connect.ts` around lines 448 - 453, The reducer currently keys entries by a possibly-presentational `name` which can collide; update the loginServices.reduce (loginServicesReducer) to use the stable provider id `item.service` as the map key instead of `name` while still populating each value with the derived `name` and `authType` (computed via determineAuthType) and only including items where authType !== 'not_supported' (type LoginServiceEntry).app/lib/methods/getMoreMessages.ts (1)
19-19: Add type guard before castingttoRoomTypes.
SubscriptionTypeincludes'e2e'and'thread'values that are not validRoomTypes('c' | 'd' | 'p' | 'l'). The cast at line 19 bypasses type checking, allowing invalid values to reachloadMessagesForRoom. Add a guard or mapper to ensure only valid room types are passed.Verification details
From
app/definitions/ISubscription.ts:export enum SubscriptionType { GROUP = 'p', DIRECT = 'd', CHANNEL = 'c', OMNICHANNEL = 'l', E2E = 'e2e', // FIXME: this is not a type of subscription THREAD = 'thread' // FIXME: this is not a type of subscription }From
app/lib/methods/roomTypeToApiType.ts:export type RoomTypes = 'c' | 'd' | 'p' | 'l';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/getMoreMessages.ts` at line 19, The code casts t to RoomTypes before validation, which allows invalid SubscriptionType values like 'e2e' or 'thread' to reach loadMessagesForRoom; update getMoreMessages (the variable/parameter t) to validate or map the incoming SubscriptionType to a proper RoomTypes value (e.g., check t is one of 'c'|'d'|'p'|'l' or use a mapper function that converts SubscriptionType enum values to RoomTypes), and only call loadMessagesForRoom with the validated/mapped RoomTypes; if t is not mappable, handle the case (return early or throw) to prevent unsafe casting.app/views/SettingsView/index.tsx (1)
2-2: Replace double-cast navigation with a composed navigation type.Line [143] uses
as unknown as ..., which hides type mismatches. UseCompositeNavigationPropsonavigationis natively compatible with both stack and drawer usage.♻️ Suggested change
-import { useNavigation } from '@react-navigation/native'; +import { useNavigation, type CompositeNavigationProp } from '@react-navigation/native'; ... - const navigation = useNavigation<NativeStackNavigationProp<SettingsStackParamList, 'SettingsView'>>(); + type TSettingsNavigation = CompositeNavigationProp< + NativeStackNavigationProp<SettingsStackParamList, 'SettingsView'>, + DrawerNavigationProp<DrawerParamList> + >; + const navigation = useNavigation<TSettingsNavigation>(); ... - <SidebarView navigation={navigation as unknown as DrawerNavigationProp<DrawerParamList>} /> + <SidebarView navigation={navigation} />Also applies to: 7-8, 30-31, 41-41, 143-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/SettingsView/index.tsx` at line 2, The code is double-casting the navigation from useNavigation (using "as unknown as ...") which hides type issues; replace that with a composed navigation prop type using CompositeNavigationProp so navigation is correctly typed for both stack and drawer. Import CompositeNavigationProp from `@react-navigation/native` and the appropriate StackNavigationProp/DrawerNavigationProp types, define a type like SettingsNavProp = CompositeNavigationProp<StackNavigationProp<YourStackParamList, 'Settings'>, DrawerNavigationProp<YourDrawerParamList>> and then call const navigation = useNavigation<SettingsNavProp>() (update the type names to match your Root/Drawer param list types and the Settings screen name); remove all occurrences of the double-cast (the "as unknown as" usages) and replace them with this typed useNavigation hook.app/containers/MessageComposer/hooks/useAutoSaveDraft.ts (1)
15-16: Use the native timer-handle type instead of a double cast.Line [41] currently bypasses type safety with
as unknown as number. UseReturnType<typeof setInterval>forintervalRefand remove the cast.♻️ Suggested change
- const intervalRef = useRef<number | null>(null); + const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null); ... - intervalRef.current = setInterval(saveMessageDraft, 3000) as unknown as number; + intervalRef.current = setInterval(saveMessageDraft, 3000);Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/MessageComposer/hooks/useAutoSaveDraft.ts` around lines 15 - 16, The ref intervalRef in useAutoSaveDraft is currently typed as number | null and uses a double cast (as unknown as number); change its type to ReturnType<typeof setInterval> | null and update any assignments/clears to remove the cast (e.g., when assigning the result of setInterval and when calling clearInterval) so the native timer handle type is used safely with intervalRef and no unknown/number casts remain.app/views/ThreadMessagesView/index.tsx (1)
345-345: Avoidas anyin thread sync/load payloads.Lines [345] and [369] bypass the new rest-typing guarantees. Prefer a narrow input type for
updateThreads(or a small normalizer) so payload drift is caught by the compiler.Also applies to: 369-369
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/ThreadMessagesView/index.tsx` at line 345, The call sites using "result.threads as any" should be fixed by removing the cast and passing a correctly typed payload to updateThreads: define a narrow input type (e.g., ThreadPayload[] or NormalizedThread[]) for updateThreads's parameter or add a small normalizeThreads(result.threads) function that validates/maps the API shape into that type; replace both usages (the call with result.threads and lastThreadSync) to pass the normalized/typed array and update the updateThreads signature to accept the new type so the compiler catches payload drift.app/views/RoomView/JoinCode.tsx (1)
45-48: Consider tightening thetprop type.The
tprop is typed asstringbut is cast to'c' | 'p'at line 69 when callingjoinRoom. For better type safety, consider updating the interface to use the narrower type directly:Proposed fix
export interface IJoinCodeProps { rid: string; - t: string; + t: 'c' | 'p'; onJoin: Function; isMasterDetail: boolean; theme: TSupportedThemes; }This would eliminate the need for the cast at line 69 and catch type mismatches at compile time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/RoomView/JoinCode.tsx` around lines 45 - 48, Update the IJoinCodeProps interface to narrow the t prop from string to the specific union type used by joinRoom (e.g., 'c' | 'p') so TypeScript enforces correct values at compile time; modify IJoinCodeProps.t to that union (referencing IJoinCodeProps and the t prop) and then remove the unnecessary type cast when calling joinRoom in the JoinCode component to rely on the strengthened prop type.app/lib/services/restApi.ts (1)
492-499: Path interpolation with query string embedded in URL.The query parameter
enabledDepartmentsOnly=trueis embedded directly in the URL string rather than passed as a separate parameter object. While this works, it's less maintainable than using the SDK's query parameter mechanism if available.The type cast to the parameterized path
'livechat/agents/:uid/departments'is a necessary workaround for SDK path matching, as noted in the TODO.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/services/restApi.ts` around lines 492 - 499, The getAgentDepartments function embeds the query string directly in the URL; update getAgentDepartments to call sdk.get with the parameterized path 'livechat/agents/:uid/departments' (as currently cast) and pass enabledDepartmentsOnly: true via the SDK's query/options object instead of appending ?enabledDepartmentsOnly=true to the path; modify the sdk.get invocation in getAgentDepartments to use the parameter object form while keeping the type cast to 'livechat/agents/:uid/departments' to satisfy the SDK path matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/definitions/rest/v1/chat.ts`:
- Around line 8-12: The POST signature currently types the content param as
"string | unknown", which collapses to unknown; replace that union with the
concrete EncryptedContent type so callers get proper type checking. Update the
POST overload that currently lists "{ roomId: string; msgId: string; content:
string | unknown }" to use "{ roomId: string; msgId: string; content:
EncryptedContent }" (Imported from the IMessage definition where
EncryptedContent is declared) and ensure the file imports EncryptedContent from
IMessage.ts so the POST signature references the correct type.
In `@app/definitions/rest/v1/settings.ts`:
- Around line 13-29: The three exported type guards
(isSettingsUpdatePropsActions, isSettingsUpdatePropsColor,
isSettingsUpdatePropDefault) are not null-safe because they use the `in`
operator on a parameter typed as Partial<SettingsUpdateProps>; change each
function signature to accept `unknown` (e.g., props: unknown) and first check
that props is a non-null object (typeof props === 'object' && props !== null)
before using `'execute' in props`, `'editor' in props && 'value' in props`, and
`'value' in props` respectively; keep the same return type predicates (props is
...) so the guard semantics stay the same.
In `@app/lib/methods/getSingleMessage.ts`:
- Line 13: The promise rejection in getSingleMessage (the reject() call) should
include a descriptive Error so callers can inspect the failure; locate the
reject() inside the getSingleMessage function (or its Promise executor) and
replace the empty reject() with reject(new Error("Failed to get single message:
<brief reason>")) or a similarly descriptive Error object that includes context
(e.g., messageId or error details) so callers can debug.
In `@app/lib/services/connect.ts`:
- Around line 443-446: The code assumes loginServicesResult.services is an array
but only uses a TypeScript assertion; change the assignment to first check
Array.isArray(loginServicesResult.services) and only cast/assign to
loginServices when it is an array (otherwise set loginServices to an empty array
or skip the reduce path). Update the block handling loginServicesResult
(referencing loginServicesResult, services, loginServices, IServices, and any
subsequent reduce usage) so reduce is only ever called on a validated array to
avoid runtime errors when settings.oauth returns a truthy non-array.
In `@app/views/RoomActionsView/index.tsx`:
- Around line 203-211: The code currently turns a missing counters.members into
0 and persists it; change both places where you call updateUsersCount (the block
after getRoomCounters and the similar block around updateUsersCount at lines
~247-250) to only call updateUsersCount when counters.members is actually a
numeric value (e.g., typeof counters.members === 'number'); do not coerce
missing/undefined members to 0 for persistence—keep setting
this.state.membersCount to counters.members ?? undefined but only sync to DB via
updateUsersCount(counters.members) when it is a number.
In `@app/views/ThreadMessagesView/index.tsx`:
- Around line 346-350: The pagination end logic in the setState call incorrectly
uses (result.total ?? result.threads.length) < API_FETCH_COUNT causing endless
fetches for multi-page totals; change end to true when the fetched page returned
fewer items than API_FETCH_COUNT OR when a known total has been reached (i.e.,
offset + result.threads.length >= result.total). Update the end expression in
the setState for the component (where loading, end, offset are set) to use
(result.threads.length < API_FETCH_COUNT) || (result.total != null && offset +
result.threads.length >= result.total) so it correctly terminates pagination
after the last page.
---
Outside diff comments:
In `@app/lib/methods/getUsersPresence.ts`:
- Around line 91-109: The current users.forEach(async (presenceUser) => { ... })
launches async DB operations without awaiting them; replace that forEach with a
sequential for...of loop over users (e.g., for (const presenceUser of users) {
... }) and await each async step (userCollection.find(user._id), db.write(...)
and userCollection.create(...)) so each write completes before continuing; keep
the existing try/catch logic, ensure you await the db.write calls and
userRecord.update / userCollection.create invocations, and retain
sanitizedRaw(...) for new records.
---
Nitpick comments:
In `@app/containers/EmojiPicker/index.tsx`:
- Line 42: The code is using an inline cast category={route.key as
TEmojiCategory} which can mask mismatches; update the route typing so route.key
is already TEmojiCategory instead of casting. Concretely, change the
routes/renderScene types (or the navigator route type) to a specific route
subtype where key: TEmojiCategory, update the signature used where EmojiCategory
is rendered (the renderScene/route type and the routes array), and then remove
the inline cast in the EmojiCategory usage so it becomes category={route.key}.
In `@app/containers/MessageComposer/hooks/useAutoSaveDraft.ts`:
- Around line 15-16: The ref intervalRef in useAutoSaveDraft is currently typed
as number | null and uses a double cast (as unknown as number); change its type
to ReturnType<typeof setInterval> | null and update any assignments/clears to
remove the cast (e.g., when assigning the result of setInterval and when calling
clearInterval) so the native timer handle type is used safely with intervalRef
and no unknown/number casts remain.
In `@app/lib/methods/getMoreMessages.ts`:
- Line 19: The code casts t to RoomTypes before validation, which allows invalid
SubscriptionType values like 'e2e' or 'thread' to reach loadMessagesForRoom;
update getMoreMessages (the variable/parameter t) to validate or map the
incoming SubscriptionType to a proper RoomTypes value (e.g., check t is one of
'c'|'d'|'p'|'l' or use a mapper function that converts SubscriptionType enum
values to RoomTypes), and only call loadMessagesForRoom with the
validated/mapped RoomTypes; if t is not mappable, handle the case (return early
or throw) to prevent unsafe casting.
In `@app/lib/methods/getSingleMessage.ts`:
- Around line 4-17: Replace the Promise-constructor anti-pattern in
getSingleMessage by making getSingleMessage an async function that awaits
getSingleMessageService(messageId), checks result.success and result.message,
returns the mapped IMessage ({ ...message, id: message._id }) when present, and
throws an error (or the caught error) when the service indicates failure or an
exception occurs so callers receive the rejected promise naturally; keep the
mapping of API _id to id and preserve error propagation rather than using new
Promise, and reference getSingleMessage and getSingleMessageService when
implementing the change.
In `@app/lib/methods/helpers/findSubscriptionsRooms.ts`:
- Line 63: The concat uses an unsafe double-cast—change the mapper that produces
mappedExistingSubs to return IServerSubscription[] (or create a typed mapper
function) so you can concat directly into subscriptions without using "as
unknown as IServerSubscription[]"; update the declaration/return type of
mappedExistingSubs (or the mapper function) in findSubscriptionsRooms.ts and
remove the double-cast on the concat line to restore compile-time shape checks.
In `@app/lib/services/connect.ts`:
- Around line 448-453: The reducer currently keys entries by a
possibly-presentational `name` which can collide; update the
loginServices.reduce (loginServicesReducer) to use the stable provider id
`item.service` as the map key instead of `name` while still populating each
value with the derived `name` and `authType` (computed via determineAuthType)
and only including items where authType !== 'not_supported' (type
LoginServiceEntry).
In `@app/lib/services/restApi.ts`:
- Around line 492-499: The getAgentDepartments function embeds the query string
directly in the URL; update getAgentDepartments to call sdk.get with the
parameterized path 'livechat/agents/:uid/departments' (as currently cast) and
pass enabledDepartmentsOnly: true via the SDK's query/options object instead of
appending ?enabledDepartmentsOnly=true to the path; modify the sdk.get
invocation in getAgentDepartments to use the parameter object form while keeping
the type cast to 'livechat/agents/:uid/departments' to satisfy the SDK path
matching.
In `@app/views/RoomInfoView/index.tsx`:
- Around line 171-173: Introduce an explicit state type for the display-ready
user and use it for roomUser instead of relying on the inferred type; define a
type (e.g. TRoomUser) that represents the partial display shape (Partial<IUser &
{ os?: string; browser?: string }>) and change the useState declaration for
roomUser to useState<TRoomUser>(member || {}), then update the setRoomUser call
in the code that uses handleRoles to setRoomUser({ ...user, roles: r })
(removing or replacing the current cast to typeof roomUser) so the state and
casts are explicit and type-safe.
In `@app/views/RoomView/JoinCode.tsx`:
- Around line 45-48: Update the IJoinCodeProps interface to narrow the t prop
from string to the specific union type used by joinRoom (e.g., 'c' | 'p') so
TypeScript enforces correct values at compile time; modify IJoinCodeProps.t to
that union (referencing IJoinCodeProps and the t prop) and then remove the
unnecessary type cast when calling joinRoom in the JoinCode component to rely on
the strengthened prop type.
In `@app/views/SettingsView/index.tsx`:
- Line 2: The code is double-casting the navigation from useNavigation (using
"as unknown as ...") which hides type issues; replace that with a composed
navigation prop type using CompositeNavigationProp so navigation is correctly
typed for both stack and drawer. Import CompositeNavigationProp from
`@react-navigation/native` and the appropriate
StackNavigationProp/DrawerNavigationProp types, define a type like
SettingsNavProp =
CompositeNavigationProp<StackNavigationProp<YourStackParamList, 'Settings'>,
DrawerNavigationProp<YourDrawerParamList>> and then call const navigation =
useNavigation<SettingsNavProp>() (update the type names to match your
Root/Drawer param list types and the Settings screen name); remove all
occurrences of the double-cast (the "as unknown as" usages) and replace them
with this typed useNavigation hook.
In `@app/views/ThreadMessagesView/index.tsx`:
- Line 345: The call sites using "result.threads as any" should be fixed by
removing the cast and passing a correctly typed payload to updateThreads: define
a narrow input type (e.g., ThreadPayload[] or NormalizedThread[]) for
updateThreads's parameter or add a small normalizeThreads(result.threads)
function that validates/maps the API shape into that type; replace both usages
(the call with result.threads and lastThreadSync) to pass the normalized/typed
array and update the updateThreads signature to accept the new type so the
compiler catches payload drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 416017f1-8335-4780-8f90-b3cd6c3bd5b8
📒 Files selected for processing (49)
app/containers/ActionSheet/ActionSheetContentWithInputAndSubmit/index.tsxapp/containers/EmojiPicker/index.tsxapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/MessageComposer/hooks/useIOSBackSwipeHandler.tsapp/definitions/rest/adaptEndpoints.tsapp/definitions/rest/v1/autotranslate.tsapp/definitions/rest/v1/channels.tsapp/definitions/rest/v1/chat.tsapp/definitions/rest/v1/directory.tsapp/definitions/rest/v1/dm.tsapp/definitions/rest/v1/e2e.tsapp/definitions/rest/v1/emojiCustom.tsapp/definitions/rest/v1/groups.tsapp/definitions/rest/v1/im.tsapp/definitions/rest/v1/index.tsapp/definitions/rest/v1/invites.tsapp/definitions/rest/v1/moderation.tsapp/definitions/rest/v1/omnichannel.tsapp/definitions/rest/v1/permissions.tsapp/definitions/rest/v1/push.tsapp/definitions/rest/v1/rooms.tsapp/definitions/rest/v1/settings.tsapp/definitions/rest/v1/teams.tsapp/definitions/rest/v1/users.tsapp/lib/encryption/helpers/mapMessageFromApi.tsapp/lib/encryption/helpers/mapMessageFromDB.tsapp/lib/encryption/room.tsapp/lib/methods/getMoreMessages.tsapp/lib/methods/getPermissions.tsapp/lib/methods/getSingleMessage.tsapp/lib/methods/getUsersPresence.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/methods/helpers/goRoom.tsapp/lib/methods/helpers/parseUrls.tsapp/lib/methods/isRoomFederated.tsapp/lib/methods/updateMessages.tsapp/lib/services/connect.tsapp/lib/services/restApi.tsapp/views/RoomActionsView/index.tsxapp/views/RoomInfoView/Livechat.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomMembersView/helpers.tsapp/views/RoomMembersView/index.tsxapp/views/RoomView/JoinCode.tsxapp/views/RoomView/RightButtons.tsxapp/views/RoomView/index.tsxapp/views/SettingsView/index.tsxapp/views/TeamChannelsView.tsxapp/views/ThreadMessagesView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- app/definitions/rest/v1/invites.ts
- app/views/RoomView/RightButtons.tsx
- app/definitions/rest/v1/emojiCustom.ts
- app/views/RoomInfoView/Livechat.tsx
- app/definitions/rest/v1/moderation.ts
- app/definitions/rest/v1/dm.ts
- app/views/TeamChannelsView.tsx
- app/definitions/rest/v1/permissions.ts
- app/definitions/rest/v1/users.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/views/SettingsView/index.tsx
🔇 Additional comments (29)
app/lib/methods/getPermissions.ts (1)
78-78: Good removal of unsafe cast on permission query.Line 78 keeps the same filtering behavior while removing the
as unknown as string[]type escape hatch, which makes this query safer and clearer.app/lib/methods/isRoomFederated.ts (1)
15-16: Nice type-safe guard cleanup.Replacing the casted access with
room.federatedkeeps behavior intact and improves type-safety/readability of the predicate.app/views/RoomMembersView/helpers.ts (1)
40-40: Good simplification with no behavior change.Passing
roomTypedirectly togetRoomRolesremoves an unnecessary local variable and keeps the same control flow/error handling.app/views/RoomMembersView/index.tsx (1)
155-155: Good type-safety improvement (any→TRoomType).This is a clean tightening of the call-site contract and aligns with the REST typings migration goal.
app/lib/methods/updateMessages.ts (1)
38-39: Good improvement replacinganywith an explicit model cast.Using
TSubscriptionModelhere (Line 38-39) makes the fallback intent clearer and aligns with the PR’s typing-tightening goal while preserving existing behavior.app/lib/methods/helpers/goRoom.ts (1)
40-45: Nice type narrowing for current route access.This keeps behavior unchanged while removing untyped route access and making the
RoomViewguard safer.app/lib/encryption/room.ts (1)
324-333: LGTM! Good defensive coding improvements.The changes properly:
- Add explicit typing for
usersSuggestedGroupKeysreplacing implicitany[]- Extract
publicKeyto avoid repeated property access- Add guard clauses for missing
publicKey,user._id, and falsykeyvaluesThe validation on lines 329 and 331 correctly handles edge cases where
user.e2e?.public_keyis missing orencryptRoomKeyForUserreturnsundefined.app/containers/EmojiPicker/index.tsx (1)
12-12: Nice typing improvement on interfaces import.Pulling
TEmojiCategorydirectly into this module keeps the picker’s category contract explicit and consistent with the rest of this typing-focused PR.app/containers/MessageComposer/hooks/useIOSBackSwipeHandler.ts (2)
16-20: Cleaner transition event payload typing with unchanged behavior.Nice improvement: the optional
data.closingshape keeps the listener defensive while preserving existing back-swipe detection logic.
26-28: Good removal of unnecessary cast on blur listener.Using
'blur'directly simplifies the listener registration and keeps the keyboard-dismiss behavior intact.app/lib/encryption/helpers/mapMessageFromApi.ts (1)
9-12: LGTM!Removing the
as anycast improves type safety. With the migration to@rocket.chat/rest-typings, the attachment types should now be properly defined, making this cast unnecessary.app/lib/encryption/helpers/mapMessageFromDB.ts (1)
11-14: LGTM!Consistent with the parallel change in
mapMessageFromApi.ts. Removing theas anycast leverages the improved type definitions from@rocket.chat/rest-typings, providing better type safety without changing runtime behavior.app/containers/ActionSheet/ActionSheetContentWithInputAndSubmit/index.tsx (1)
2-2: LGTM! Clean type safety improvements.Good refactoring:
- Using
type TextInputimport for type-only usage- Properly typed
createRef<TextInput>()instead of implicit any- Optional chaining (
?.current?.focus()) is safer than a hard cast- Removing
as anyon theinputRefprop improves type checkingAlso applies to: 110-110, 129-129, 137-137
app/lib/methods/getMoreMessages.ts (1)
5-5: Type-only import is a solid change.Line 5 correctly keeps
RoomTypesas a compile-time-only dependency, avoiding runtime coupling.app/definitions/rest/v1/im.ts (1)
1-5: LGTM: clean migration to canonical REST typings.This keeps IM endpoint typing aligned with the shared
@rocket.chat/rest-typingssource and reduces local duplication.app/definitions/rest/v1/push.ts (1)
5-11: LGTM: endpoint extraction is precise and maintainable.Filtering push endpoints from
Endpointsand then adapting keeps this file minimal while still strongly typed.app/definitions/rest/adaptEndpoints.ts (1)
5-9: LGTM: solid shared utility for endpoint key normalization.This is a clean type-level abstraction and a good foundation for the rest-typings migration.
app/definitions/rest/v1/rooms.ts (1)
5-9: UseOmitbefore overriding'rooms.invite'to avoid accidental intersection types.The current type composition intersects two definitions for the same key instead of replacing it. While this pattern is used consistently across similar endpoint files (channels, omnichannel, settings, e2e, chat, users), using
Omitprovides explicit intent and guards against future upstream type changes that could create unusable merged signatures.🐛 Suggested fix
-export type RoomsEndpoints = AdaptEndpoints<RestTypingsRoomsEndpoints> & { +export type RoomsEndpoints = Omit<AdaptEndpoints<RestTypingsRoomsEndpoints>, 'rooms.invite'> & { 'rooms.invite': { POST: (params: { roomId: string; action: 'accept' | 'reject' }) => void; }; };app/definitions/rest/v1/groups.ts (1)
1-5: Clean migration to rest-typings.The file correctly follows the established pattern for adapting external REST typings. The
AdaptEndpointsutility will strip the/v1/prefix from endpoint keys, making them compatible with the app's SDK calls.app/definitions/rest/v1/e2e.ts (1)
5-12: Endpoint override pattern is correct.The intersection type correctly extends the adapted rest-typings with app-specific endpoint definitions. The
e2e.resetRoomKeyandusers.resetE2EKeyendpoints are added as overrides, presumably because they're either missing from or differ from the upstream package.Note:
users.resetE2EKeyis a users-domain endpoint defined in the e2e file. This is acceptable since it's E2E-related, but consider documenting this cross-domain placement with a comment for future maintainers.app/definitions/rest/v1/teams.ts (1)
1-5: Clean migration to rest-typings.The file correctly follows the established pattern. No overrides needed for teams endpoints.
app/lib/methods/helpers/parseUrls.ts (1)
9-21: Clean refactor of URL parsing logic.The consolidation into a single object literal with spread is cleaner than incremental property assignment. The image computation logic correctly applies
buildImageURLonly when an image source exists, and the fallback chain for title/description is preserved.app/definitions/rest/v1/omnichannel.ts (1)
5-23: Omnichannel endpoint overrides are well-structured.The custom endpoint definitions align with their usage in
restApi.ts. Thelivechat/config/routingresponse shape with optionalconfigcorrectly matches the conditional access pattern at line 479 ofrestApi.ts.app/lib/services/restApi.ts (3)
438-448: Well-designed discriminated union for department info.The
IDepartmentInfoResulttype with its success discriminant enables type-safe extraction viagetDepartmentFromInfo. This pattern cleanly handles API responses that may fail.
1095-1102: Guard for undefined user ID correctly implemented.The added check for
!idprevents API calls when the user is not logged in, addressing the previously identified concern. The call tousers.removeOtherTokensnow correctly passes no parameters, matching the API specification.
660-661: Type casts documented with TODO comments.The
as anycasts forsaveUserProfileandsaveNotificationSettingsare documented with TODO comments explaining the mismatch between local types and rest-typings. This is acceptable for the migration phase, but these should be tracked for future resolution.app/views/RoomView/index.tsx (2)
1120-1120: Type cast for room type parameter.The cast
this.t as 'c' | 'p'narrows the optional string type to the union expected byjoinRoom. This is safe in context sincejoinRoom()is only called when the room exists and has a valid type. The same pattern is used inJoinCode.tsx.
1432-1432: User prop type narrowing for Message component.The explicit type cast ensures the user object matches the Message component's expected prop interface. This is a safe narrowing since
getUserSelectorprovides these fields.app/definitions/rest/v1/index.ts (1)
13-37: Nice cleanup of the aggregate endpoint map.Replacing
OauthCustomConfigurationwithSettingsEndpointskeepsEndpointsas a pure intersection of endpoint surfaces, which matches the new rest-typings layout cleanly.
deepak0x
force-pushed
the
refactor/rest-typings-setup_and_helpers_migration
branch
4 times, most recently
from
6e99b35 to
2c0d4ae
Compare
Replace locally defined REST endpoint types with @rocket.chat/rest-typings v6.13.1 package types. Uses AdaptEndpoints utility to map /v1/ prefixed paths from rest-typings to unprefixed paths used by the app. - Add @rocket.chat/core-typings and @rocket.chat/rest-typings 6.13.1 - Delete local PaginatedRequest and PaginatedResult in favor of package exports - Migrate all 23 endpoint files to use rest-typings via AdaptEndpoints - Add shared RemoveV1Prefix and AdaptEndpoints utilities in adaptEndpoints.ts - Extend helpers/index.ts with rest-typings compatible type utilities - Override endpoints where local behavior differs from rest-typings - Remove as-any casts and use proper type assertions
deepak0x
force-pushed
the
refactor/rest-typings-setup_and_helpers_migration
branch
from
2c0d4ae to
fe10775
Compare
2 participants
Proposed changes
This PR refactors the app to use @rocket.chat/rest-typings as the single source of truth for REST API type definitions.
Issue(s)
Closes #6232
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
Refactor
Bug Fixes
New Features
Chores