refactor: migrate RoomView to a function component - #7482
Open
diegolmello wants to merge 319 commits into
Open
Conversation
…essageRoomStore's
Add 'use memo' so React Compiler (annotation mode) returns a stable closure when inputs are unchanged, instead of a fresh arrow each call.
- hideSystemMessages returns stable refs (model field / redux prop / shared empty) instead of a fresh [] each render, so the message-list WatermelonDB query stops re-subscribing on every RoomView render - RoomView.shouldComponentUpdate ignores lastMessage for non-livechat rooms (livechat still uses it for on-hold header updates), so a new message no longer forces a full RoomView re-render
A manually revealed ignored message stayed revealed forever because the manual-reveal flag was never reset. It now resets when the isIgnored prop transitions, so the message re-hides; a benign re-render with the same value keeps the reveal. This rides on converting MessageStore to a store-only context, matching the MessageRoomStore and InteractionStore siblings: item, previousItem and isIgnored move into the zustand store state, the context carries only the store ref, field/domain/derived hooks read state inside their selectors via a private useMessageStore helper, reveal is a stable action built in the store initializer, and useMessageCtx is retired in favour of useMessageItem.
Reordering or editing quoted attachments reused the wrong Reply instance by raw index, carrying a stale loading state onto a different attachment. Key by the stable-identity expression the sibling Attachments.tsx already uses (title_link || message_link, index only as fallback).
The display msg was snapshotted from isEncrypted/tmsg into useState at mount and a []-deps effect overwrote it, so it went stale when the model updated and missed later tmid/id changes (the captured fetch closed over the initial ids). Derive the display value during render, keep state only for the fetched thread name, and depend the fetch effect on tmid/id/ displayMsg so it re-runs on change.
Status was written through scattered setStatus literals across five handlers and onPress re-branched on the status string, duplicating the transition table. Extract a downloadStatusReducer over named events (download_started/succeeded/failed/canceled, cache_hit); handlers dispatch events and onPress derives its action from state. Behavior unchanged.
- Split into a base MessageRoomStoreProvider (no useSetting) and a WithSetting variant, dispatched on whether a timeFormat prop is passed. Callers that pass timeFormat (MessagesView, SearchMessagesView) no longer subscribe to or re-render on Message_TimeFormat changes. Public MessageRoomProvider name and prop contract are unchanged. - Guard the props->store mirror effect with a shallow diff against the store's current state (keys derived from the state object, no hand-maintained key list), so setState fires only on a real change instead of every render. Reactive props still propagate.
The 233b131 rework moved the fetchThreadName call into an effect above the !tmid early return, so tmid (string | undefined) was no longer narrowed to string at the call site, breaking the type check. Guard the effect on tmid as well — the render path already returns null when tmid is absent, so fetching in that case was a no-op.
InteractionStore held interaction as two flat fields (action + selectedMessages), so invalid combos (edit with many selected ids, action out of sync with the selection) were representable and only prevented by call-site discipline. Replace them with a single discriminated union — edit/quote/react/null — and event-style action creators, making invalid states unrepresentable at the type level. Public selectors (useMessageAction, useSelectedMessages, useIsBeingEdited) are preserved by deriving from the union (useSelectedMessages via useShallow, since edit/react derive a fresh single-element array), so the composer consumers need no changes. RoomView getState() reads and createInteractionStore initializers updated to the union. ShareView drops its duplicate action/selectedMessages state; send(), onRemoveQuoteMessage and componentWillUnmount now read the interaction from the store instead of this.state.
The 233b131 change made the thread-name fetch effect re-runnable (deps went from [] to tmid/id/displayMsg/fetchThreadName). Add an ignore flag cleared on cleanup so a slow in-flight fetch can no longer overwrite the name after the inputs change.
Delivers the valid managing-state review findings as per-ticket commits: 001 Quote reply key from index to stable identity; 002 RepliedThread display derived in render + cancellable fetch; 003 media auto-download via reducer; 004 MessageRoomStore split (base vs WithSetting) + diff-guarded mirror; 005 InteractionStore discriminated union + ShareView reads the store; 006 re-hide a revealed ignored message on ignore-state transition.
…er snapshots (NATIVE-22)
…e leaves (NATIVE-22)
…n ReactionsList (NATIVE-22)
…e components (NATIVE-22)
…-hooks-3 # Conflicts: # CONTEXT.md # app/containers/message/hooks/useMediaAutoDownload.tsx # app/lib/methods/helpers/isReadOnly.ts
10 tasks
10 tasks
10 tasks
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