[lib] Introduce chat mention context
ClosedPublic
Actions

Authored by tomek on Oct 12 2023, 7:13 AM.

Details

Reviewers

kamil
inka
atul
ashoat

Commits

rCOMM5011d6a78baf: [lib] Introduce chat mention context

Summary

In the previous implementation, we were correctly memoizing computations, but it was done using hooks, so in each component where these were used, we were doing the same computations again. In this approach, the memoization is moved to a new context, so that computations aren't repeated.

Depends on D9465

https://linear.app/comm/issue/ENG-5224/ashoats-js-thread-freezes-on-app-start-on-build-268

Test Plan

Check if getChatMentionCandidates function invocations happen every time with new inputs (by adding code that saves the input to a global variable, compares new arguments with the previous ones, and console logs the result).
Check if chat mentioning works correctly on the web and native.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

tomek created this revision.Oct 12 2023, 7:13 AM

Herald added a subscriber: will. · View Herald TranscriptOct 12 2023, 7:13 AM

Harbormaster completed remote builds in B23212: Diff 31965.Oct 12 2023, 7:48 AM

tomek requested review of this revision.Oct 12 2023, 7:48 AM

This diff needs more work before it can be reviewed, but seems to fix (at least a part of) the performance issue.

Clean up the code

tomek edited the summary of this revision. (Show Details)Oct 13 2023, 9:34 AM

tomek edited the test plan for this revision. (Show Details)

tomek added inline comments.Oct 13 2023, 9:42 AM

lib/components/chat-mention-provider.react.js
44 ↗	(On Diff #31994)	This replaces `useThreadChatMentionSearchIndex` - it returns stable indexes which don't need to be memoized
74 ↗	(On Diff #31994)	Moved from `thread-utils` - it isn't used anywhere else
179 ↗	(On Diff #31994)	Also moved from `thread-utils`
198–200 ↗	(On Diff #31994)	Moved from `thread-utils` with a slight difference: now it takes `chatMentionCandidatesObj` as an argument instead of creating a new value by using a hook. This change makes it possible to call `useChatMentionCandidatesObjAndUtils` in only one place.
lib/hooks/chat-mention-hooks.js
26–27 ↗	(On Diff #31994)	Use global value from a context

Harbormaster completed remote builds in B23238: Diff 31994.Oct 13 2023, 9:52 AM

ashoat accepted this revision.Oct 16 2023, 5:54 PM

ashoat added inline comments.

lib/hooks/chat-mention-hooks.js

29–40 ↗

(On Diff #31994)

I know this code is just getting moved, but I think this is more readable:

const communityID =
  threadInfo.community === genesis.id
    ? communityThreadIDForGenesisThreads[threadInfo.id]
    : (threadInfo.community ?? threadInfo.id);
const allChatsWithinCommunity = chatMentionCandidatesObj[communityID];
if (!allChatsWithinCommunity) {
  return undefined;
}
const { [threadInfo.id]: _, ...result } = allChatsWithinCommunity;
return result;