Page MenuHomePhabricator
Differential D10409

[lib] Memoize userinfos in usePotentialMemberItems
ClosedPublic
Actions

Authored by rohan on Dec 19 2023, 8:47 PM.
Tags
None
Referenced Files
F6281129: D10409.diff
Fri, Apr 25, 7:30 PM
Unknown Object (File)
Fri, Apr 25, 3:17 AM
Unknown Object (File)
Sat, Apr 5, 4:25 PM
Unknown Object (File)
Sat, Apr 5, 11:18 AM
Unknown Object (File)
Sat, Apr 5, 2:11 AM
Unknown Object (File)
Thu, Apr 3, 10:31 PM
Unknown Object (File)
Wed, Apr 2, 4:31 AM
Unknown Object (File)
Mon, Mar 31, 10:58 PM
View All 84 Files
Subscribers
tomek

Details

Reviewers
ashoat
michal
Commits
rCOMMf6eaff7c3230: [lib] Memoize userinfos in usePotentialMemberItems
Summary

This addresses the urgent issue outlined in ENG-6139. I should've memoized the userInfos object.

Test Plan

Since usePotentialMemberItems is used in both chat-thread-composer.react.js and add-members-modal.react.js (both also mentioned in the original bug report), I made sure that those both no longer completely freeze/crash.

Diff Detail

Repository
rCOMM Comm
Branch
fix
Lint
No Lint Coverage
Unit
No Test Coverage

Revision Contents
Changeset List

PathSize
lib/
shared/
3 lines

Diff 34868

View Options

lib/shared/search-utils.js

