refactor: migrate RoomView to a function component - #7482
Open
diegolmello wants to merge 263 commits into
Open
Conversation
The message render tree is annotated for the React Compiler ('use memo').
A compiled parent emits referentially-stable child elements, so React.memo
on a child whose parent is also compiled is redundant. Remove the memo
wrappers and their hand-written comparators (both dequal deep-compares and
scalar shouldComponentUpdate-era allowlists) across the presentational
message components, leaving compilation as the single change-detection
mechanism.
The deep-compare removals are safe because the @JSON model fields
(attachments, urls, md, mentions, channels, reactions, replies) resolve to
stable references, making reference comparison equivalent to dequal.
MessageContainer (index.tsx) keeps its memo: its parent is RoomView's
renderItem class method, which the compiler never compiles, so that wrapper
is load-bearing.
Removing Thread's tcount-only comparator also fixes a latent bug: a tlm
change with unchanged tcount no longer suppresses the re-render that toggles
the thread button's visibility. Add Thread.test.tsx as a regression guard.
Also delete dead StyleSheet keys the memo wrappers had hidden from
react-native/no-unused-styles.
Message rendering funneled every field of a message through a single useMessage hook that snapshotted the whole record and prop-drilled it down the subtree, so a change to any one field re-rendered the entire message. Introduce a per-message store (MessageStore) created once per MessageProvider and ticked by the WatermelonDB record's experimentalSubscribe. Leaves read only the fields they need through granular selector hooks that bail via Object.is for a single field or useShallow for a domain group, so a field change re-renders just its consumers. Delete useMessage and drop the props that existed only to carry its snapshot. Attachments and Quote keep attachments/author as props: Reply renders them recursively with the nested attachment array, so a message-level hook there would recurse infinitely. Claude-Session: https://claude.ai/code/session_01Q27JMv1gF6V9urfyvGpWqn
…ubscription Row grouping depended on the previous row's mutable scalars, propagated only by the areEqual memo comparator — which cannot observe an in-place mutation of a stable model instance, so a previous message going Sent to Error never re-derived the next message's header until an unrelated re-render. Subscribe each per-message store to both its own record and the previous record; grouping and thread-position derivations read the previous model live. Delete areEqual and the MessagePrev scalar snapshot; the row keeps memo with the default comparator as a perf-only guard. Claude-Session: https://claude.ai/code/session_01PNjqp2s36bgagFz7Lo6xsV
The Timestamp story renders a dayjs fromNow relative to the real current
date; the stored snapshot ('a year ago') drifted to '2 years ago',
failing the ESLint and Test job on every run after the boundary.
Claude-Session: https://claude.ai/code/session_01PNjqp2s36bgagFz7Lo6xsV
…-1376) Adds pickMessageRoomState + a MessageProviders/renderWithMessageProviders helper and adopts it in the container test and stories so the room store is mounted alongside MessageContext during the dual-run. No production behavior change; 328 tests / 128 snapshots unchanged.
…ad (NATIVE-1376) First stable-field consumer off MessageContext. RepliedThread now subscribes only to fetchThreadName (a stable ref) via useFetchThreadName(), so it no longer re-renders on this row's send/decrypt/status transitions.
…sageStore hooks (NATIVE-1376) useMessageGrouping, useThreadPosition and useMessageText now read broadcast, Message_GroupingPeriod, isThreadRoom, user and auto-translate settings from the per-instance MessageRoomStore selectors instead of useContext(MessageContext). Consumer tests (Content, Thread, useMessageAccessibilityLabel, MessageStore) mount MessageRoomProvider alongside the legacy MessageContext.Provider, mirroring values via pickMessageRoomState during the dual-run migration.
… components (NATIVE-1376) MessageAvatar, User, Urls, CallButton, Video, Audio and Quote now read user, navToRoomInfo, getCustomEmoji, handleEnterCall, baseUrl, rid (room store) plus id and translateLanguage (per-message store) through selector hooks instead of useContext(MessageContext). No test or story changes needed: these leaves render under Message.stories and the container test, which already mount MessageRoomProvider and MessageProvider.
…s and hooks (NATIVE-1376) Attachments, Image/Container, CollapsibleQuote, useMediaAutoDownload and useMessageAccessibilityHint now read translateLanguage/id (per-message store) and user/baseUrl/showAttachment/getCustomEmoji/isThreadRoom (room store) via selector hooks instead of useContext(MessageContext). Their hand-written tests and stories mount MessageRoomProvider (and MessageProvider where a per-message hook is exercised) alongside the legacy MessageContext.Provider, mirroring values through pickMessageRoomState during the dual-run.
MessagePreview renders the message container outside RoomView/MessagesView/SearchMessagesView, which each already mount a MessageRoomProvider. After the message container's grouping/thread/translate hooks were repointed to the room store, this path had no provider and threw 'Message room hooks must be used within a MessageRoomProvider' on render, crashing ForwardMessageView. Seed a MessageRoomProvider from the props MessagePreview already holds (user, baseUrl, getCustomEmoji, rid); the remaining room fields stay unset to preserve the legacy preview defaults. Adds a regression test rendering MessagePreview under Redux only.
Blocks reads blockAction from the room store (useBlockAction); Reply reads user/baseUrl (room store) and id/isEncrypted (per-message store) via selector hooks instead of useContext(MessageContext). Drops the dead e2e read: the message MessageContext value built in index.tsx never sets e2e, so the guard (isEncrypted && !e2e) was always equivalent to isEncrypted. This retires the last stable/per-message-data consumers; only closure handlers remain on MessageContext.
…VE-1376) Step 4a of ADR 0018. Adds the room-global row handlers (onReactionPress, onReactionLongPress, reactionInit, onDiscussionPress, onThreadPress, replyBroadcast, errorActionsShow, onAnswerButtonPress, onEncryptedPress) plus archived to MessageRoomState, ROOM_STATE_KEYS and matching selector hooks, and seeds them from RoomView's MessageRoomProvider. Purely additive: no consumer reads them yet, MessageContext still carries the closures. MessagesView and SearchMessagesView provide none of these handlers, so they stay unwired.
…IVE-1376) Step 4b of ADR 0018. Extends the per-message zustand store with onPress, onLongPress and threadBadgeColor (mirrored from MessageProvider props every render, same pattern as MessageRoomProvider) and adds the composite hooks useMessageLongPress, useMessagePress and useOnLinkPress as a faithful relocation of the closures still living in MessageContainerInner. Adds closeEmojiAndAction to the room store (it was RoomView-internal, never in MessageContext) so useMessagePress can read it. Purely additive: the new hooks have no consumers yet and MessageContainerInner still builds the legacy Context closures, so behaviour and snapshots are unchanged.
…TIVE-1376) Move the 8 remaining MessageContext consumers (Discussion, Reactions, Broadcast, AttachedActions, MessageError, Encrypted, Thread, Content) onto the room-level and per-message store selector hooks, reconstructing each handler's argument from the per-message context. Seed the per-message store's initial state from the provider's props so the first render sees onPress/onLongPress/threadBadgeColor synchronously, matching MessageRoomStore and avoiding an extra mount render. Rewire Reactions.test onto the shared MessageProviders helper for the dual-run providers.
…o store hooks (NATIVE-1376) MessageContainer now renders the provider stack directly; the deleted MessageContainerInner's press and long-press logic moves into MessageTouchable via the useMessagePress and useMessageLongPress composite hooks instead of MessageContext. Relocate the manual-unignore state and the isBeingEdited lookup into MessageTouchable, the component that consumes them. RCTouchable reads the long-press guard from useMessageLongPress. MessageContext.Provider stays mounted with an empty value that no component reads, leaving step 5 a pure deletion. Wrap two attachment test harnesses in MessageProvider now that their Touchable subtree needs it.
MessageContainer no longer reads theme after the Inner collapse; leaves already receive it through the room store's useMessageTheme selector. Removing the prop stops a theme toggle from churning memo(MessageContainer) and removes the last theme forward into the message subtree from RoomView, MessagesView, SearchMessagesView, and the Message stories.
MessageContext held no readers after row consumers moved to the room and per-message zustand stores. Remove Context.ts, the EMPTY provider in the container, and the inert legacy provider wrappers left in tests and stories; test subtrees now rely solely on MessageRoomProvider and MessageProvider.
…e (NATIVE-1376) Row consumers now read the room and per-message zustand stores, so the container no longer relays these handlers/constants. Remove them from IMessageContainerProps and stop passing them at the RoomView, MessagesView, and SearchMessagesView call sites, where the same handlers are already supplied via MessageRoomProvider.
The old deleted MessageContext returned {} outside a provider (graceful
degrade). The zustand room/message stores throw instead, so a UIKit
modal with a top-level image block crashed at useMessageUser/useMessageId/
useMessageLongPress. Fix self-provides a MessageRoomProvider and an empty
MessageProvider at ModalBlockView.
…active state - type navToRoomInfo with IRoomInfoParam (drop any) - fold view-level navToRoomInfo/showAttachment overrides into the handlers bag at provider construction so selectors read a single path - split state into FrozenState/ReactiveState; ReactiveSnapshot forces the resync effect payload to cover every reactive key
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. 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 |
…rom JoinCode Read master-detail via useMasterDetail() and take ref as a normal prop (React 19 ref-as-prop). Removes the empty connect() and withMasterDetail HOC.
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. 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 |
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. 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 |
…-hooks # Conflicts: # app/containers/message/components/__tests__/__snapshots__/Message.test.tsx.snap # app/views/RoomView/List/hooks/useScroll.ts # app/views/RoomView/index.tsx # pnpm-lock.yaml
…et.Chat.ReactNative into native-34-roomview-hooks
…et.Chat.ReactNative into native-34-roomview-hooks
…et.Chat.ReactNative into native-34-roomview-hooks
|
iOS Build Available Rocket.Chat 4.75.0.109491 |
|
iOS Build Available Rocket.Chat 4.76.0.109495 |
…ative-34-roomview-hooks
The header hooks read route.params reactively, so popTo('RoomView') replacing
the retained route's params with undefined broke the header permanently.
useHeader now takes { rid, tmid, name } from the screen's mount-time snapshots,
useGoRoomActionsView reads t from the RoomStore, and popToRoom dispatches
popTo with merge: true so params are no longer wiped at the source.
loading was per-screen state living in the per-rid shared RoomStore, so a thread's init() rewrote the room screen's flag. init() no longer writes loading; each screen owns it in useRoomInit via useState, set around an awaited init() behind a cancelled ref guard, and it reaches TakeOrJoin and OnHold as a prop through RoomFooter.
lastSeen was per-screen state living in the per-rid shared room store, so a send from the thread screen nulled the room screen's unread separator. init() now returns the value, the screen owns it and hands it to useMessageSeparators through a RoomView-local context; markMessageSent is gone and sendRoomMessage takes a callback. Also drops the dead stateAttrsUpdate/TStateAttrsUpdate pair.
useRoomInit held one cancelledRef for an unbounded number of init runs, and reset it to false at the top of every run. A new run therefore un-cancelled a previous run that was still in flight, which then resolved and wrote lastSeen and loading for a room the screen had already left. The INVITED-accepted effect reaches this, as does any rid/tmid/isAuthenticated change. init() also returned one nullable value that flattened four outcomes into null, so the caller could not tell success from failure — which is why cancellation had to live outside the store in the first place. init() now returns a discriminated TRoomInitResult and accepts an AbortSignal. Each run owns its own AbortController and aborts the previous one, so a token belongs to one run and is never reset by a later one. The store checks the signal after each await, including after the retry sleep. lastSeen is now written only on a loaded result, so a failed reload no longer clears an existing unread divider anchor.
loadRoom took the store's get and set directly, which is not an interface but a handover of the whole store. Three things followed from it. The retry never re-read the room. init snapshotted startedEmpty once and loadRoom captured the room at the top of an attempt, so when the subscription observer filled a store that started empty mid-retry, init bailed instead of retrying. Messages had never been fetched on that path, so the screen stayed empty in exactly the case the retry exists to prevent. The unread divider anchor was computed from a room captured before two awaits, and joined was read separately, so both could straddle an observer emit. getRoomMember wrote roomUserId behind the caller's back, mid-await, before getUserInfo resolved. loadRoom now takes a room snapshot and returns what it learned, including a patch and a read receipt for init to apply. init re-reads the room at the top of every attempt, which is what makes the retry pick up an observer-delivered room, and applies nothing once the run is aborted.
loading, lastSeen and clearLastSeen are all per-screen for one reason — a room and its thread mount two RoomViews over one rid-keyed store — but they travelled three different ways, and the reason was written out four times in three files. lastSeen went through LastSeenContext while loading was drilled from index.tsx down to OnHold and TakeOrJoin, so RoomFooter read its room and joined state from context but its loading from a prop. RoomScreenContext now carries all three, per RoomView instance, and states the reason once. LastSeenContext is gone. loading was useState(true) behind an effect that early-returned without a rid or without auth, so such a screen stayed loading forever and its Join button stayed disabled. It is now derived from whether an init run is actually pending, so no work pending reads as idle rather than as loading.
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. Behavior-preserving — no user-visible change intended.shouldComponentUpdate;connect(mapStateToProps)and all HOCs preserved.RoomStoreregistry (stores/RoomStore.ts): self-hydrates from the DB, shared by a room and its threads, torn down on last release;goRoomwarms it at nav time.RoomContextreplaced by a per-instance composer Zustand store (stores/ComposerStore.tsx).useRoomInit,useRoomAudioLifecycle,useRoomRemoved,useHeader,useJumpToMessage,useMessageActions,useOmnichannelPermissions,useRoomNavigation) and presentational components (MessageRow,RoomFooter,RoomMessageActions, room-state screens).'use memo'throughout so the React Compiler owns memoization.SearchMessagesViewresults are wrapped inA11yGateProviderso long-press message actions work there too — a small a11y addition riding along with the shared message-handler extraction.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-34
How to test or reproduce
Use a room end to end (send/edit/quote/react, drafts, autocomplete, threads, jump-to-message, join a not-subscribed room, header actions) on phone and tablet, plus an omnichannel/livechat room. Behavior should match 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