chat clean#29369
Open
chrisnojima wants to merge 18 commits into
Open
Conversation
…tore Makes the inbox metadata store (useInboxMetadataState) the single owner of per-conversation participant info. The per-conversation thread store (ConversationThreadState) drops its participants field, setParticipants action, seeded-at-mount snapshot, mirror effect, and ChatParticipantsInfo listener. Readers switch to useConversationParticipants(conversationIDKey) (data-hooks.tsx) or, for the one non-hook call site, a direct getInboxConversationParticipants(id) read. The chatInboxFailed listener now writes straight to the inbox store via participantInfoReceived instead of round-tripping through the thread store. Verified no global-coverage regression: chat/inbox/engine.tsx's handleConvoEngineIncoming handles both ChatParticipantsInfo and chatInboxFailed unconditionally (no per-conversation gating), writing into useInboxMetadataState for every conversationIDKey in the RPC payload. constants/init/shared.tsx's _onEngineIncoming routes both notification types through this handler unconditionally, and it's wired once at the engine level (engine/index-impl.tsx), not per mounted provider - so the deleted per-conversation listener was fully redundant. yarn lint, yarn tsc, and yarn jest stores chat all pass (45 suites, 188 tests).
metasReceived overwrote the inbox store unconditionally; the thread
store's meta path version-gated via updateMeta. Fold that gating into
metasReceived so a stale-version update can't clobber newer data.
Add a {force} option for callers that must bypass gating:
- metaReceivedError (error metas reuse the prior inbox version but flip
trustedState to 'error', which gating would swallow)
- onChatInboxSynced incremental (authoritative unverified sync)
- updateInboxConversationMeta and the input-area draft write (both merge
from the current meta at the same version)
Gating is computed against getState() rather than the immer draft so
updateMeta never sees a proxy.
The per-conversation thread store kept its own copy of ConversationMeta, bi-directionally synced with the inbox metadata store. Remove that copy so the inbox store is the single owner: reads come from it, writes flow one way (RPC/engine -> metasReceived -> inbox store -> subscribers). Thread-context changes: - Drop `meta` from ConversationThreadState, its seeding, and the setMeta/updateMeta actions plus the applyConversationMetaToThread / applyInboxUIItemToThread helpers. - Internal reads now go through a module-local getMeta(id) (or getInboxConversationMeta for presence checks); the offline write uses updateInboxConversationMeta directly. - Add useThreadMeta(selector), a narrow inbox-store read keyed by the current thread id, and repoint every component/hook that read the thread-store meta (render-hot message rows use narrow field selectors; open-on-demand popups read the whole meta). Deleted thread-context listeners (each only refreshed the local meta copy; verified the global engine path in chat/inbox/engine.tsx already writes the inbox store, wired via constants/init/shared.tsx): - NewChatActivity setStatus/readMessage/newConversation branches and the applyInboxUIItemToThread calls in incomingMessage/failedMessage -> onNewChatActivity returns the inboxUIItem, pushed through onIncomingInboxUIItem -> hydrateInboxConversations -> metasReceived (readMessage with no conv -> forceUnboxRowsForService). - setAppNotificationSettings branch -> engine onNewChatActivity calls updateInboxConversationMeta(parseNotificationSettings). - ChatConvUpdate -> engine metasReceived. - chatInboxFailed -> engine metaReceivedError (error meta + rekey participants via participantInfoReceived; covered by engine.test.tsx 'global inbox failure routing stores error metadata and rekey participants'). - ChatSetConvSettings -> engine updateInboxConversationMeta(minWriterRole). - ChatSetConvRetention / ChatSetTeamRetention -> engine metasReceived. Tests: delete the thread-context test that asserted the removed mirroring listener (its behavior is covered by engine.test.tsx against the inbox store); teach normal/container.test.tsx to mock useThreadMeta.
Drop the state.username === username guard boilerplate in
useInboxState; inboxControls is component-local state, so resetting
it on user switch is handled by remounting InboxBody via key={username}
at its render sites instead of threading username through every
setInboxControls updater.
Traced all three pickKey flows (addAlias, chatInput, reaction): desktop already renders EmojiPicker in-tree inside a popup and hands picks back via a plain onPickAction callback, bypassing the store entirely. Mobile opens the picker as a separate routed screen (chatChooseEmoji), so a callback can't cross the nav-params boundary (serializable only) - keeping the store there is correct. Both mobile consumers (addAlias, chatInput) already clear their key with updatePickerMap(key, undefined) right after reading it, so the handoff is already self-cleaning; added a file-header comment documenting the pattern so the two-path split (callback on desktop, mailbox on mobile) isn't rediscovered from scratch. The 'reaction' pickKey is written by the picker route but never read back - the reaction toggle happens directly inside the picker via onPickAddToMessageID/conversationIDKey route params - a pre-existing dead write left untouched as out of scope for this pass.
Audited three never-explicitly-reset module-level stores for logout survival: - header-portal-state: portalNode clears via its ref callback firing null on unmount, portalContent clears in an unmount effect cleanup; both owning components only live in the logged-in chat tree, so logout unmounts them and clears this state as a side effect. Left as-is, documented the invariant. - block-buttons-state: moved loadGeneration from a module let into the store as a plain number field so resetState's existing dispatch path covers it; loadPromise stays module-level (renamed to activeLoadPromise) since promises can't live in immer state, with a comment tying it to resetState's generation bump so a load in flight at reset time can't win the race. - shared-timers: observers are added/removed in message row effect cleanups, and logout unmounts every message row, so the timer/ref maps always drain on logout. No code change, documented the invariant.
useMessageData ran per mounted message row and pulled participants via useConversationParticipants, which registers ~6 engine action listeners and fires an unboxRows mount effect per row. Read directly from useInboxMetadataState instead; singleton call sites (special-top-message, bottom-banner, reset-user, popups) keep the reload-driving hook.
… shared thread helpers, typing map growth)
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.
No description provided.