Show All 11 Lines
import SWMansionIcon from 'lib/components/SWMansionIcon.react.js'; import SWMansionIcon from 'lib/components/SWMansionIcon.react.js';
import { createLoadingStatusSelector } from 'lib/selectors/loading-selectors.js'; import { createLoadingStatusSelector } from 'lib/selectors/loading-selectors.js';
import { threadInfoSelector } from 'lib/selectors/thread-selectors.js'; import { threadInfoSelector } from 'lib/selectors/thread-selectors.js';
import { userStoreMentionSearchIndex } from 'lib/selectors/user-selectors.js'; import { userStoreMentionSearchIndex } from 'lib/selectors/user-selectors.js';
import { import {
getMentionTypeaheadUserSuggestions, getMentionTypeaheadUserSuggestions,
getTypeaheadRegexMatches, getTypeaheadRegexMatches,
getUserMentionsCandidates, getUserMentionsCandidates,
getMentionTypeaheadChatSuggestions,
type MentionTypeaheadSuggestionItem, type MentionTypeaheadSuggestionItem,
type TypeaheadMatchedStrings, type TypeaheadMatchedStrings,
} from 'lib/shared/mention-utils.js'; } from 'lib/shared/mention-utils.js';
import { localIDPrefix, trimMessage } from 'lib/shared/message-utils.js'; import { localIDPrefix, trimMessage } from 'lib/shared/message-utils.js';
import { import {
threadHasPermission, threadHasPermission,
viewerIsMember, viewerIsMember,
threadFrozenDueToViewerBlock, threadFrozenDueToViewerBlock,
threadActualMembers, threadActualMembers,
checkIfDefaultMembersAreVoiced, checkIfDefaultMembersAreVoiced,
useThreadChatMentionCandidates,
useThreadChatMentionSearchIndex,
} from 'lib/shared/thread-utils.js'; } from 'lib/shared/thread-utils.js';
import type { CalendarQuery } from 'lib/types/entry-types.js'; import type { CalendarQuery } from 'lib/types/entry-types.js';
import type { LoadingStatus } from 'lib/types/loading-types.js'; import type { LoadingStatus } from 'lib/types/loading-types.js';
import { messageTypes } from 'lib/types/message-types-enum.js'; import { messageTypes } from 'lib/types/message-types-enum.js';
import { threadPermissions } from 'lib/types/thread-permission-types.js'; import { threadPermissions } from 'lib/types/thread-permission-types.js';
import { import {
type ThreadInfo, type ThreadInfo,
type ClientThreadJoinRequest, type ClientThreadJoinRequest,
▲ Show 20 Lines • Show All 535 Lines • ▼ Show 20 Lines React.memo<BaseProps>(function ConnectedChatInputBar(props) {
const createThreadLoadingStatus = useSelector( const createThreadLoadingStatus = useSelector(
createThreadLoadingStatusSelector, createThreadLoadingStatusSelector,
); );
const threadCreationInProgress = createThreadLoadingStatus === 'loading'; const threadCreationInProgress = createThreadLoadingStatus === 'loading';
const calendarQuery = useSelector(nonThreadCalendarQuery); const calendarQuery = useSelector(nonThreadCalendarQuery);
const dispatchActionPromise = useDispatchActionPromise(); const dispatchActionPromise = useDispatchActionPromise();
const callJoinThread = useServerCall(joinThread); const callJoinThread = useServerCall(joinThread);
const userSearchIndex = useSelector(userStoreMentionSearchIndex); const userSearchIndex = useSelector(userStoreMentionSearchIndex);
const chatMentionSearchIndex = useThreadChatMentionSearchIndex(
props.threadInfo,
);
const { parentThreadID } = props.threadInfo; const { parentThreadID } = props.threadInfo;
const parentThreadInfo = useSelector(state => const parentThreadInfo = useSelector(state =>
parentThreadID ? threadInfoSelector(state)[parentThreadID] : null, parentThreadID ? threadInfoSelector(state)[parentThreadID] : null,
); );
const userMentionsCandidates = getUserMentionsCandidates( const userMentionsCandidates = getUserMentionsCandidates(
props.threadInfo, props.threadInfo,
parentThreadInfo, parentThreadInfo,
); );
const chatMentionCandidates = useThreadChatMentionCandidates(
props.threadInfo,
);
const typeaheadRegexMatches = React.useMemo( const typeaheadRegexMatches = React.useMemo(
() => () =>
getTypeaheadRegexMatches( getTypeaheadRegexMatches(
props.inputState.draft, props.inputState.draft,
{ {
start: props.inputState.textCursorPosition, start: props.inputState.textCursorPosition,
end: props.inputState.textCursorPosition, end: props.inputState.textCursorPosition,
}, },
Show All 14 Lines const typeaheadMatchedStrings: ?TypeaheadMatchedStrings = React.useMemo(
[typeaheadRegexMatches], [typeaheadRegexMatches],
); );
React.useEffect(() => { React.useEffect(() => {
if (props.inputState.typeaheadState.keepUpdatingThreadMembers) { if (props.inputState.typeaheadState.keepUpdatingThreadMembers) {
const setter = props.inputState.setTypeaheadState; const setter = props.inputState.setTypeaheadState;
setter({ setter({
frozenUserMentionsCandidates: userMentionsCandidates, frozenUserMentionsCandidates: userMentionsCandidates,
frozenChatMentionsCandidates: chatMentionCandidates,
}); });
} }
}, [ }, [
userMentionsCandidates, userMentionsCandidates,
props.inputState.setTypeaheadState, props.inputState.setTypeaheadState,
props.inputState.typeaheadState.keepUpdatingThreadMembers, props.inputState.typeaheadState.keepUpdatingThreadMembers,
chatMentionCandidates,
]); ]);
const suggestedUsers = React.useMemo(() => { const suggestedUsers = React.useMemo(() => {
if (!typeaheadMatchedStrings) { if (!typeaheadMatchedStrings) {
return []; return [];
} }
return getMentionTypeaheadUserSuggestions( return getMentionTypeaheadUserSuggestions(
userSearchIndex, userSearchIndex,
props.inputState.typeaheadState.frozenUserMentionsCandidates, props.inputState.typeaheadState.frozenUserMentionsCandidates,
viewerID, viewerID,
typeaheadMatchedStrings.textPrefix, typeaheadMatchedStrings.textPrefix,
); );
}, [ }, [
userSearchIndex, userSearchIndex,
props.inputState.typeaheadState.frozenUserMentionsCandidates, props.inputState.typeaheadState.frozenUserMentionsCandidates,
viewerID, viewerID,
typeaheadMatchedStrings, typeaheadMatchedStrings,
]); ]);
const suggestedChats = React.useMemo(() => {
if (!typeaheadMatchedStrings) {
return [];
}
return getMentionTypeaheadChatSuggestions(
chatMentionSearchIndex,
props.inputState.typeaheadState.frozenChatMentionsCandidates,
typeaheadMatchedStrings.textPrefix,
);
}, [
chatMentionSearchIndex,
props.inputState.typeaheadState.frozenChatMentionsCandidates,
typeaheadMatchedStrings,
]);
const suggestions = React.useMemo(() => {
if (!typeaheadMatchedStrings) {
return [];
}
return [...suggestedUsers, ...suggestedChats];
}, [suggestedUsers, suggestedChats, typeaheadMatchedStrings]);
rohanUnsubmitted
Not Done
Inline Actions

Does it make sense to merge these two together? Something like

const suggestions = React.useMemo(() => {
  if (!typeaheadMatchedStrings) {
    return [];
  }
  const suggestedUsers = getMentionTypeaheadUserSuggestions(
    userSearchIndex,
    props.inputState.typeaheadState.frozenUserMentionsCandidates,
    viewerID,
    typeaheadMatchedStrings.textPrefix,
  );

  const suggestedChats = getMentionTypeaheadChatSuggestions(
    chatMentionSearchIndex,
    props.inputState.typeaheadState.frozenChatMentionsCandidates,
    typeaheadMatchedStrings.textPrefix,
  );

  return [...suggestedUsers, ...suggestedChats];
}, [
  typeaheadMatchedStrings,
  userSearchIndex,
  props.inputState.typeaheadState.frozenUserMentionsCandidates,
  viewerID,
  chatMentionSearchIndex,
]);
rohan: Does it make sense to merge these two together? Something like ``` const suggestions =…
patrykAuthorUnsubmitted
Done
Inline Actions

Yes, my motivation to split this code into three useMemos was that if userSearchIndex changes and chatMentionSearchIndex not, then we would recalculate userSuggestions only, but the only time userSearchIndex and chatMentionSearchIndex would change is when user switches the focused chat, so my intuition was def wrong.

patryk: Yes, my motivation to split this code into three `useMemos` was that if `userSearchIndex`…
return ( return (
<ChatInputBar <ChatInputBar
{...props} {...props}
viewerID={viewerID} viewerID={viewerID}
joinThreadLoadingStatus={joinThreadLoadingStatus} joinThreadLoadingStatus={joinThreadLoadingStatus}
threadCreationInProgress={threadCreationInProgress} threadCreationInProgress={threadCreationInProgress}
calendarQuery={calendarQuery} calendarQuery={calendarQuery}
nextLocalID={nextLocalID} nextLocalID={nextLocalID}
isThreadActive={isThreadActive} isThreadActive={isThreadActive}
userInfos={userInfos} userInfos={userInfos}
dispatchActionPromise={dispatchActionPromise} dispatchActionPromise={dispatchActionPromise}
joinThread={callJoinThread} joinThread={callJoinThread}
typeaheadMatchedStrings={typeaheadMatchedStrings} typeaheadMatchedStrings={typeaheadMatchedStrings}
suggestions={suggestedUsers} suggestions={suggestions}
parentThreadInfo={parentThreadInfo} parentThreadInfo={parentThreadInfo}
/> />
); );
}); });
export default ConnectedChatInputBar; export default ConnectedChatInputBar;