refactor: Message component#7455
Conversation
…state vocabulary Rename UBIQUITOUS_LANGUAGE.md to CONTEXT.md (history preserved) and update its three references in CLAUDE.md, the VoIP docs README, and the RoomView architecture doc. Add the "Message Interaction & Position State" section distinguishing Interaction state (selection plus the reply/quote/edit/react action, owned by the per-Room interaction store) from Positional state (highlight plus jump/scroll position, owned by the Room view scroll machinery), since the two are migrated independently. Ref: NATIVE-1345 Claude-Session: https://claude.ai/code/session_01Cj1jHhEbiC9mWGe8dpqQeS
Switch every @json-decorated WatermelonDB model field to the memoized form
({ memo: true }) so an unchanged nested JSON value keeps its reference identity
across reads, enabling per-field re-render bailout once consumers adopt it.
The memo cache is invalidated only when the raw column string changes, so
in-place mutation of a memoized value would silently corrupt it. Convert the
two remaining in-place mutations to replacement:
- ReactionsList sorts a copy ([...reactions].sort) instead of the field.
- encryption quote-attachment decryption reassigns message.attachments instead
of pushing into it.
Ref: NATIVE-1346
Claude-Session: https://claude.ai/code/session_01Cj1jHhEbiC9mWGe8dpqQeS
Introduce useMessage, a reactive hook that takes a live WatermelonDB Message model and returns a plain Message Snapshot via useSyncExternalStore. It subscribes to model mutations, paints synchronously on first render with no flash, and reads the memoized @JSON getters directly so unchanged fields keep their reference for per-field re-render bailout. All WatermelonDB specifics are confined to the hook; it is not yet wired into the row container.
Convert MessageContainer from a class to a function component. - Replace manual experimentalSubscribe + forceUpdate with the reactive useMessage snapshot hook; model mutations now re-render through useSyncExternalStore. - Replace shouldComponentUpdate with React.memo and an equivalent custom comparator over the same nine props. - Move isManualUnignored to useState, theming to useTheme, and defaultProps to destructuring parameter defaults. - Keep the debounced onPress as a single lifetime instance via a ref, always invoking the latest closure. Render output is unchanged: the existing Message snapshot suite passes without updates. Adds a behavior test for the row container.
Move `action` and `selectedMessages` out of the RoomView class state into a per-Room Zustand store (InteractionStore), mirroring the MessageComposer store topology. Message rows now read their edit state via a `useIsBeingEdited` selector instead of a prop, so starting or cancelling an edit re-renders only the affected row instead of triggering a RoomView render and a FlatList walk. Composer consumers read the interaction values from the store while keeping their handlers on RoomContext, so the RoomContext value no longer changes when interaction state changes. ShareView provides its own store for its composer subtree. Behavior is unchanged.
Remove the umbrella IMessage/IMessageInner interfaces and the {...props}
bag spread from the message component tree. Each presentational component
now declares the narrow interface it consumes and receives explicit props.
Shared handlers (getCustomEmoji, navToRoomInfo, showAttachment, blockAction,
handleEnterCall, fetchThreadName) are read from MessageContext rather than
threaded as props, and MessageContext is now typed (IMessageContext) in
place of `any`. Behavior is unchanged.
…NATIVE-1351) Add the `'use memo'` opt-in directive to the three presentational memo components in Message.tsx (MessageInner, Message, MessageTouchable), keeping their explicit React.memo gate. This mirrors the existing precedent in message/index.tsx (MessageContainer) under the project's React Compiler annotation mode. Clean Message.stories.tsx to pass only the explicit data props each story exercises: drop the per-story handler props (onReactionPress, onErrorPress, replyBroadcast) and the dead `theme` prop, all of which are now supplied through the MessageContext provider value. Behavior is unchanged — the snapshot suite passes without updates.
Drop three confirmed-dead members from the RoomView container state: the unused reaction-modal visibility flag (reactionsModalVisible), the unused reply-with-mention flag (replyWithMention), and the redundant on-hold flag (isOnHold). reactionsModalVisible and replyWithMention were only ever initialized and type-declared, never read or re-set. isOnHold was set alongside roomUpdate and only compared in shouldComponentUpdate; that comparison is redundant because roomUpdate already carries onHold (it is part of roomAttrsUpdate), so on-hold transitions still trigger a re-render through the existing roomUpdate diff. No behavior change — the full test suite passes without snapshot updates.
|
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:
WalkthroughThis PR moves room interaction state into a Zustand-backed store, adds a per-message store for derived message data, refactors message rendering and attachment components to use typed context/hooks, updates WatermelonDB JSON decorators to memoize sanitization, and switches documentation references to ChangesInteractionStore and message rendering
WatermelonDB JSON Decorator Memoization
Documentation Reference Updates
Estimated code review effort: 5 (Critical) | ~150 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (9)
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 |
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 (2)
app/lib/database/model/Subscription.js (1)
40-162: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftClone memoized JSON fields before exposing them from
asPlain()
@json(..., { memo: true })returns the same object reference across reads, andasPlain()only creates a shallow snapshot. The JSON-backed fields are still shared by reference, so mutating them through the returned “plain” object mutates the model cache too. Clone or freeze those fields before returning them.🤖 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/lib/database/model/Subscription.js` around lines 40 - 162, The issue is that memoized `@json` fields are being exposed by asPlain() by reference, so the “plain” snapshot still shares mutable objects with the model cache. Update asPlain() in Subscription to deep-clone or freeze the memoized JSON-backed properties (those declared with `@json`(..., { memo: true })) before returning them, so reads from the plain object cannot mutate the cached model state.app/containers/message/Components/Attachments/Reply.tsx (1)
219-228: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReset
loadingon preview failures.If
fileDownloadAndPreview()rejects here,setLoading(false)never runs and this reply stays disabled behind the spinner. Wrap the awaited path intry/catch/finallyso the UI always recovers and the failure is handled explicitly. As per coding guidelines, "Use explicit error handling with try/catch blocks for async operations." Based on learnings, this repo only treats async event handlers as a review concern when there is a real unhandled promise path.Suggested fix
if (attachment.type === 'file' && attachment.title_link) { setLoading(true); - url = formatAttachmentUrl(attachment.title_link, user?.id ?? '', user?.token ?? '', baseUrl ?? ''); - await fileDownloadAndPreview(url, attachment, id ?? ''); - setLoading(false); + try { + url = formatAttachmentUrl(attachment.title_link, user?.id ?? '', user?.token ?? '', baseUrl ?? ''); + await fileDownloadAndPreview(url, attachment, id ?? ''); + } catch (e) { + // surface/log the failure here + } finally { + setLoading(false); + } 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/containers/message/Components/Attachments/Reply.tsx` around lines 219 - 228, In Reply.tsx, the onPress async handler leaves loading stuck true if fileDownloadAndPreview() rejects, so update the file-download branch in onPress to use explicit try/catch/finally around the awaited call and always reset loading in the finally block. Keep the existing attachment.url formatting logic, but make sure any preview failure is handled in the catch path so the reply UI can recover instead of staying disabled.Sources: Coding guidelines, Learnings
🧹 Nitpick comments (4)
app/views/RoomView/InteractionStore.tsx (2)
6-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse interfaces for the store object shapes.
TInteractionActionsandInteractionStatedefine object shapes, so they should be interfaces per the TS guideline.Refactor sketch
-type TInteractionActions = { +interface IInteractionActions { setEditing(messageId: string): void; initQuote(messageId: string): void; appendQuote(messageId: string): void; removeQuote(messageId: string): void; setReacting(messageId: string): void; setQuotes(messageIds: string[]): void; reset(): void; -}; +} -type InteractionState = { +interface IInteractionState { action: TMessageAction; selectedMessages: string[]; // Built once inside the store initializer — stable ref for consumers. - actions: TInteractionActions; -}; + actions: IInteractionActions; +}As per coding guidelines, “Prefer interfaces over type aliases for defining object shapes in TypeScript”.
🤖 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/InteractionStore.tsx` around lines 6 - 21, The store object shapes in TInteractionActions and InteractionState should use interfaces instead of type aliases. Update the declarations in InteractionStore to convert both shape definitions to interfaces while keeping the same members and preserving the stable actions reference used by the store initializer.Source: Coding guidelines
23-38: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCopy
selectedMessagesat the store boundary.
initialState.selectedMessagesandsetQuotes(messageIds)currently retain caller-owned array references. Copy them before storing so external mutations cannot change Zustand state without notifying subscribers.Proposed fix
- selectedMessages: initialState?.selectedMessages ?? [], + selectedMessages: initialState?.selectedMessages ? [...initialState.selectedMessages] : [], ... - setQuotes: messageIds => set({ action: messageIds.length ? 'quote' : null, selectedMessages: messageIds }), + setQuotes: messageIds => set({ action: messageIds.length ? 'quote' : null, selectedMessages: [...messageIds] }),🤖 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/InteractionStore.tsx` around lines 23 - 38, The InteractionStore state is keeping external array references for selectedMessages, which can let caller mutations bypass Zustand updates. In createInteractionStore, copy initialState.selectedMessages when initializing state, and in the setQuotes action copy the incoming messageIds before saving them. Use the existing createInteractionStore and actions.setQuotes symbols to locate the boundary where the array should be cloned.app/containers/MessageComposer/components/CancelEdit.tsx (1)
6-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return type to
CancelEdit.This changed TSX component still relies on inference. Please annotate the return type explicitly so the exported contract stays obvious.
As per coding guidelines,
**/*.{ts,tsx}:Use TypeScript for type safety; add explicit type annotations to function parameters and return types.🤖 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/containers/MessageComposer/components/CancelEdit.tsx` around lines 6 - 10, The CancelEdit component is still relying on inferred return typing, so make its exported contract explicit. Update the CancelEdit function declaration to include an explicit return type, using the function name CancelEdit as the anchor, while keeping its current use of useRoomContext and useMessageAction unchanged.Source: Coding guidelines
app/containers/MessageComposer/hooks/useAutoSaveDraft.ts (1)
9-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the
useAutoSaveDraftsignature explicit.
textand the returned shape are both inferred here. Please add explicit parameter and return types so this exported hook’s contract is clear at the call site.As per coding guidelines,
**/*.{ts,tsx}:Use TypeScript for type safety; add explicit type annotations to function parameters and return types.🤖 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/containers/MessageComposer/hooks/useAutoSaveDraft.ts` around lines 9 - 15, The exported hook useAutoSaveDraft currently relies on inference for both its input and return value, so make the contract explicit by adding a type annotation for the text parameter and an explicit return type on useAutoSaveDraft. Keep the existing logic intact, but ensure the returned object shape from the hook is declared so call sites can rely on the typed API even if internals change.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/containers/message/Components/Attachments/Reply.tsx`:
- Line 168: The thumbnail URL fallback in Reply should not interpolate baseUrl
when it may be missing, since that can produce invalid undefined/... paths.
Update the relative-image handling in Reply.tsx to guard the baseUrl-dependent
branch in the same place where image is normalized, so quoted attachment
thumbnails fail closed when MessageContext is partial or default. Use the
existing Reply component’s image assignment logic and preserve the http check,
but only build the relative URL when baseUrl is available.
In `@app/containers/message/index.tsx`:
- Around line 248-253: The link handling in onLinkPress incorrectly treats
external links as message links when message.attachments is undefined because
the optional chain makes the comparison to -1 evaluate true. Update the
isMessageLink check in message/containers/message/index.tsx to only return true
when attachments actually exist and a matching attachment.message_link is found,
so jumpToMessage is only called for real message links and openLink remains the
fallback.
- Around line 69-79: The memo comparator in areEqual is missing item and other
render-consumed props, so it can incorrectly skip updates and leave
MessageContainer rendering stale data. Update areEqual to compare prev.item and
every prop that affects rendering or hook inputs such as useMessage(next.item),
or remove the custom comparator entirely if exhaustive comparison is
impractical. Keep the check aligned with IMessageContainerProps and the values
actually used inside the message container render path.
In `@app/lib/encryption/room.ts`:
- Around line 731-732: The `message.attachments` update in `room.ts` is racing
across concurrent `Promise.all` branches, so later URL decryptions can overwrite
earlier quote attachments. Update the quote-handling flow in the relevant
decrypt/attach logic to collect all new quote attachments first, then assign to
`message.attachments` once after `Promise.all` completes, using the existing
`message.attachments` and quote attachment creation path as the reference
points.
In `@app/views/ShareView/index.tsx`:
- Line 243: ShareView is still using local state as the quote source in send()
and unmount, which can drift from the interactionStore updates made through
setQuotes(selectedMessages). Update ShareView to read the current quote/action
data from interactionStore everywhere it needs quote state, and keep the local
state in sync only if necessary. Use the existing ShareView methods and fields
around send(), componentWillUnmount, this.state.action, and
this.state.selectedMessages to replace stale reads with store-backed values.
---
Outside diff comments:
In `@app/containers/message/Components/Attachments/Reply.tsx`:
- Around line 219-228: In Reply.tsx, the onPress async handler leaves loading
stuck true if fileDownloadAndPreview() rejects, so update the file-download
branch in onPress to use explicit try/catch/finally around the awaited call and
always reset loading in the finally block. Keep the existing attachment.url
formatting logic, but make sure any preview failure is handled in the catch path
so the reply UI can recover instead of staying disabled.
In `@app/lib/database/model/Subscription.js`:
- Around line 40-162: The issue is that memoized `@json` fields are being exposed
by asPlain() by reference, so the “plain” snapshot still shares mutable objects
with the model cache. Update asPlain() in Subscription to deep-clone or freeze
the memoized JSON-backed properties (those declared with `@json`(..., { memo: true
})) before returning them, so reads from the plain object cannot mutate the
cached model state.
---
Nitpick comments:
In `@app/containers/MessageComposer/components/CancelEdit.tsx`:
- Around line 6-10: The CancelEdit component is still relying on inferred return
typing, so make its exported contract explicit. Update the CancelEdit function
declaration to include an explicit return type, using the function name
CancelEdit as the anchor, while keeping its current use of useRoomContext and
useMessageAction unchanged.
In `@app/containers/MessageComposer/hooks/useAutoSaveDraft.ts`:
- Around line 9-15: The exported hook useAutoSaveDraft currently relies on
inference for both its input and return value, so make the contract explicit by
adding a type annotation for the text parameter and an explicit return type on
useAutoSaveDraft. Keep the existing logic intact, but ensure the returned object
shape from the hook is declared so call sites can rely on the typed API even if
internals change.
In `@app/views/RoomView/InteractionStore.tsx`:
- Around line 6-21: The store object shapes in TInteractionActions and
InteractionState should use interfaces instead of type aliases. Update the
declarations in InteractionStore to convert both shape definitions to interfaces
while keeping the same members and preserving the stable actions reference used
by the store initializer.
- Around line 23-38: The InteractionStore state is keeping external array
references for selectedMessages, which can let caller mutations bypass Zustand
updates. In createInteractionStore, copy initialState.selectedMessages when
initializing state, and in the setQuotes action copy the incoming messageIds
before saving them. Use the existing createInteractionStore and
actions.setQuotes symbols to locate the boundary where the array should be
cloned.
🪄 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: 9bcc5d87-b924-4943-af20-abe404e704ac
⛔ Files ignored due to path filters (1)
app/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (64)
.rnstorybook/preview.tsxCLAUDE.mdCONTEXT.mdapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/MessageComposer/components/Quotes/Quotes.tsxapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/containers/ReactionsList/index.tsxapp/containers/message/Blocks.tsapp/containers/message/Broadcast.tsxapp/containers/message/CallButton.tsxapp/containers/message/Components/Attachments/AttachedActions.tsxapp/containers/message/Components/Attachments/Attachments.stories.tsxapp/containers/message/Components/Attachments/Attachments.tsxapp/containers/message/Components/Attachments/Audio.tsxapp/containers/message/Components/Attachments/CollapsibleQuote/index.tsxapp/containers/message/Components/Attachments/Image/Container.tsxapp/containers/message/Components/Attachments/Quote.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/containers/message/Components/Attachments/Video.tsxapp/containers/message/Content.test.tsxapp/containers/message/Content.tsxapp/containers/message/Context.tsapp/containers/message/Message.stories.tsxapp/containers/message/Message.tsxapp/containers/message/MessageAvatar.tsxapp/containers/message/Reactions.test.tsxapp/containers/message/Reactions.tsxapp/containers/message/RepliedThread.tsxapp/containers/message/Thread.tsxapp/containers/message/Urls.tsxapp/containers/message/User.tsxapp/containers/message/hooks/useMediaAutoDownload.tsxapp/containers/message/hooks/useMessage.test.tsapp/containers/message/hooks/useMessage.tsapp/containers/message/hooks/useMessageAccessibilityLabel.test.tsapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/index.test.tsxapp/containers/message/index.tsxapp/containers/message/interfaces.tsapp/lib/database/model/CustomEmoji.jsapp/lib/database/model/Message.jsapp/lib/database/model/Permission.jsapp/lib/database/model/Room.jsapp/lib/database/model/Setting.jsapp/lib/database/model/Subscription.jsapp/lib/database/model/Thread.jsapp/lib/database/model/ThreadMessage.jsapp/lib/database/model/User.jsapp/lib/database/model/servers/Server.jsapp/lib/database/model/servers/User.jsapp/lib/encryption/room.tsapp/lib/services/voip/docs/README.mdapp/views/RoomView/InteractionStore.tsxapp/views/RoomView/constants.tsapp/views/RoomView/context.tsapp/views/RoomView/definitions.tsapp/views/RoomView/docs/ARCHITECTURE.mdapp/views/RoomView/index.tsxapp/views/ShareView/index.tsx
💤 Files with no reviewable changes (2)
- app/views/RoomView/constants.ts
- app/views/RoomView/context.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/containers/message/Components/Attachments/Audio.tsxapp/containers/message/Components/Attachments/Image/Container.tsxapp/containers/MessageComposer/components/Quotes/Quotes.tsxapp/containers/message/Broadcast.tsxapp/containers/message/Components/Attachments/Video.tsxapp/lib/database/model/User.jsapp/containers/message/Thread.tsxapp/containers/message/Content.tsxapp/containers/message/Components/Attachments/AttachedActions.tsxapp/lib/database/model/Permission.jsapp/containers/message/Urls.tsxapp/containers/message/Components/Attachments/CollapsibleQuote/index.tsxapp/lib/database/model/servers/Server.jsapp/containers/message/Reactions.test.tsxapp/containers/ReactionsList/index.tsxapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/RepliedThread.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.test.tsapp/lib/database/model/CustomEmoji.jsapp/lib/database/model/servers/User.jsapp/containers/message/Components/Attachments/Quote.tsxapp/containers/message/hooks/useMessage.tsapp/containers/message/hooks/useMessage.test.tsapp/lib/encryption/room.tsapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/lib/database/model/Setting.jsapp/containers/message/index.test.tsxapp/containers/message/Blocks.tsapp/lib/database/model/ThreadMessage.jsapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/message/CallButton.tsxapp/views/RoomView/InteractionStore.tsxapp/containers/message/hooks/useMediaAutoDownload.tsxapp/containers/message/Context.tsapp/lib/database/model/Message.jsapp/containers/message/Components/Attachments/Attachments.stories.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/message/Components/Attachments/Attachments.tsxapp/lib/database/model/Room.jsapp/containers/message/MessageAvatar.tsxapp/views/ShareView/index.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/Content.test.tsxapp/lib/database/model/Subscription.jsapp/containers/MessageComposer/MessageComposer.test.tsxapp/views/RoomView/definitions.tsapp/lib/database/model/Thread.jsapp/containers/MessageComposer/MessageComposer.tsxapp/containers/message/User.tsxapp/containers/message/Reactions.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/containers/message/Message.stories.tsxapp/containers/message/Message.tsxapp/containers/message/interfaces.tsapp/views/RoomView/index.tsxapp/containers/message/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
Files:
app/containers/message/Components/Attachments/Audio.tsxapp/containers/message/Components/Attachments/Image/Container.tsxapp/containers/MessageComposer/components/Quotes/Quotes.tsxapp/containers/message/Broadcast.tsxapp/containers/message/Components/Attachments/Video.tsxapp/containers/message/Thread.tsxapp/containers/message/Content.tsxapp/containers/message/Components/Attachments/AttachedActions.tsxapp/containers/message/Urls.tsxapp/containers/message/Components/Attachments/CollapsibleQuote/index.tsxapp/containers/message/Reactions.test.tsxapp/containers/ReactionsList/index.tsxapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/RepliedThread.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.test.tsapp/containers/message/Components/Attachments/Quote.tsxapp/containers/message/hooks/useMessage.tsapp/containers/message/hooks/useMessage.test.tsapp/lib/encryption/room.tsapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/containers/message/index.test.tsxapp/containers/message/Blocks.tsapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/message/CallButton.tsxapp/views/RoomView/InteractionStore.tsxapp/containers/message/hooks/useMediaAutoDownload.tsxapp/containers/message/Context.tsapp/containers/message/Components/Attachments/Attachments.stories.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/message/Components/Attachments/Attachments.tsxapp/containers/message/MessageAvatar.tsxapp/views/ShareView/index.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/Content.test.tsxapp/containers/MessageComposer/MessageComposer.test.tsxapp/views/RoomView/definitions.tsapp/containers/MessageComposer/MessageComposer.tsxapp/containers/message/User.tsxapp/containers/message/Reactions.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/containers/message/Message.stories.tsxapp/containers/message/Message.tsxapp/containers/message/interfaces.tsapp/views/RoomView/index.tsxapp/containers/message/index.tsx
{index.js,app/**/*.{js,jsx,ts,tsx}}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow the project’s Prettier style: tabs, single quotes, 130-character line width, no trailing commas, omit arrow-function parens when possible, and keep bracket spacing on the same line.
Files:
app/containers/message/Components/Attachments/Audio.tsxapp/containers/message/Components/Attachments/Image/Container.tsxapp/containers/MessageComposer/components/Quotes/Quotes.tsxapp/containers/message/Broadcast.tsxapp/containers/message/Components/Attachments/Video.tsxapp/lib/database/model/User.jsapp/containers/message/Thread.tsxapp/containers/message/Content.tsxapp/containers/message/Components/Attachments/AttachedActions.tsxapp/lib/database/model/Permission.jsapp/containers/message/Urls.tsxapp/containers/message/Components/Attachments/CollapsibleQuote/index.tsxapp/lib/database/model/servers/Server.jsapp/containers/message/Reactions.test.tsxapp/containers/ReactionsList/index.tsxapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/RepliedThread.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.test.tsapp/lib/database/model/CustomEmoji.jsapp/lib/database/model/servers/User.jsapp/containers/message/Components/Attachments/Quote.tsxapp/containers/message/hooks/useMessage.tsapp/containers/message/hooks/useMessage.test.tsapp/lib/encryption/room.tsapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/lib/database/model/Setting.jsapp/containers/message/index.test.tsxapp/containers/message/Blocks.tsapp/lib/database/model/ThreadMessage.jsapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/message/CallButton.tsxapp/views/RoomView/InteractionStore.tsxapp/containers/message/hooks/useMediaAutoDownload.tsxapp/containers/message/Context.tsapp/lib/database/model/Message.jsapp/containers/message/Components/Attachments/Attachments.stories.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/message/Components/Attachments/Attachments.tsxapp/lib/database/model/Room.jsapp/containers/message/MessageAvatar.tsxapp/views/ShareView/index.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/Content.test.tsxapp/lib/database/model/Subscription.jsapp/containers/MessageComposer/MessageComposer.test.tsxapp/views/RoomView/definitions.tsapp/lib/database/model/Thread.jsapp/containers/MessageComposer/MessageComposer.tsxapp/containers/message/User.tsxapp/containers/message/Reactions.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/containers/message/Message.stories.tsxapp/containers/message/Message.tsxapp/containers/message/interfaces.tsapp/views/RoomView/index.tsxapp/containers/message/index.tsx
🧠 Learnings (5)
📚 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/containers/message/Components/Attachments/Audio.tsxapp/containers/message/Components/Attachments/Image/Container.tsxapp/containers/MessageComposer/components/Quotes/Quotes.tsxapp/containers/message/Broadcast.tsxapp/containers/message/Components/Attachments/Video.tsxapp/containers/message/Thread.tsxapp/containers/message/Content.tsxapp/containers/message/Components/Attachments/AttachedActions.tsxapp/containers/message/Urls.tsxapp/containers/message/Components/Attachments/CollapsibleQuote/index.tsxapp/containers/message/Reactions.test.tsxapp/containers/ReactionsList/index.tsxapp/containers/MessageComposer/hooks/useChooseMedia.tsapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/RepliedThread.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.test.tsapp/containers/message/Components/Attachments/Quote.tsxapp/containers/message/hooks/useMessage.tsapp/containers/message/hooks/useMessage.test.tsapp/lib/encryption/room.tsapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/containers/message/index.test.tsxapp/containers/message/Blocks.tsapp/containers/MessageComposer/hooks/useAutoSaveDraft.tsapp/containers/message/CallButton.tsxapp/views/RoomView/InteractionStore.tsxapp/containers/message/hooks/useMediaAutoDownload.tsxapp/containers/message/Context.tsapp/containers/message/Components/Attachments/Attachments.stories.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/message/Components/Attachments/Attachments.tsxapp/containers/message/MessageAvatar.tsxapp/views/ShareView/index.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/Content.test.tsxapp/containers/MessageComposer/MessageComposer.test.tsxapp/views/RoomView/definitions.tsapp/containers/MessageComposer/MessageComposer.tsxapp/containers/message/User.tsxapp/containers/message/Reactions.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/containers/message/Message.stories.tsxapp/containers/message/Message.tsxapp/containers/message/interfaces.tsapp/views/RoomView/index.tsxapp/containers/message/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/containers/message/Components/Attachments/Audio.tsxapp/containers/message/Components/Attachments/Image/Container.tsxapp/containers/MessageComposer/components/Quotes/Quotes.tsxapp/containers/message/Broadcast.tsxapp/containers/message/Components/Attachments/Video.tsxapp/containers/message/Thread.tsxapp/containers/message/Content.tsxapp/containers/message/Components/Attachments/AttachedActions.tsxapp/containers/message/Urls.tsxapp/containers/message/Components/Attachments/CollapsibleQuote/index.tsxapp/containers/message/Reactions.test.tsxapp/containers/ReactionsList/index.tsxapp/containers/MessageComposer/components/CancelEdit.tsxapp/containers/message/RepliedThread.tsxapp/containers/message/Components/Attachments/Quote.tsxapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/containers/message/index.test.tsxapp/containers/message/CallButton.tsxapp/views/RoomView/InteractionStore.tsxapp/containers/message/hooks/useMediaAutoDownload.tsxapp/containers/message/Components/Attachments/Attachments.stories.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/message/Components/Attachments/Attachments.tsxapp/containers/message/MessageAvatar.tsxapp/views/ShareView/index.tsxapp/containers/message/Content.test.tsxapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/message/User.tsxapp/containers/message/Reactions.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/containers/message/Message.stories.tsxapp/containers/message/Message.tsxapp/views/RoomView/index.tsxapp/containers/message/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/containers/message/Reactions.test.tsxapp/containers/message/hooks/useMessageAccessibilityLabel.test.tsapp/containers/message/hooks/useMessage.test.tsapp/containers/MessageComposer/hooks/useChooseMedia.test.tsxapp/containers/message/index.test.tsxapp/containers/message/Content.test.tsxapp/containers/MessageComposer/MessageComposer.test.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/InteractionStore.tsxapp/views/ShareView/index.tsxapp/views/RoomView/index.tsx
📚 Learning: 2026-03-15T13:55:42.038Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6911
File: app/containers/markdown/Markdown.stories.tsx:104-104
Timestamp: 2026-03-15T13:55:42.038Z
Learning: In Rocket.Chat React Native, the markdown parser requires a space between the underscore wrapping italic text and a mention sigil (_ mention _ instead of _mention_). Ensure stories and tests that include italic-wrapped mentions follow this form to guarantee proper parsing. Specifically, for files like app/containers/markdown/Markdown.stories.tsx, and any test/content strings that exercise italic-mentions, use the pattern _ mention _ (with spaces) to prevent the mention from being treated as plain text. Validate any test strings or story content accordingly.
Applied to files:
app/containers/message/Components/Attachments/Attachments.stories.tsxapp/containers/message/Message.stories.tsx
🔇 Additional comments (51)
CONTEXT.md (1)
55-63: LGTM!CLAUDE.md (1)
12-12: LGTM!app/lib/services/voip/docs/README.md (1)
15-15: LGTM!app/views/RoomView/docs/ARCHITECTURE.md (1)
3-3: LGTM!app/containers/message/hooks/useMessageAccessibilityLabel.ts (1)
4-27: LGTM!Also applies to: 29-49
app/containers/message/hooks/useMessageAccessibilityLabel.test.ts (1)
4-11: LGTM!app/containers/ReactionsList/index.tsx (1)
33-33: LGTM!app/containers/message/Broadcast.tsx (1)
20-20: LGTM!app/containers/message/Components/Attachments/AttachedActions.tsx (1)
28-28: LGTM!app/containers/message/hooks/useMediaAutoDownload.tsx (1)
65-71: 🗄️ Data Integrity & IntegrationEmpty-string fallbacks need a contract check
Passing
''for missingid,baseUrl, oruserintouseFile(),formatAttachmentUrl(), anddownloadMediaFile()may turn missing context into real cache or network keys. If those helpers don’t treat empty strings as invalid sentinels, this can produce malformed URLs or collisions.app/containers/message/Components/Attachments/Audio.tsx (1)
26-27: LGTM!app/containers/message/Components/Attachments/CollapsibleQuote/index.tsx (1)
96-96: LGTM!Also applies to: 119-119
app/containers/message/Components/Attachments/Image/Container.tsx (1)
55-55: LGTM!app/containers/message/Components/Attachments/Video.tsx (1)
104-104: LGTM!app/containers/message/Urls.tsx (1)
133-134: LGTM!app/views/RoomView/InteractionStore.tsx (1)
44-79: LGTM!app/views/RoomView/definitions.ts (1)
9-9: LGTM!Also applies to: 63-67
app/views/RoomView/index.tsx (2)
101-101: LGTM!Also applies to: 146-175, 224-226, 788-846, 848-858, 876-893, 1385-1507, 1642-1719
268-301: 🎯 Functional CorrectnessKeep
RoomViewinvalidating onroom.onHoldchanges
renderFooter()still branches onroom.onHold; ifroomAttrsUpdateno longer includes that field, livechat hold/resume updates will be blocked byshouldComponentUpdate.app/views/ShareView/index.tsx (1)
42-42: LGTM!Also applies to: 84-94, 392-422
app/containers/MessageComposer/hooks/useChooseMedia.ts (2)
13-13: LGTM!
35-37: 🩺 Stability & AvailabilityVerify every media-picker composer path is wrapped in
InteractionStoreContext.useMessageAction()anduseSelectedMessages()crash without it, so any remainingMessageComposerContaineroruseChooseMediaentry point outside the provider will fail.app/containers/MessageComposer/MessageComposer.tsx (1)
8-8: LGTM!Also applies to: 58-60
app/containers/MessageComposer/components/ComposerInput.tsx (1)
36-36: LGTM!Also applies to: 54-56
app/containers/MessageComposer/components/Quotes/Quotes.tsx (1)
5-11: LGTM!app/containers/message/Context.ts (1)
2-39: LGTM!app/containers/message/interfaces.ts (1)
16-20: LGTM!Also applies to: 30-66, 95-98
app/containers/message/Blocks.ts (1)
1-11: LGTM!app/containers/message/CallButton.tsx (1)
1-1: LGTM!Also applies to: 11-21
app/containers/message/RepliedThread.tsx (1)
1-1: LGTM!Also applies to: 11-17
app/lib/database/model/ThreadMessage.js (1)
21-81: 🗄️ Data Integrity & IntegrationCheck WatermelonDB version compatibility
@json(..., { memo: true })needs@nozbe/watermelondb0.28.1+; if this repo pins an older release, keep the two-argument form.app/containers/message/MessageAvatar.tsx (1)
19-45: LGTM!app/containers/message/Reactions.tsx (1)
52-100: LGTM!app/containers/message/Components/Attachments/Attachments.tsx (1)
17-91: LGTM!app/lib/database/model/Room.js (1)
11-29: LGTM!app/lib/database/model/Setting.js (1)
17-17: LGTM!app/lib/database/model/Thread.js (1)
21-77: 🗄️ Data Integrity & Integration
Thread.asPlain()should not expose memoized JSON fields by reference. If any caller mutates the returned arrays/objects, it will mutate the cached model snapshot in place; clone or freeze these fields before returning them.app/containers/message/Components/Attachments/Quote.tsx (1)
13-27: LGTM!app/containers/message/Content.tsx (1)
21-21: LGTM!Also applies to: 35-46, 78-82
app/containers/message/User.tsx (1)
56-57: 🎯 Functional CorrectnessNormalize
tsbefore passing it toMessageTime.Widening
tstoDate | stringis fine, but the cast on Line 122 does not convert the runtime value. If the new string path is exercised,MessageTimestill receives a string here. Please either normalize toDateat this boundary or verify thatMessageTimenow accepts strings end-to-end.Also applies to: 122-122
app/containers/message/Thread.tsx (1)
42-44: 📐 Maintainability & Code QualityPlease type this callback at the boundary instead of casting to
Function.
toggleFollowThread as Functionremoves the call signature exactly where this refactor is trying to tighten contracts. IfThreadDetailsandMessageContextdrifted during the migration, this cast will hide it until runtime. As per coding guidelines, "Use TypeScript for type safety; add explicit type annotations to function parameters and return types."Source: Coding guidelines
app/lib/database/model/servers/Server.js (1)
38-38: LGTM!app/lib/database/model/servers/User.js (1)
23-23: LGTM!app/containers/message/hooks/useMessage.test.ts (1)
1-127: LGTM!app/containers/message/index.test.tsx (1)
1-127: LGTM!app/containers/MessageComposer/MessageComposer.test.tsx (1)
15-16: LGTM!Also applies to: 119-139, 650-653, 685-687, 719-726, 735-737
app/containers/MessageComposer/hooks/useChooseMedia.test.tsx (1)
21-25: LGTM!Also applies to: 53-55, 80-81
app/containers/message/Reactions.test.tsx (1)
25-26: LGTM!Also applies to: 40-43
app/containers/message/Message.stories.tsx (1)
5-6: LGTM!Also applies to: 67-98, 115-128, 300-300, 331-331, 2247-2265
app/containers/message/hooks/useMessage.ts (1)
109-114: 🩺 Stability & AvailabilityInvestigate stale subscription callbacks after
itemchanges. The callback closes over the subscribeditem, so if the old subscription fires after a model swap it can overwritesnapshotRefwith stale data.app/lib/database/model/Message.js (1)
21-87: 🗄️ Data Integrity & IntegrationProtect
asPlain()from exposing memoized JSON objects. If any consumer mutates these fields in place,memo: truewill make those changes stick to the model cache and leak across later snapshots/renders. Clone or freeze the JSON-backed fields before returning them, or keep them immutable end to end.
|
iOS Build Available Rocket.Chat 4.74.0.109241 |
|
Android Build Available Rocket.Chat 4.74.0.109240 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNRPYLAC5IzIh8x8rNLkdVJFHFLd-DfMV2u8Eq2Op-URdHLF11z7cS6exnF9GPdQJlr0Li1NHkUUqJ53neVB |
diegolmello
left a comment
There was a problem hiding this comment.
Automated max-effort review — feat: modernize message rendering with reactive hooks
8 findings below as inline comments, ranked by severity.
- High — the React Compiler silently bails on
MessageContaineranduseMessage(ref writes during render), so the'use memo'directives there are no-ops. The newMessageContextvalue/handlers are recreated every render and the WatermelonDB observer re-subscribes every render, which undercuts the per-field memoization this PR is built around. Verified against the emitted babel output. - Medium —
areEqualcompares a non-existentpreviousItem?._id(dead comparison → stale grouping);onLinkPressmis-routes plain links whenattachmentsis undefined. - Low —
appendQuotelacks dedup;useRef(debounce(...))re-allocates each render;getMessageTranslationbypasses the snapshot; ShareView keepsselectedMessagesin two stores;RepliedThread's mount-only effect.
Findings #2 and #3 are pre-existing (carried from the class component) but re-exposed in the rewritten code. Posted as comments, not change requests.
diegolmello
left a comment
There was a problem hiding this comment.
Automated two-axis review — /review (Standards + Spec)
Posted as COMMENT (the PR author can't request changes on their own PR). Fixed point origin/develop @ 3549de64b; 65 files, +1613/-886.
🔴 One merge-blocker — see the inline thread on app/containers/message/hooks/useMessage.ts. useMessage calls experimentalSubscribe unconditionally, dropping the old container's if (item && item.experimentalSubscribe) guard. Files / Mentions / Starred / Pinned (MessagesView, REST) and non-encrypted Search (SearchMessagesView, REST) render the row with plain objects → TypeError on mount. This matches the deterministic Android E2E shard-11 failures (message-content-30, starred-messages-view, 3/3 retries). Those two views are unchanged by this PR, so they carry no inline anchor — root cause + fix live on the useMessage.ts line 111 thread.
Standards axis: 3 hard (no-noise comments ×2, 'use memo' on a non-component hook), 3 judgement (Duplicated Code, Middle Man, Speculative Generality).
Spec axis: 1 critical (above); NATIVE-1349 structural AC gaps (no separate compiler-memoized provider component; RoomView/context.ts still room: any, not split into individual fields); NATIVE-1348 mock not at the DB-service boundary; NATIVE-1351 snapshot testID="...-undefined"; scope creep (data fields in MessageContext).
The two axes are reported separately by design and not reranked against each other.
Collect quote attachments from Promise.all and assign message.attachments once after it resolves, instead of racing read-modify-write across the concurrent decrypt branches (which dropped quotes on multi-quote messages).
Dead export with zero callers; consumers use the narrower hooks or read getState().actions directly.
…reads - Guard the model subscription so plain REST objects (search, starred, pinned, mentions, files) no longer throw 'experimentalSubscribe is not a function' on mount; serve a static snapshot when the model is not observable - Remove render-body ref writes from MessageContainer and useMessage so the React Compiler memoizes both (stable subscribe/getSnapshot, snapshot cached by item identity; debounced press handler created once, latest closure via effect) - areEqual compares previousItem.id (was ._id, always undefined on WatermelonDB models) and documents why item is intentionally omitted - isMessageLink no longer treats plain links as message links when attachments is undefined - read the translation from the useMessage snapshot instead of the live model - drop redundant/historical comments Adds a useMessage regression test for the non-observable plain-object path.
|
Android Build Available Rocket.Chat 4.75.0.109321 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNQEhuXNbivFx4PP4Mq0T1JP4ktqdCQS6jQLT-HaNAo-Fd_UvjC5sR2kU2eRZ3sV7P6TGBabkDOBhCWm3wkt |
|
iOS Build Available Rocket.Chat 4.75.0.109322 |
…TIVE-22) RoomProviders now mounts MessageActionProvider (external-store param) instead of the raw context, so no message provider is prod-unused. Move index.testHelpers into __tests__/testHelpers with an ignore pattern so Jest does not treat it as a suite.
Drop inert Fields.displayName assignments (plain FCs, not React.memo) in Reply and CollapsibleQuote, and a redundant ContentLayout comment. Addresses self-review nits on PR #7455.
|
iOS Build Available Rocket.Chat 4.75.0.109324 |
Move Touch, MessageActionTouchable, and the misnamed Message/index.tsx (actually MessageTouchable) into a single components/Touchable/ folder. Pure move + rename + import path fixups, no behavior change.
…ate (NATIVE-22) InteractionStore was renamed to MessageActionStore; the ubiquitous-language glossary followed. Selection now lives inside the Message Action union, so the separate 'Interaction state' term collapses into 'Message Action State'.
- add useIsOwnMessage selector; drop repeated author/user equality in User and Broadcast - add getAttachmentKey; collapse the four copy-pasted attachment key fallbacks - extract MessageAccessibleIndex; share the a11y index between Compact/FullMessage - add shallowEqual to MessagePreview selector to stop re-renders on unrelated dispatches
|
Android Build Available Rocket.Chat 4.75.0.109325 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNSdrOCCfoCUKw3rg0577H0F-MOMy_KQGi1kLd2LOYJRj8bJ0cW39ak0BFuGnX9L6hxUrkXhss-0uNXFdmyu |
|
iOS Build Available Rocket.Chat 4.75.0.109326 |
|
Android Build Available Rocket.Chat 4.75.0.109331 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNTa7X50WiAh_GcCsrWQuI5kgPDpM-4Eq1ruG_dAMpdD-RCWxWfh8zzMfbQcSV9Ygto4WMHL28YJSO1_R5Yh |
|
iOS Build Available Rocket.Chat 4.75.0.109332 |
|
Android Build Available Rocket.Chat 4.75.0.109335 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNRbPleQZogDg4BcIvYprq7f6Hdm3ZWko7HmAxol8W1dARwo8KItlZZgsr4jNziDCnHabT9_RKbW9wkmqlQs |
|
iOS Build Available Rocket.Chat 4.75.0.109336 |
Proposed changes
Modernizes the message rendering layer and folds in the two prerequisite changes it is built on.
Message rendering
useMessagehook — which snapshotted the whole message record and prop-drilled every field down the subtree — with a per-message store. The store is created once per message and ticked by the WatermelonDB record's subscription; each leaf reads only the fields it needs through granular selector hooks. A change to one field now re-renders just the consumers of that field instead of the entire message.Object.isfor a single field, shallow equality for a domain group). This relies on the model's@json(..., { memo: true })decorators, which return a stable reference when the underlying column is unchanged.MessageContextinstead of being threaded down as individual props.'use memo'directive for the React Compiler (which runs in annotation mode in this app).reactionsModalVisible,replyWithMention,isOnHold); on-hold re-renders are already covered by theroomUpdatediff inshouldComponentUpdate.Prerequisites folded in
@jsonmodel fields so repeated reads return a stable reference. The per-field selector bailout depends on this. Previously opened as perf: memoize @json model fields #7451.CONTEXT.mdand adds message-state vocabulary. Previously opened as docs: rename ubiquitous language glossary to CONTEXT and add message state vocabulary #7450.Pull requests #7450 and #7451 are closed in favor of this one, which carries their commits.
Issue(s)
How to test or reproduce
This is a behavior-preserving refactor — the goal is that nothing changes for the user.
Verified on an Android emulator across public channels, a DM, and E2EE rooms with no red box, crash, blank rows, or visual regression.
TZ=UTC pnpm testpasses andpnpm lintreports 0 errors; message-container story snapshots are unchanged, so the presentational output is byte-identical.Performance
The refactor targets re-render churn when a message arrives. We A/B'd
developagainst this branch on the same Android emulator build (API 34), swapping only the JS bundle so the two sides are directly comparable, and delivered one message over the live connection per run. The headline metric is how many components React re-renders to show the new message (fewer = less CPU); commit time in ms is reported as a corroborating, directional-only signal (debug builds inflate ms ~3× and it is GC-noisy, so it is not an absolute figure).Result: a new incoming message does ~22% less rendering work, with no regression.
develop— components / msdevelopre-rendered exactly 615 components every run; the branch 478–486. Both metrics move the same direction on all four paired runs (ms −15% to −30%), so the direction is trustworthy even though the absolute ms is debug-inflated.Two concrete wins behind the number:
Scrolling and sending were also checked and showed no regression (existing messages re-render 0 on both sides); those paths re-window a variable number of rows, so we report the structural finding rather than a misleading %. Absolute production milliseconds would require a release build on a physical device.
Screenshots
No visual change — behavior-preserving refactor.
Types of changes
Checklist
Further comments
The refactor is staged so each step is independently reviewable: memoize the
@jsonmodel fields, introduce the per-message store and selector hooks, migrate the container to a function component, relocate interaction state to a per-Room store, narrow the props contract to a context, annotate for the React Compiler, then delete the now-dead state. Story-snapshot tests render through the real message container, so an unchanged snapshot is the behavior oracle for the whole refactor.Summary by CodeRabbit