chore: migrate MessagesView to Hooks#5922
Open
OtavioStasiak wants to merge 14 commits into
Open
Conversation
OtavioStasiak
force-pushed
the
chore-migrate-to-hooks-messages-view
branch
3 times, most recently
from
16cd401 to
48f50a6
Compare
OtavioStasiak
force-pushed
the
chore-migrate-to-hooks-messages-view
branch
from
48f50a6 to
11f7388
Compare
diegolmello
requested changes
Nov 11, 2024
…ooks-messages-view Port MessagesView Services calls to restApi (getFiles/getMessages/togglePinMessage/toggleStarMessage) and carry the lib/hooks barrel removal and React-default-import fixes so the merged tree builds, type-checks and lints clean. Claude-Session: https://claude.ai/code/session_01Tn4LYpJLScM6Uht4Jsf4eq
Replace the six screenName switch helpers (getContentTestId, getListEmptyMessage, getActionTitle, getActionIcon, fetchMessages, performMessageAction) and the inline renderItem branches with a single messagesViewContent map keyed by screen name. useMessages now receives a fetchMessages closure instead of resolving the screen itself. Claude-Session: https://claude.ai/code/session_01Tn4LYpJLScM6Uht4Jsf4eq
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
WalkthroughMessagesView now uses typed route params, a paginated message-loading hook, view-specific fetch and action handlers, and a functional screen component wired to hooks, navigation helpers, and list rendering. ChangesMessagesView refactor
Sequence Diagram(s)sequenceDiagram
participant MessagesView
participant useMessages
participant messagesViewContent
participant getMessages
participant getFiles
participant Encryption
participant toggleStarMessage
participant togglePinMessage
MessagesView->>useMessages: useMessages({ fetchMessages })
useMessages->>messagesViewContent: fetch({ rid, t, userId, offset })
alt Files view
messagesViewContent->>getFiles: getFiles({ rid, t, userId, offset })
getFiles-->>messagesViewContent: files
messagesViewContent->>Encryption: decryptFiles(files)
Encryption-->>messagesViewContent: decrypted files
else Mentions/Starred/Pinned views
messagesViewContent->>getMessages: getMessages({ rid, t, userId, offset })
getMessages-->>messagesViewContent: messages + total
end
messagesViewContent-->>useMessages: items + total
useMessages-->>MessagesView: messages + loading
MessagesView->>messagesViewContent: action.press(message)
alt Starred view
messagesViewContent->>toggleStarMessage: toggleStarMessage(messageId, starred)
else Pinned view
messagesViewContent->>togglePinMessage: togglePinMessage(messageId, pinned)
end
MessagesView->>useMessages: updateMessageOnActionPress(messageId)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
android_build
GitHub Actions
Error
![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: 3
🤖 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/MessagesView/hooks/useMessages.tsx`:
- Around line 10-12: The initial loading state in useMessages allows
MessagesView to briefly treat the data as empty before load() runs. Update the
hook’s initial state so the first render is considered loading, or add a
hasLoadedOnce guard around the loading/messaging state in useMessages and the
empty-state check in MessagesView, using the existing loading, messages, and
setLoading flow to prevent the "No_*" placeholder from flashing.
- Around line 14-39: The pagination loader in useMessages has a re-entry race
because load relies on the loading state and a stale messages snapshot, so
FlatList.onEndReached can fire duplicate requests with the same offset. Add an
immediate lock with a useRef inside useMessages/load to prevent concurrent
execution before state updates land, and switch the setMessages append logic to
a functional update (or otherwise derive the offset from the latest state) so
fetchMessages always appends to the current list without duplicates or lost
items.
In `@app/views/MessagesView/messagesViewContent.ts`:
- Around line 34-65: The messages view registry is too loosely typed, allowing
invalid route names to resolve to undefined and crash when accessing content
fields. Update messagesViewContent to use a literal union of the supported keys
and change IParams.name (and any related route param typing in
MessagesViewContent) to that same union so only 'Files', 'Mentions', 'Starred',
and 'Pinned' are accepted at compile time. Ensure the content lookup in
MessagesViewContent still uses messagesViewContent by the typed name so invalid
navigations are rejected before runtime.
🪄 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: 20cda458-c933-47db-b896-762e7155aa1e
📒 Files selected for processing (5)
app/views/MessagesView/definitions.tsapp/views/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/index.tsxapp/views/MessagesView/messagesViewContent.tsapp/views/MessagesView/methods/getFileUrlAndTypeFromMessage.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: format
🧰 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/MessagesView/definitions.tsapp/views/MessagesView/messagesViewContent.tsapp/views/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/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 numbersUse TypeScript with strict mode enabled
Files:
app/views/MessagesView/definitions.tsapp/views/MessagesView/messagesViewContent.tsapp/views/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/index.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/views/MessagesView/definitions.tsapp/views/MessagesView/messagesViewContent.tsapp/views/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/index.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/views/MessagesView/definitions.tsapp/views/MessagesView/messagesViewContent.tsapp/views/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/index.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/MessagesView/definitions.tsapp/views/MessagesView/messagesViewContent.tsapp/views/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/index.tsx
🧠 Learnings (3)
📚 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/MessagesView/definitions.tsapp/views/MessagesView/messagesViewContent.tsapp/views/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/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/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/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/MessagesView/hooks/useMessages.tsxapp/views/MessagesView/index.tsx
Address CodeRabbit review on PR 5922: - useMessages: init loading=true and gate re-entry with a ref so the empty-state placeholder no longer flashes before the first fetch and onEndReached can't fire concurrent same-offset requests; append via a functional state update to avoid stale-closure loss. - messagesViewContent: type the registry keys with a TMessagesViewScreen union and guard MessagesView against an unknown screen name resolving to undefined. Claude-Session: https://claude.ai/code/session_01Tn4LYpJLScM6Uht4Jsf4eq
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
diegolmello
requested a deployment
to
android_build
GitHub Actions
Waiting
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
2 participants
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
migrate messagesView from class components to hooks.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-17
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit