refactor: migrate RoomView to a function component#7482
Open
diegolmello wants to merge 35 commits into
Open
Conversation
The other items scoped to NATIVE-1354 (dead 'reply' action-union member, console.count, phantom allow-list entries) were already removed by the NATIVE-22 stack this branches from; only the duplicate 'status' key in roomAttrsUpdate remained.
Define Subscribed Room, Preview Mode, and Invited with detection rules, and split Positional state into Jump orchestration (RoomView, NATIVE-34) and Scroll and highlight execution (List, NATIVE-39).
Extract RoomView's subscription/room data seam into a standalone hook: merge findAndObserveRoom + observeRoom + observeSubscriptions into one column-scoped observeWithColumns stream that handles subscribed, preview and appears-later transitions, plus init/getRoomMember and retry. Add roomAttrsUpdateColumns (model-prop to snake_case DB column) so the observable is scoped to the same fields shouldComponentUpdate tracks; a drift-guard test and 'as const satisfies' keep the two in lockstep. Hook is unit-tested in isolation; wiring into the component is deferred to NATIVE-1357.
Extract RoomView's jump-orchestration seam into a standalone hook: emit the loading event, fetch the target, resolve the anchor/window, and drive the List imperative handle (isMessageInWindow, jumpToMessage, cancelJumpToMessage). Navigation stays the component's concern via injected navToRoom/navToThread callbacks; scroll and highlight remain the List component's responsibility (NATIVE-39). Hook is unit-tested in isolation; wiring into the component is deferred to NATIVE-1357.
…1356) observeWithColumns re-emits the same cached WatermelonDB model instance mutated in place, so setState with that reference is an Object.is no-op and a tracked-column change (topic, archived, autoTranslate...) never re-rendered. Rebuild a fresh roomUpdate snapshot object per emission, matching the class's observeRoom trigger, with a regression test that re-emits the same instance. Also restore the getRoomMember error log and guard the retry timer against a post-unmount schedule.
Port the class setHeader logic into a standalone useHeader hook modelled on RoomsListView/hooks/useHeader. The header re-fires on roomUpdate snapshots since the WatermelonDB room model mutates in place and keeps a stable ref. NATIVE-1361
Drop shouldComponentUpdate (stale-props fix), move class lifecycle to targeted effects, and wire useRoomSubscription, useJumpToMessage and useHeader. Expose setJoined/setLastOpen from useRoomSubscription for the handleSendMessage and onJoin write-sites.
RoomView now passes referentially stable handlers, so the migration-time identity-change warning and its FROZEN_KEYS list are dead weight. The capture-once/reactive-tail store design is unchanged.
Contributor
|
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:
WalkthroughRoomView is migrated from a class component to hooks and Zustand-backed room/composer stores. Navigation, message actions, lifecycle behavior, headers, jumping, rendering, and tests are extracted into focused modules. ChangesRoom subscription state
Composer state
Room navigation and actions
Room lifecycle
Functional RoomView
Memo and store-capture updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/views/RoomView/hooks/useRoomSubscription.ts`:
- Line 166: The `useRoomSubscription` error handling is inconsistent because
`readMessages` failures are being sent to `console.log` instead of the shared
`log` reporter. Update the `readMessages(currentRoom.rid, newLastOpen, true)`
catch path in `useRoomSubscription` to use the imported `log` function, matching
the existing logging pattern already used elsewhere in the hook and keeping
errors in the crash-reporting pipeline.
- Around line 116-133: The getRoomMember callback in useRoomSubscription can
pass a null or undefined value from getUidDirectMessage directly into
getUserInfo, which can trigger a malformed request. Add a falsy guard
immediately after calling getUidDirectMessage(currentRoom) and before
setRoomUserId/getUserInfo so the function returns early when nextRoomUserId is
missing. Keep the fix within getRoomMember and preserve the existing mountedRef
and log handling for the valid-user path.
- Around line 178-185: The bare catch in useRoomSubscription’s init/reconnect
flow drops the failure details, so update the catch block to capture and log the
thrown error before scheduling the retry. While touching initRef.current and
retryTimeoutRef logic, add a bounded retry strategy for the setTimeout loop in
useRoomSubscription: track attempts (or similar) and stop retrying after a
reasonable cap, and/or increase delay with backoff instead of retrying every
RETRY_DELAY indefinitely.
In `@app/views/RoomView/index.tsx`:
- Around line 210-279: `navToRoom` and `navToThread` need explicit async error
handling so rejected `getRoomInfo`, `getThreadById`, or `getThreadName` calls do
not become unhandled rejections. Wrap the bodies of both `useCallback` handlers
in `try/catch` (and `finally` for `navToThread`) and ensure `sendLoadingEvent({
visible: false })` is always called when an error occurs or before returning
early. Keep the fixes localized to `navToRoom`, `navToThread`, and their
loading-state logic so callers like the debounce path and `useJumpToMessage` can
remain unchanged.
- Around line 626-636: handleSendMessage currently fires sendMessage(...).then()
without error handling, so convert it to an async function and use await with
try/catch. Keep the existing side effects tied to success by moving
setLastOpen(null) and Review.pushPositiveEvent() into the success path, and make
sure resetAction() runs in a finally block only if it should always execute. Use
the handleSendMessage symbol to update the callback and explicitly handle any
sendMessage rejection to avoid unhandled promise rejections.
- Around line 873-903: The permission lookup flow in RoomView can still break
when hasPermission resolves undefined, causing getCanForwardGuest and
getCanViewCannedResponse to throw on permissions[0]. Update hasPermission so its
failure path returns a boolean array fallback (false for each requested
permission), or add a defensive check in those two helpers before indexing, so
updateOmnichannel can always complete without rejecting from permission lookup
errors.
- Around line 1110-1120: Move the `hapticFeedback(item.id)` call out of
`renderItem` in `RoomView` so the row renderer stays pure. Keep `renderItem`
limited to computing `content`, and trigger the feedback side effect from a
`useEffect` or similar handler keyed off the relevant message/item state, still
using `hapticFeedback` and `removeInAppFeedback` logic but not during render.
- Around line 568-587: The updateUnreadCount callback in RoomView is creating an
observer asynchronously without guarding against unmount, so if the await
completes after cleanup the subscription can be leaked. Add a cancelled/mounted
flag inside updateUnreadCount and use it to skip subscription setup when the
component has already been cleaned up; if the observable is created after
cancellation, unsubscribe it immediately before returning. Make sure
queryUnreadsRef.current and its unsubscribe path still handle the normal active
case.
🪄 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: bfb44624-e5a1-428b-a6ba-ae8c467ec033
📒 Files selected for processing (11)
CONTEXT.mdapp/containers/message/stores/MessageRoomStore.tsxapp/containers/message/stores/__tests__/MessageRoomStore.test.tsxapp/views/RoomView/constants.tsapp/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useHeader.tsxapp/views/RoomView/hooks/useJumpToMessage.test.tsxapp/views/RoomView/hooks/useJumpToMessage.tsapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useRoomSubscription.tsapp/views/RoomView/index.tsx
💤 Files with no reviewable changes (1)
- app/containers/message/stores/tests/MessageRoomStore.test.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/views/RoomView/hooks/useJumpToMessage.tsapp/views/RoomView/constants.tsapp/views/RoomView/hooks/useHeader.tsxapp/containers/message/stores/MessageRoomStore.tsxapp/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useRoomSubscription.tsapp/views/RoomView/hooks/useJumpToMessage.test.tsxapp/views/RoomView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
**/*.{ts,tsx}: Use TypeScript in strict mode, with imports resolved from theapp/baseUrl
Follow the repository's TypeScript/React Native code style enforced by Prettier: tabs, single quotes, 130-character width, no trailing commas, omit arrow-function parens when possible, and keep brackets on the same line
Files:
app/views/RoomView/hooks/useJumpToMessage.tsapp/views/RoomView/constants.tsapp/views/RoomView/hooks/useHeader.tsxapp/containers/message/stores/MessageRoomStore.tsxapp/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useRoomSubscription.tsapp/views/RoomView/hooks/useJumpToMessage.test.tsxapp/views/RoomView/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use the project's ESLint setup (
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins) for code quality and style compliance
Files:
app/views/RoomView/hooks/useJumpToMessage.tsapp/views/RoomView/constants.tsapp/views/RoomView/hooks/useHeader.tsxapp/containers/message/stores/MessageRoomStore.tsxapp/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useRoomSubscription.tsapp/views/RoomView/hooks/useJumpToMessage.test.tsxapp/views/RoomView/index.tsx
app/views/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components under
app/views/
Files:
app/views/RoomView/hooks/useHeader.tsxapp/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useJumpToMessage.test.tsxapp/views/RoomView/index.tsx
app/containers/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components under
app/containers/
Files:
app/containers/message/stores/MessageRoomStore.tsx
🧠 Learnings (4)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/views/RoomView/hooks/useJumpToMessage.tsapp/views/RoomView/constants.tsapp/views/RoomView/hooks/useHeader.tsxapp/containers/message/stores/MessageRoomStore.tsxapp/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useRoomSubscription.tsapp/views/RoomView/hooks/useJumpToMessage.test.tsxapp/views/RoomView/index.tsx
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/RoomView/hooks/useHeader.tsxapp/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useJumpToMessage.test.tsxapp/views/RoomView/index.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/views/RoomView/hooks/useHeader.tsxapp/containers/message/stores/MessageRoomStore.tsxapp/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useJumpToMessage.test.tsxapp/views/RoomView/index.tsx
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/views/RoomView/hooks/useHeader.test.tsxapp/views/RoomView/hooks/useRoomSubscription.test.tsxapp/views/RoomView/hooks/useJumpToMessage.test.tsx
🔇 Additional comments (14)
app/containers/message/stores/MessageRoomStore.tsx (1)
1-1: LGTM!Also applies to: 63-65
app/views/RoomView/constants.ts (1)
45-46: LGTM!Also applies to: 48-79
app/views/RoomView/hooks/useRoomSubscription.test.tsx (1)
79-88: LGTM!Also applies to: 90-103, 105-118, 120-132, 134-150, 152-174, 176-181, 183-194, 196-214, 216-235, 237-251, 253-264, 266-268, 270-297
CONTEXT.md (1)
5-19: LGTM!Also applies to: 114-119
app/views/RoomView/hooks/useRoomSubscription.ts (1)
186-197: 🎯 Functional CorrectnessNo issue:
onThreadMessagesLoadedis already memoized in the parent. This re-initialization concern doesn’t apply here.> Likely an incorrect or invalid review comment.app/views/RoomView/hooks/useHeader.tsx (3)
1-65: LGTM!
66-96: LGTM!Also applies to: 149-196
97-148: 🎯 Functional CorrectnessNo issue: the
ISubscriptioncast is benignisInviteSubscription()only checksstatusandinviter, and the downstream header props already accept optionalabacAttributes/disableNotifications, so the thin-room fallback does not create unsafe behavior.> Likely an incorrect or invalid review comment.app/views/RoomView/hooks/useHeader.test.tsx (1)
1-96: LGTM!app/views/RoomView/hooks/useJumpToMessage.ts (2)
1-38: LGTM!
40-107: LGTM!Navigation-vs-anchor branching logic (
shouldNavigateToRoom, thread/main-room special case, in-window anchoring) is correctly implemented and matches the accompanying test suite's expectations.app/views/RoomView/hooks/useJumpToMessage.test.tsx (1)
1-206: LGTM!Good coverage of in-window/out-of-window anchoring, cancellation, missing-message, cross-room/thread navigation, and the reply-specific error path.
app/views/RoomView/index.tsx (2)
1-101: LGTM!The remaining hook wiring, message-action/reaction/quote/edit flows, join/resume/thread-name handlers, effect-based header/E2EE/read-only sync, and the render tree (footer/actions/providers/list composition) look consistent with the class-to-hooks migration and preserve prior behavior.
Also applies to: 280-567, 638-872, 905-999, 1007-1109, 1121-1341
1000-1006: 🎯 Functional CorrectnessroomAttrsUpdate already includes
visitor,status, andlastMessage> Likely an incorrect or invalid review comment.
Comment on lines
+116
to
+133
| const getRoomMember = useCallback(async () => { | ||
| const currentRoom = roomRef.current; | ||
| if ('id' in currentRoom && currentRoom.t === 'd' && !isGroupChat(currentRoom)) { | ||
| try { | ||
| const nextRoomUserId = getUidDirectMessage(currentRoom); | ||
| if (mountedRef.current) { | ||
| setRoomUserId(nextRoomUserId); | ||
| } | ||
| const result = await getUserInfo(nextRoomUserId); | ||
| if (result.success) { | ||
| return result.user; | ||
| } | ||
| } catch (e) { | ||
| log(e); | ||
| } | ||
| } | ||
| return {}; | ||
| }, []); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against null/undefined nextRoomUserId before calling getUserInfo.
getUidDirectMessage can return null or undefined (e.g., when room.uids is absent and the legacy path doesn't apply). The value is passed directly to getUserInfo(userId: string), which would send a malformed API request. Add a falsy guard after the getUidDirectMessage call.
🛡️ Proposed guard
const nextRoomUserId = getUidDirectMessage(currentRoom);
+ if (!nextRoomUserId) {
+ return {};
+ }
if (mountedRef.current) {
setRoomUserId(nextRoomUserId);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const getRoomMember = useCallback(async () => { | |
| const currentRoom = roomRef.current; | |
| if ('id' in currentRoom && currentRoom.t === 'd' && !isGroupChat(currentRoom)) { | |
| try { | |
| const nextRoomUserId = getUidDirectMessage(currentRoom); | |
| if (mountedRef.current) { | |
| setRoomUserId(nextRoomUserId); | |
| } | |
| const result = await getUserInfo(nextRoomUserId); | |
| if (result.success) { | |
| return result.user; | |
| } | |
| } catch (e) { | |
| log(e); | |
| } | |
| } | |
| return {}; | |
| }, []); | |
| const getRoomMember = useCallback(async () => { | |
| const currentRoom = roomRef.current; | |
| if ('id' in currentRoom && currentRoom.t === 'd' && !isGroupChat(currentRoom)) { | |
| try { | |
| const nextRoomUserId = getUidDirectMessage(currentRoom); | |
| if (!nextRoomUserId) { | |
| return {}; | |
| } | |
| if (mountedRef.current) { | |
| setRoomUserId(nextRoomUserId); | |
| } | |
| const result = await getUserInfo(nextRoomUserId); | |
| if (result.success) { | |
| return result.user; | |
| } | |
| } catch (e) { | |
| log(e); | |
| } | |
| } | |
| return {}; | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/views/RoomView/hooks/useRoomSubscription.ts` around lines 116 - 133, The
getRoomMember callback in useRoomSubscription can pass a null or undefined value
from getUidDirectMessage directly into getUserInfo, which can trigger a
malformed request. Add a falsy guard immediately after calling
getUidDirectMessage(currentRoom) and before setRoomUserId/getUserInfo so the
function returns early when nextRoomUserId is missing. Keep the fix within
getRoomMember and preserve the existing mountedRef and log handling for the
valid-user path.
| if (mountedRef.current) { | ||
| setLastOpen(currentRoom.alert || currentRoom.unread || currentRoom.userMentions ? currentRoom.ls : null); | ||
| } | ||
| readMessages(currentRoom.rid, newLastOpen, true).catch(e => console.log(e)); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use log(e) instead of console.log(e) for error reporting consistency.
The file imports log (line 9) and uses it on line 129, but readMessages errors are logged via console.log, which won't reach the app's crash-reporting pipeline.
🔧 Proposed fix
- readMessages(currentRoom.rid, newLastOpen, true).catch(e => console.log(e));
+ readMessages(currentRoom.rid, newLastOpen, true).catch(e => log(e));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| readMessages(currentRoom.rid, newLastOpen, true).catch(e => console.log(e)); | |
| readMessages(currentRoom.rid, newLastOpen, true).catch(e => log(e)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/views/RoomView/hooks/useRoomSubscription.ts` at line 166, The
`useRoomSubscription` error handling is inconsistent because `readMessages`
failures are being sent to `console.log` instead of the shared `log` reporter.
Update the `readMessages(currentRoom.rid, newLastOpen, true)` catch path in
`useRoomSubscription` to use the imported `log` function, matching the existing
logging pattern already used elsewhere in the hook and keeping errors in the
crash-reporting pipeline.
Comment on lines
+178
to
+185
| } catch { | ||
| if (mountedRef.current) { | ||
| setLoading(false); | ||
| retryTimeoutRef.current = setTimeout(() => { | ||
| initRef.current(); | ||
| }, RETRY_DELAY); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Log errors and consider bounding the retry loop.
The bare catch discards the error entirely — there's no way to diagnose init failures from logs. Additionally, the 300ms fixed-delay retry has no max-attempt cap or backoff, so a persistent failure (e.g., server down) produces unbounded network calls every 300ms until the user navigates away.
🔒 Proposed fix — add error logging
- } catch {
+ } catch (e) {
+ log(e);
if (mountedRef.current) {
setLoading(false);
retryTimeoutRef.current = setTimeout(() => {
initRef.current();
}, RETRY_DELAY);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch { | |
| if (mountedRef.current) { | |
| setLoading(false); | |
| retryTimeoutRef.current = setTimeout(() => { | |
| initRef.current(); | |
| }, RETRY_DELAY); | |
| } | |
| } | |
| } catch (e) { | |
| log(e); | |
| if (mountedRef.current) { | |
| setLoading(false); | |
| retryTimeoutRef.current = setTimeout(() => { | |
| initRef.current(); | |
| }, RETRY_DELAY); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/views/RoomView/hooks/useRoomSubscription.ts` around lines 178 - 185, The
bare catch in useRoomSubscription’s init/reconnect flow drops the failure
details, so update the catch block to capture and log the thrown error before
scheduling the retry. While touching initRef.current and retryTimeoutRef logic,
add a bounded retry strategy for the setTimeout loop in useRoomSubscription:
track attempts (or similar) and stop retrying after a reasonable cap, and/or
increase delay with backoff instead of retrying every RETRY_DELAY indefinitely.
Comment on lines
+568
to
+587
| const updateUnreadCount = useCallback(async () => { | ||
| if (!rid) { | ||
| return; | ||
| } | ||
| const db = database.active; | ||
| const observable = await db | ||
| .get('subscriptions') | ||
| .query(Q.where('archived', false), Q.where('open', true), Q.where('rid', Q.notEq(this.rid))) | ||
| .query(Q.where('archived', false), Q.where('open', true), Q.where('rid', Q.notEq(rid))) | ||
| .observeWithColumns(['unread']); | ||
|
|
||
| this.queryUnreads = observable.subscribe(rooms => { | ||
| queryUnreadsRef.current = observable.subscribe(rooms => { | ||
| const unreadsCount = rooms.reduce( | ||
| (unreadCount, room) => (room.unread > 0 && !room.hideUnreadStatus ? unreadCount + room.unread : unreadCount), | ||
| (unreadCount, item) => (item.unread > 0 && !item.hideUnreadStatus ? unreadCount + item.unread : unreadCount), | ||
| 0 | ||
| ); | ||
| if (this.state.unreadsCount !== unreadsCount) { | ||
| this.setState({ unreadsCount }, this.setHeader); | ||
| if (unreadsCountRef.current !== unreadsCount) { | ||
| setState({ unreadsCount }); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| onThreadPress = debounce((item: TAnyMessageModel) => this.navToThread(item), 1000, true); | ||
|
|
||
| shouldNavigateToRoom = (message: TGetMessageInfoResult) => { | ||
| if (message.tmid && message.tmid === this.tmid) { | ||
| return false; | ||
| } | ||
| if (!message.tmid && message.rid === this.rid) { | ||
| return false; | ||
| } | ||
| return true; | ||
| }; | ||
|
|
||
| jumpToMessageByUrl = async (messageUrl?: string, isFromReply?: boolean) => { | ||
| if (!messageUrl) { | ||
| return; | ||
| } | ||
| try { | ||
| const parsedUrl = parse(messageUrl, true); | ||
| const messageId = parsedUrl.query.msg; | ||
| if (messageId) { | ||
| await this.jumpToMessage(messageId, isFromReply); | ||
| } | ||
| } catch (e) { | ||
| log(e); | ||
| } | ||
| }; | ||
|
|
||
| // Fire a jump from a Navigation param, then consume the one-shot param so re-selecting the SAME | ||
| // message id reads as a change (undefined -> id edge) and re-fires, instead of matching a stale | ||
| // param and no-opping. Both mount (initial param) and update (Search delivers via setParams) use this. | ||
| consumeJumpParam = (messageId: string) => { | ||
| this.jumpToMessageId = undefined; | ||
| this.jumpToMessage(messageId); | ||
| this.props.navigation.setParams({ jumpToMessageId: undefined }); | ||
| }; | ||
| }, [rid]); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section of RoomView
FILE="app/views/RoomView/index.tsx"
wc -l "$FILE"
sed -n '520,640p' "$FILE"
# Find the effect that calls updateUnreadCount and its cleanup
rg -n "updateUnreadCount|queryUnreadsRef|InteractionManager|unsubscribe|subscription" "$FILE" -n -C 5
# Show any related ref initialization near the top of the file
sed -n '1,140p' "$FILE"Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 15482
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/views/RoomView/index.tsx"
# Inspect the full mount/cleanup effect around updateUnreadCount
sed -n '920,972p' "$FILE"
# Search for any other cleanup or reassignment of queryUnreadsRef.current
rg -n "queryUnreadsRef\\.current|updateUnreadCount\\(" "$FILE" -n -C 3
# Check whether observeWithColumns is treated as async elsewhere in the repo
rg -n "observeWithColumns\\(" app -n -C 2Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 5766
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the sibling subscription hook for its async lifecycle handling
FILE="app/views/RoomView/hooks/useRoomSubscription.ts"
wc -l "$FILE"
sed -n '1,180p' "$FILE"
# Inspect the async subscription pattern in SelectedUsersView for comparison
FILE2="app/views/SelectedUsersView/index.tsx"
sed -n '80,130p' "$FILE2"Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 7291
Guard the async subscription init against unmount app/views/RoomView/index.tsx:568-587 — if updateUnreadCount() resolves after cleanup, the subscription is created too late for queryUnreadsRef.current.unsubscribe() to catch it, leaving a live observer behind. Add a cancelled/mounted guard and tear it down immediately when the promise resolves.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/views/RoomView/index.tsx` around lines 568 - 587, The updateUnreadCount
callback in RoomView is creating an observer asynchronously without guarding
against unmount, so if the await completes after cleanup the subscription can be
leaked. Add a cancelled/mounted flag inside updateUnreadCount and use it to skip
subscription setup when the component has already been cleaned up; if the
observable is created after cancellation, unsubscribe it immediately before
returning. Make sure queryUnreadsRef.current and its unsubscribe path still
handle the normal active case.
Close the React Compiler annotation-mode gaps: add 'use memo' and remove the useCallback/useMemo/React.memo it obviates across RoomView, message, and MessageComposer hooks and components. useMessages is excluded: its Anchored-Window/Gap state machine relies on precise useLayoutEffect dependency control that 'use memo' does not preserve (Gap-release regression in useMessages.test.tsx), so it keeps its manual memoization.
Replace the per-instance useRoomSubscription hook with a module-level, refcounted zustand RoomStore registry keyed by rid. A room and its thread screen (same rid) now share one store and one DB subscription; the store tears down when the last screen releases. The store owns init() plus the join()/markMessageSent() intents (replacing setJoined/setLastOpen), self-hydrates from the subscription observable (sub-backed / preview / auto-flip), and drops the 300ms init retry timer. RoomView consumes it via zustand selectors and provides it through RoomStoreContext for descendants. Behavior-neutral.
Dissolve RoomView's RoomContext (plain React Context, 12 values, fanned out to every consumer on any change) into a per-instance zustand store mirroring MessageRoomStore: createStore-once + effect sync + per-field named hooks. MessageComposer-tree consumers now select their slice and re-render only when it changes. RoomProviders keeps its external prop API so RoomView and ShareView are untouched.
Move the message-action handler cluster (edit/quote/react/reply/long-press/ attachment/error actions, setQuotesAndText/getText) into a compiled useMessageActions hook. Behavior-neutral; RoomView wires the returned handlers into the same providers.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/views/RoomView/stores/RoomStore.ts (1)
101-101: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog errors consistently in
initinstead of swallowing them.The
catchblock on line 109 silently discards the error without logging, and line 101 usesconsole.loginstead of the already-importedlogfunction. Ifinitfails in production, there is no diagnostic trail.♻️ Proposed fix
- readMessages(currentRoom.rid, newLastOpen, true).catch(e => console.log(e)); + readMessages(currentRoom.rid, newLastOpen, true).catch(e => log(e));- } catch { + } catch (e) { + log(e); set({ loading: false }); }Also applies to: 109-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/views/RoomView/stores/RoomStore.ts` at line 101, Replace the console.log catch handler in init with the imported log function, and add consistent error logging to the catch block around the init operation at line 109. Preserve the existing error details and include clear context identifying the failed initialization or readMessages operation.Source: Learnings
app/views/RoomView/stores/ComposerStore.tsx (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winapp/views/RoomView/stores/ComposerStore.tsx:14: Type
onSendMessageas(message: string, tshow?: boolean) => void.handleSendMessagealready uses that shape, soFunctionadds no safety here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/views/RoomView/stores/ComposerStore.tsx` at line 14, Replace the broad Function type on ComposerStore’s onSendMessage property with (message: string, tshow?: boolean) => void, matching the signature used by handleSendMessage.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/views/RoomView/hooks/useRoomLifecycle.ts`:
- Around line 152-159: Add a rejection handler to the promise returned by
sendMessage in handleSendMessage, chaining .catch(...) after the existing
.then(...) to handle failures from sending or its success callback without
leaving an unhandled rejection.
- Around line 224-231: Update the InteractionManager callback in
useRoomLifecycle to handle the asynchronous result of sub?.subscribe?.(): attach
a catch handler that passes rejected errors to log(e), while preserving the
existing synchronous try/catch behavior.
In `@app/views/RoomView/stores/RoomStore.ts`:
- Around line 46-48: Guard the result of getUidDirectMessage in the RoomStore
update flow before calling getUserInfo: after assigning nextRoomUserId, return
or skip the lookup when it is null or undefined, while preserving the roomUserId
state update as appropriate. Use the getUidDirectMessage and getUserInfo symbols
to locate the change and ensure getUserInfo is only called with a confirmed
string.
---
Nitpick comments:
In `@app/views/RoomView/stores/ComposerStore.tsx`:
- Line 14: Replace the broad Function type on ComposerStore’s onSendMessage
property with (message: string, tshow?: boolean) => void, matching the signature
used by handleSendMessage.
In `@app/views/RoomView/stores/RoomStore.ts`:
- Line 101: Replace the console.log catch handler in init with the imported log
function, and add consistent error logging to the catch block around the init
operation at line 109. Preserve the existing error details and include clear
context identifying the failed initialization or readMessages operation.
🪄 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: 4d4031d4-62f2-4aa6-9aec-cd08aa64434a
⛔ Files ignored due to path filters (1)
app/containers/MessageComposer/__snapshots__/MessageComposer.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (36)
app/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/Autocomplete/Autocomplete.tsxapp/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/containers/MessageComposer/components/Buttons/MicOrSendButton.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/containers/MessageComposer/components/Toolbar/Default.tsxapp/containers/MessageComposer/components/Unfocused/Left.tsxapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/containers/message/hooks/useImageDescriptionLabel.tsapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/views/ForwardMessageView/index.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/List/components/EmptyRoom.tsxapp/views/RoomView/List/components/List.tsxapp/views/RoomView/List/components/NavBottomFAB.tsxapp/views/RoomView/List/hooks/useScroll.tsapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/RoomProviders.tsxapp/views/RoomView/context.tsapp/views/RoomView/hooks/useJumpToMessage.tsapp/views/RoomView/hooks/useMessageActions.test.tsxapp/views/RoomView/hooks/useMessageActions.tsxapp/views/RoomView/hooks/useRoomLifecycle.tsapp/views/RoomView/hooks/useRoomNavigation.tsapp/views/RoomView/index.tsxapp/views/RoomView/stores/ComposerStore.tsxapp/views/RoomView/stores/RoomStore.test.tsapp/views/RoomView/stores/RoomStore.tsapp/views/RoomView/stores/RoomStoreContext.tsx
💤 Files with no reviewable changes (1)
- app/views/RoomView/context.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/views/RoomView/hooks/useJumpToMessage.ts
- app/views/RoomView/index.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/views/RoomView/stores/RoomStoreContext.tsxapp/containers/MessageComposer/components/Toolbar/Default.tsxapp/containers/MessageComposer/components/Unfocused/Left.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/hooks/useImageDescriptionLabel.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/RoomView/List/components/List.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/views/ForwardMessageView/index.tsxapp/containers/MessageComposer/components/Autocomplete/Autocomplete.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/components/Buttons/MicOrSendButton.tsxapp/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/views/RoomView/stores/ComposerStore.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/views/RoomView/List/components/NavBottomFAB.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/RoomProviders.tsxapp/views/RoomView/List/components/EmptyRoom.tsxapp/views/RoomView/hooks/useRoomNavigation.tsapp/views/RoomView/stores/RoomStore.test.tsapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/hooks/useMessageActions.test.tsxapp/views/RoomView/hooks/useRoomLifecycle.tsapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/views/RoomView/stores/RoomStore.tsapp/views/RoomView/List/hooks/useScroll.tsapp/views/RoomView/hooks/useMessageActions.tsxapp/containers/MessageComposer/components/ComposerInput.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
**/*.{ts,tsx}: Use TypeScript in strict mode, with imports resolved from theapp/baseUrl
Follow the repository's TypeScript/React Native code style enforced by Prettier: tabs, single quotes, 130-character width, no trailing commas, omit arrow-function parens when possible, and keep brackets on the same line
Files:
app/views/RoomView/stores/RoomStoreContext.tsxapp/containers/MessageComposer/components/Toolbar/Default.tsxapp/containers/MessageComposer/components/Unfocused/Left.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/hooks/useImageDescriptionLabel.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/RoomView/List/components/List.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/views/ForwardMessageView/index.tsxapp/containers/MessageComposer/components/Autocomplete/Autocomplete.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/components/Buttons/MicOrSendButton.tsxapp/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/views/RoomView/stores/ComposerStore.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/views/RoomView/List/components/NavBottomFAB.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/RoomProviders.tsxapp/views/RoomView/List/components/EmptyRoom.tsxapp/views/RoomView/hooks/useRoomNavigation.tsapp/views/RoomView/stores/RoomStore.test.tsapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/hooks/useMessageActions.test.tsxapp/views/RoomView/hooks/useRoomLifecycle.tsapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/views/RoomView/stores/RoomStore.tsapp/views/RoomView/List/hooks/useScroll.tsapp/views/RoomView/hooks/useMessageActions.tsxapp/containers/MessageComposer/components/ComposerInput.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use the project's ESLint setup (
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins) for code quality and style compliance
Files:
app/views/RoomView/stores/RoomStoreContext.tsxapp/containers/MessageComposer/components/Toolbar/Default.tsxapp/containers/MessageComposer/components/Unfocused/Left.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/hooks/useImageDescriptionLabel.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/RoomView/List/components/List.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/views/ForwardMessageView/index.tsxapp/containers/MessageComposer/components/Autocomplete/Autocomplete.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/components/Buttons/MicOrSendButton.tsxapp/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/views/RoomView/stores/ComposerStore.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/views/RoomView/List/components/NavBottomFAB.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/RoomProviders.tsxapp/views/RoomView/List/components/EmptyRoom.tsxapp/views/RoomView/hooks/useRoomNavigation.tsapp/views/RoomView/stores/RoomStore.test.tsapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/hooks/useMessageActions.test.tsxapp/views/RoomView/hooks/useRoomLifecycle.tsapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/views/RoomView/stores/RoomStore.tsapp/views/RoomView/List/hooks/useScroll.tsapp/views/RoomView/hooks/useMessageActions.tsxapp/containers/MessageComposer/components/ComposerInput.tsx
app/views/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components under
app/views/
Files:
app/views/RoomView/stores/RoomStoreContext.tsxapp/views/RoomView/List/components/List.tsxapp/views/ForwardMessageView/index.tsxapp/views/RoomView/stores/ComposerStore.tsxapp/views/RoomView/List/components/NavBottomFAB.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/RoomProviders.tsxapp/views/RoomView/List/components/EmptyRoom.tsxapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/hooks/useMessageActions.test.tsxapp/views/RoomView/hooks/useMessageActions.tsx
app/containers/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components under
app/containers/
Files:
app/containers/MessageComposer/components/Toolbar/Default.tsxapp/containers/MessageComposer/components/Unfocused/Left.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/containers/MessageComposer/components/Autocomplete/Autocomplete.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/components/Buttons/MicOrSendButton.tsxapp/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/containers/MessageComposer/components/ComposerInput.tsx
🧠 Learnings (4)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/views/RoomView/stores/RoomStoreContext.tsxapp/containers/MessageComposer/components/Toolbar/Default.tsxapp/containers/MessageComposer/components/Unfocused/Left.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/hooks/useImageDescriptionLabel.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/RoomView/List/components/List.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/views/ForwardMessageView/index.tsxapp/containers/MessageComposer/components/Autocomplete/Autocomplete.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/components/Buttons/MicOrSendButton.tsxapp/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/views/RoomView/stores/ComposerStore.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/views/RoomView/List/components/NavBottomFAB.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/RoomProviders.tsxapp/views/RoomView/List/components/EmptyRoom.tsxapp/views/RoomView/hooks/useRoomNavigation.tsapp/views/RoomView/stores/RoomStore.test.tsapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/hooks/useMessageActions.test.tsxapp/views/RoomView/hooks/useRoomLifecycle.tsapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/views/RoomView/stores/RoomStore.tsapp/views/RoomView/List/hooks/useScroll.tsapp/views/RoomView/hooks/useMessageActions.tsxapp/containers/MessageComposer/components/ComposerInput.tsx
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/RoomView/stores/RoomStoreContext.tsxapp/views/RoomView/List/components/List.tsxapp/views/ForwardMessageView/index.tsxapp/views/RoomView/stores/ComposerStore.tsxapp/views/RoomView/List/components/NavBottomFAB.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/RoomProviders.tsxapp/views/RoomView/List/components/EmptyRoom.tsxapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/hooks/useMessageActions.test.tsxapp/views/RoomView/hooks/useMessageActions.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/views/RoomView/stores/RoomStoreContext.tsxapp/containers/MessageComposer/components/Toolbar/Default.tsxapp/containers/MessageComposer/components/Unfocused/Left.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/RoomView/List/components/List.tsxapp/views/ForwardMessageView/index.tsxapp/containers/MessageComposer/components/Autocomplete/Autocomplete.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/components/Buttons/MicOrSendButton.tsxapp/containers/MessageComposer/components/Buttons/ActionsButton.tsxapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/views/RoomView/stores/ComposerStore.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/views/RoomView/List/components/NavBottomFAB.tsxapp/views/RoomInfoView/index.tsxapp/views/RoomView/RoomProviders.tsxapp/views/RoomView/List/components/EmptyRoom.tsxapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/hooks/useMessageActions.test.tsxapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/views/RoomView/hooks/useMessageActions.tsxapp/containers/MessageComposer/components/ComposerInput.tsx
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/views/RoomView/stores/RoomStore.test.tsapp/views/RoomView/RoomProviders.test.tsxapp/views/RoomView/hooks/useMessageActions.test.tsx
🔇 Additional comments (33)
app/views/RoomView/hooks/useRoomNavigation.ts (1)
81-330: LGTM!app/views/RoomView/hooks/useMessageActions.tsx (1)
83-267: LGTM!app/views/RoomView/hooks/useMessageActions.test.tsx (1)
1-438: LGTM!app/views/RoomView/hooks/useRoomLifecycle.ts (1)
268-322: LGTM!app/views/RoomView/stores/RoomStoreContext.tsx (1)
1-14: LGTM!app/views/RoomView/stores/RoomStore.test.ts (1)
1-283: LGTM!app/containers/message/hooks/useImageDescriptionLabel.ts (1)
6-6: LGTM!app/containers/message/hooks/useMessageAccessibilityLabel.ts (1)
35-35: LGTM!app/views/ForwardMessageView/index.tsx (1)
23-23: LGTM!app/views/RoomInfoView/index.tsx (1)
43-43: LGTM!app/views/RoomView/List/components/EmptyRoom.tsx (1)
13-21: LGTM!app/views/RoomView/List/components/NavBottomFAB.tsx (1)
1-1: LGTM!Also applies to: 29-31, 52-52
app/views/RoomView/List/hooks/useScroll.ts (2)
87-164: LGTM!
50-50: 🩺 Stability & AvailabilityNo issue with
'use memo'here
The[messages]layout effects remain explicit, so the jump/release flow still re-runs on re-observe as intended.app/views/RoomView/stores/ComposerStore.tsx (1)
1-13: LGTM!Also applies to: 15-89
app/views/RoomView/RoomProviders.tsx (1)
3-6: LGTM!Also applies to: 32-47
app/views/RoomView/RoomProviders.test.tsx (1)
4-26: LGTM!Also applies to: 27-50
app/containers/MessageComposer/MessageComposer.test.tsx (1)
15-15: LGTM!Also applies to: 125-138
app/containers/MessageComposer/components/Buttons/MicOrSendButton.tsx (1)
10-19: LGTM!app/containers/MessageComposer/components/SendThreadToChannel.tsx (1)
7-7: LGTM!Also applies to: 24-24
app/containers/MessageComposer/hooks/useAutoSaveDraft.ts (1)
5-7: LGTM!Also applies to: 9-14
app/containers/MessageComposer/MessageComposer.tsx (1)
7-14: LGTM!Also applies to: 65-70
app/containers/MessageComposer/components/Autocomplete/Autocomplete.tsx (1)
11-11: LGTM!Also applies to: 25-26
app/containers/MessageComposer/components/Buttons/ActionsButton.tsx (1)
12-19: LGTM!app/containers/MessageComposer/components/CancelEdit.tsx (1)
2-9: LGTM!app/containers/MessageComposer/components/ComposerInput.tsx (1)
1-1: LGTM!Also applies to: 35-41, 57-140, 141-169, 171-236, 237-317, 318-375, 376-404
app/containers/MessageComposer/components/Quotes/Quote.tsx (1)
6-6: LGTM!Also applies to: 18-18
app/containers/MessageComposer/components/RecordAudio/RecordAudio.tsx (1)
19-19: LGTM!Also applies to: 33-34
app/containers/MessageComposer/components/Toolbar/Default.tsx (1)
8-14: LGTM!app/containers/MessageComposer/components/Unfocused/Left.tsx (1)
8-13: LGTM!app/containers/MessageComposer/hooks/useChooseMedia.test.tsx (1)
17-19: LGTM!Also applies to: 53-54, 76-77
app/containers/MessageComposer/hooks/useChooseMedia.ts (1)
12-12: LGTM!Also applies to: 35-36
app/views/RoomView/List/components/List.tsx (1)
15-15: LGTM!Also applies to: 28-28
Comment on lines
+152
to
+159
| const handleSendMessage = (message: string, tshow?: boolean) => { | ||
| logEvent(events.ROOM_SEND_MESSAGE); | ||
| sendMessage(rid as string, message, tmid, userRef.current, tshow).then(() => { | ||
| roomStore.getState().markMessageSent(); | ||
| Review.pushPositiveEvent(); | ||
| }); | ||
| resetAction(); | ||
| }; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Attach .catch to the sendMessage promise chain.
The .then callback (markMessageSent, Review.pushPositiveEvent) has no rejection handler, so any error there becomes an unhandled rejection.
🛡️ Proposed fix
- sendMessage(rid as string, message, tmid, userRef.current, tshow).then(() => {
- roomStore.getState().markMessageSent();
- Review.pushPositiveEvent();
- });
+ sendMessage(rid as string, message, tmid, userRef.current, tshow)
+ .then(() => {
+ roomStore.getState().markMessageSent();
+ Review.pushPositiveEvent();
+ })
+ .catch(log);Based on learnings, floating promises should be handled explicitly with .catch(...).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleSendMessage = (message: string, tshow?: boolean) => { | |
| logEvent(events.ROOM_SEND_MESSAGE); | |
| sendMessage(rid as string, message, tmid, userRef.current, tshow).then(() => { | |
| roomStore.getState().markMessageSent(); | |
| Review.pushPositiveEvent(); | |
| }); | |
| resetAction(); | |
| }; | |
| const handleSendMessage = (message: string, tshow?: boolean) => { | |
| logEvent(events.ROOM_SEND_MESSAGE); | |
| sendMessage(rid as string, message, tmid, userRef.current, tshow) | |
| .then(() => { | |
| roomStore.getState().markMessageSent(); | |
| Review.pushPositiveEvent(); | |
| }) | |
| .catch(log); | |
| resetAction(); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/views/RoomView/hooks/useRoomLifecycle.ts` around lines 152 - 159, Add a
rejection handler to the promise returned by sendMessage in handleSendMessage,
chaining .catch(...) after the existing .then(...) to handle failures from
sending or its success callback without leaving an unhandled rejection.
Source: Learnings
Comment on lines
+224
to
+231
| const didMountInteraction = InteractionManager.runAfterInteractions(() => { | ||
| if (rid) { | ||
| try { | ||
| sub?.subscribe?.(); | ||
| } catch (e) { | ||
| log(e); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'app/views/RoomView/hooks/useRoomLifecycle.ts' 'app/views/RoomView/hooks/*' 'app/views/RoomView/**' | sed -n '1,200p'
echo '--- outline ---'
ast-grep outline app/views/RoomView/hooks/useRoomLifecycle.ts --view expanded || true
echo '--- relevant slices ---'
nl -ba app/views/RoomView/hooks/useRoomLifecycle.ts | sed -n '130,280p'
echo '--- search subscribe/sendMessage usage ---'
rg -n "subscribe\\?|sendMessage\\(|markMessageSent|Review\\.pushPositiveEvent|InteractionManager\\.runAfterInteractions|catch\\(" app/views/RoomView/hooks/useRoomLifecycle.ts app/views/RoomView -g '*.ts' -g '*.tsx'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 4367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- useRoomLifecycle.ts around relevant lines ---'
sed -n '130,280p' app/views/RoomView/hooks/useRoomLifecycle.ts | cat -n
echo '--- search for subscribe implementation and type ---'
rg -n "class .*subscribe|subscribe\\s*\\(|interface .*subscribe|type .*subscribe|subscribe\\?:|subscribe:" app -g '*.ts' -g '*.tsx'
echo '--- search for sendMessage implementation and return type ---'
rg -n "sendMessage\\s*\\(" app -g '*.ts' -g '*.tsx'
echo '--- RoomClass definition ---'
rg -n "export (class|interface|type) RoomClass|class RoomClass" app -g '*.ts' -g '*.tsx'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 15745
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- sendMessage implementation ---'
sed -n '1,220p' app/lib/methods/sendMessage.ts | cat -n
echo '--- room subscription implementation ---'
sed -n '1,220p' app/lib/methods/subscriptions/room.ts | cat -n
echo '--- subscription interface ---'
sed -n '90,130p' app/definitions/ISubscription.ts | cat -nRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 17649
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '220,360p' app/lib/methods/sendMessage.ts | cat -nRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 529
Handle the subscribe() promise here
sub?.subscribe?.() is async, so the surrounding try/catch only covers synchronous throws; a rejected subscription will escape. Attach a .catch(...) to the returned promise or await it inside an async callback so failures are logged.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/views/RoomView/hooks/useRoomLifecycle.ts` around lines 224 - 231, Update
the InteractionManager callback in useRoomLifecycle to handle the asynchronous
result of sub?.subscribe?.(): attach a catch handler that passes rejected errors
to log(e), while preserving the existing synchronous try/catch behavior.
Source: Learnings
Comment on lines
+46
to
+48
| const nextRoomUserId = getUidDirectMessage(currentRoom); | ||
| set({ roomUserId: nextRoomUserId }); | ||
| const result = await getUserInfo(nextRoomUserId); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Guard against null/undefined from getUidDirectMessage before calling getUserInfo.
getUidDirectMessage can return null (when !room or isGroupChat) or undefined (when uids is empty and me is undefined), but getUserInfo expects string. Although the surrounding if guard makes the null paths unlikely at runtime, TypeScript cannot narrow based on !isGroupChat(currentRoom), so this is both a type-safety gap and a runtime hazard — getUserInfo(undefined) would fire a wasteful API call and produce a confusing error log.
🛡️ Proposed fix
const nextRoomUserId = getUidDirectMessage(currentRoom);
set({ roomUserId: nextRoomUserId });
- const result = await getUserInfo(nextRoomUserId);
- if (result.success) {
- return result.user;
+ if (nextRoomUserId) {
+ const result = await getUserInfo(nextRoomUserId);
+ if (result.success) {
+ return result.user;
+ }
}#!/bin/bash
# Verify the return type of getUidDirectMessage to confirm the null/undefined type mismatch.
rg -n -A 20 'export function getUidDirectMessage' app/lib/methods/helpers/helpers.ts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/views/RoomView/stores/RoomStore.ts` around lines 46 - 48, Guard the
result of getUidDirectMessage in the RoomStore update flow before calling
getUserInfo: after assigning nextRoomUserId, return or skip the lookup when it
is null or undefined, while preserving the roomUserId state update as
appropriate. Use the getUidDirectMessage and getUserInfo symbols to locate the
change and ensure getUserInfo is only called with a confirmed string.
Remove the non-serializable room model object from the RoomView route and all reads of it. Callers keep threading identity/display scalars (rid, t, name, fname, prid, visitor, roomUserId); RoomView builds initialRoom from those and self-hydrates via the RoomStore subscription observer. Kills react-navigation's non-serializable-params warning and lets deeper screens take simple params. Drops now-dead room-object plumbing in MessagesView and SearchMessagesView (incl. a getRoomInfo fetch that only fed the param).
useHeader stops being a 21-param passthrough. It now self-sources navigation/route (useNavigation/useRoute), layout and redux data (useMasterDetail, baseUrl, user), and RoomStore fields via selectors off a passed roomStore handle. Only genuine callbacks plus the reducer-owned unreadsCount and E2EE flags remain as params.
goRoom acquires the rid's RoomStore at press time so its DB observer hydrates during the nav transition and RoomView mounts against a warm store. A grace release via InteractionManager tears the store back down if navigation was cancelled and no RoomView claimed it. RoomStore is lazy-required inside the warm-up guard: goRoom is a low-level helper imported broadly, RoomStore pulls the view/encryption graph, so it loads only when a warm-up actually runs.
…tive-34-roomview-hooks # Conflicts: # CONTEXT.md # app/views/SearchMessagesView/index.tsx
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
android_build
GitHub Actions
Error
…d contract surface
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
android_build
GitHub Actions
Error
diegolmello
requested a deployment
to
upload_android
GitHub Actions
Waiting
|
Android Build Available Rocket.Chat 4.75.0.109341 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNR4PPhHhHdmDXJS791Zk4fxmM-g5O9fKedE0xnfI_g4AMisl-Kc4S1ZhdqKVu3KvorSmNQlaAamgRxy0_Ij |
|
iOS Build Available Rocket.Chat 4.75.0.109342 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Migrates
app/views/RoomViewfrom a 1726-line class component to a function component, moving its responsibilities into focused, unit-tested hooks, presentational components, and per-room stores. Behavior-preserving — no user-visible change intended.Highlights:
RoomViewto a function component and dropsshouldComponentUpdate, removing a stale-props hazard the class guarded manually. Lifecycle work moves to targeted effects;connect(mapStateToProps)and all existing HOCs are preserved.RoomStoreregistry (stores/RoomStore.ts): the store self-hydrates from the database, is shared by a room and its thread screens, and is torn down when the last consumer releases it.goRoomwarms the store at navigation time so the view mounts against an already-hydrated store.RoomContextwith a per-instance composer Zustand store (stores/ComposerStore.tsx), scoping composer state to a single room instance.useHeader,useJumpToMessage,useMessageActions,useOmnichannelPermissions,useRoomLifecycle,useRoomNavigation— and presentational components —MessageRow,RoomFooter,RoomMessageActions, and the room-state screensEncryptedRoom,InvitedRoom,MissingRoomE2EEKey.RoomViewand the extracted units with'use memo'so the React Compiler owns memoization; no manualuseCallback/useMemoremains in the orchestrator.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-34
How to test or reproduce
Open and use a room end to end — send/edit/quote/react to messages, work with composer drafts and autocomplete, follow threads, jump to a linked message, join a not-subscribed room, and open room actions from the header. Exercise both phone and tablet (master-detail) layouts, and an omnichannel/livechat room. Everything should behave exactly as on the base branch.
Screenshots
Types of changes
Checklist
Further comments
Stacked on
native-22-message-hooks(PR #7455) and targets it until NATIVE-22 lands ondevelop, after which this will be rebased and retargeted todevelop.Summary by CodeRabbit