Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: remove getSearchText function #46409

Merged
5 changes: 5 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,11 @@ const CONST = {
// It's copied here so that the same regex pattern can be used in form validations to be consistent with the server.
VALIDATE_FOR_HTML_TAG_REGEX: /<([^>\s]+)(?:[^>]*?)>/g,

// The regex below is used to remove dots only from the local part of the user email (local-part@domain)
// so when we are using search, we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain)
// More info https://github.com/Expensify/App/issues/8007
EMAIL_SEARCH_REGEX: /\.(?=[^\s@]*@)/g,

VALIDATE_FOR_LEADINGSPACES_HTML_TAG_REGEX: /<([\s]+.+[\s]*)>/g,

WHITELISTED_TAGS: [/<>/, /< >/, /<->/, /<-->/, /<br>/, /<br\/>/],
Expand Down
106 changes: 17 additions & 89 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -512,50 +512,6 @@ function uniqFast(items: string[]): string[] {
return result;
}

/**
* Returns a string with all relevant search terms.
*
* This method must be incredibly performant. It was found to be a big performance bottleneck
* when dealing with accounts that have thousands of reports. For loops are more efficient than _.each
* Array.prototype.push.apply is faster than using the spread operator.
*/
function getSearchText(
report: OnyxInputOrEntry<Report>,
reportName: string,
personalDetailList: Array<Partial<PersonalDetails>>,
isChatRoomOrPolicyExpenseChat: boolean,
isThread: boolean,
): string {
const searchTerms: string[] = [];

for (const personalDetail of personalDetailList) {
if (personalDetail.login) {
// The regex below is used to remove dots only from the local part of the user email (local-part@domain)
// so that we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain)
// More info https://github.com/Expensify/App/issues/8007
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to link this old issue anymore?

searchTerms.push(PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail, '', false), personalDetail.login, personalDetail.login.replace(/\.(?=[^\s@]*@)/g, ''));
}
}

if (report) {
Array.prototype.push.apply(searchTerms, reportName.split(/[,\s]/));

if (isThread) {
const title = ReportUtils.getReportName(report);
const chatRoomSubtitle = ReportUtils.getChatRoomSubtitle(report);

Array.prototype.push.apply(searchTerms, title.split(/[,\s]/));
Array.prototype.push.apply(searchTerms, chatRoomSubtitle?.split(/[,\s]/) ?? ['']);
} else if (isChatRoomOrPolicyExpenseChat) {
const chatRoomSubtitle = ReportUtils.getChatRoomSubtitle(report);

Array.prototype.push.apply(searchTerms, chatRoomSubtitle?.split(/[,\s]/) ?? ['']);
}
Comment on lines -543 to -553
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had made threads searchable in #19117.
Please make sure that this removal doesn't cause regression.

}

return uniqFast(searchTerms).join(' ');
}

/**
* Get an object of error messages keyed by microtime by combining all error objects related to the report.
*/
Expand Down Expand Up @@ -790,7 +746,6 @@ function createOption(
phoneNumber: undefined,
hasDraftComment: false,
keyForList: undefined,
searchText: undefined,
isDefaultRoom: false,
isPinned: false,
isWaitingOnBankAccount: false,
Expand Down Expand Up @@ -887,9 +842,6 @@ function createOption(
}

result.text = reportName;
// Disabling this line for safeness as nullish coalescing works only if the value is undefined or null
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
result.searchText = getSearchText(report, reportName, personalDetailList, !!result.isChatRoom || !!result.isPolicyExpenseChat, !!result.isThread);
result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID, null);
result.subtitle = subtitle;

Expand Down Expand Up @@ -2053,22 +2005,6 @@ function getOptions(
continue;
}

// Finally check to see if this option is a match for the provided search string if we have one
const {searchText, participantsList, isChatRoom} = reportOption;
const participantNames = getParticipantNames(participantsList);

if (searchValue) {
// Determine if the search is happening within a chat room and starts with the report ID
const isReportIdSearch = isChatRoom && Str.startsWith(reportOption.reportID ?? '-1', searchValue);

// Check if the search string matches the search text or participant names considering the type of the room
const isSearchMatch = isSearchStringMatch(searchValue, searchText, participantNames, isChatRoom);

if (!isReportIdSearch && !isSearchMatch) {
continue;
}
}

reportOption.isSelected = isReportSelected(reportOption, selectedOptions);

if (action === CONST.IOU.ACTION.CATEGORIZE) {
Expand All @@ -2093,19 +2029,11 @@ function getOptions(
if (personalDetailsOptionsToExclude.some((optionToExclude) => optionToExclude.login === personalDetailOption.login)) {
return;
}
const {searchText, participantsList, isChatRoom} = personalDetailOption;
const participantNames = getParticipantNames(participantsList);
if (searchValue && !isSearchStringMatch(searchValue, searchText, participantNames, isChatRoom)) {
return;
}

personalDetailsOptions.push(personalDetailOption);
});

let currentUserOption = allPersonalDetailsOptions.find((personalDetailsOption) => personalDetailsOption.login === currentUserLogin);
if (searchValue && currentUserOption && !isSearchStringMatch(searchValue, currentUserOption.searchText)) {
currentUserOption = undefined;
}
const currentUserOption = allPersonalDetailsOptions.find((personalDetailsOption) => personalDetailsOption.login === currentUserLogin);

let userToInvite: ReportUtils.OptionData | null = null;
if (
Expand Down Expand Up @@ -2438,11 +2366,12 @@ function formatSectionsFromSearchTerm(
};
}

const cleanSearchTerm = searchTerm.trim().toLowerCase();
// If you select a new user you don't have a contact for, they won't get returned as part of a recent report or personal details
// This will add them to the list of options, deduping them if they already exist in the other lists
const selectedParticipantsWithoutDetails = selectedOptions.filter((participant) => {
const accountID = participant.accountID ?? null;
const isPartOfSearchTerm = participant.searchText?.toLowerCase().includes(searchTerm.trim().toLowerCase());
const isPartOfSearchTerm = getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(cleanSearchTerm);
const isReportInRecentReports = filteredRecentReports.some((report) => report.accountID === accountID);
const isReportInPersonalDetails = filteredPersonalDetails.some((personalDetail) => personalDetail.accountID === accountID);
return isPartOfSearchTerm && !isReportInRecentReports && !isReportInPersonalDetails;
Expand Down Expand Up @@ -2474,6 +2403,14 @@ function getFirstKeyForList(data?: Option[] | null) {

return firstNonEmptyDataObj.keyForList ? firstNonEmptyDataObj.keyForList : '';
}

function getPersonalDetailSearchTerms(item: Partial<ReportUtils.OptionData>) {
return [item.participantsList?.[0]?.displayName ?? '', item.login ?? '', item.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? ''];
}

function getCurrentUserSearchTerms(item: ReportUtils.OptionData) {
return [item.text ?? '', item.login ?? '', item.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? ''];
}
/**
* Filters options based on the search input value
*/
Expand All @@ -2495,10 +2432,6 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt
const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase();
const searchTerms = searchValue ? searchValue.split(' ') : [];

// The regex below is used to remove dots only from the local part of the user email (local-part@domain)
// so that we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain)
const emailRegex = /\.(?=[^\s@]*@)/g;

const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}];

excludeLogins.forEach((login) => {
Expand All @@ -2518,7 +2451,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt

if (login) {
keys.push(login);
keys.push(login.replace(emailRegex, ''));
keys.push(login.replace(CONST.EMAIL_SEARCH_REGEX, ''));
}
});
}
Expand All @@ -2534,7 +2467,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt

if (item.login) {
values.push(item.login);
values.push(item.login.replace(emailRegex, ''));
values.push(item.login.replace(CONST.EMAIL_SEARCH_REGEX, ''));
}

if (item.isThread) {
Expand All @@ -2560,15 +2493,9 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt

return uniqFast(values);
});
const personalDetails = filterArrayByMatch(items.personalDetails, term, (item) =>
uniqFast([item.participantsList?.[0]?.displayName ?? '', item.login ?? '', item.login?.replace(emailRegex, '') ?? '']),
);
const personalDetails = filterArrayByMatch(items.personalDetails, term, (item) => uniqFast(getPersonalDetailSearchTerms(item)));

const currentUserOptionSearchText = uniqFast([
items.currentUserOption?.text ?? '',
items.currentUserOption?.login ?? '',
items.currentUserOption?.login?.replace(emailRegex, '') ?? '',
]).join(' ');
const currentUserOptionSearchText = items.currentUserOption ? uniqFast(getCurrentUserSearchTerms(items.currentUserOption)).join(' ') : '';

const currentUserOption = isSearchStringMatch(term, currentUserOptionSearchText) ? items.currentUserOption : null;

Expand Down Expand Up @@ -2635,7 +2562,6 @@ export {
getSearchValueForPhoneOrEmail,
getPersonalDetailsForAccountIDs,
getIOUConfirmationOptionsFromPayeePersonalDetail,
getSearchText,
isSearchStringMatchUserDetails,
getAllReportErrors,
getPolicyExpenseReportOption,
Expand Down Expand Up @@ -2664,6 +2590,8 @@ export {
canCreateOptimisticPersonalDetailOption,
getUserToInviteOption,
shouldShowViolations,
getPersonalDetailSearchTerms,
getCurrentUserSearchTerms,
};

export type {MemberForList, CategorySection, CategoryTreeSection, Options, OptionList, SearchOption, PayeePersonalDetails, Category, Tax, TaxRatesOption, Option, OptionTree};
1 change: 0 additions & 1 deletion src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,6 @@ function getOptionData({
result.participantsList = participantPersonalDetailList;

result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID, policy);
result.searchText = OptionsListUtils.getSearchText(report, reportName, participantPersonalDetailList, result.isChatRoom || result.isPolicyExpenseChat, result.isThread);
result.displayNamesWithTooltips = displayNamesWithTooltips;

if (status) {
Expand Down
20 changes: 9 additions & 11 deletions src/pages/NewChatPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,15 @@ function useOptions({isGroupChat}: NewChatPageProps) {

return filteredOptions;
}, [debouncedSearchTerm, defaultOptions, isGroupChat, selectedOptions]);

const headerMessage = useMemo(
() =>
OptionsListUtils.getHeaderMessage(
options.personalDetails.length + options.recentReports.length !== 0,
!!options.userToInvite,
debouncedSearchTerm.trim(),
selectedOptions.some((participant) => participant?.searchText?.toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())),
),
[debouncedSearchTerm, options.personalDetails.length, options.recentReports.length, options.userToInvite, selectedOptions],
);
const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]);
const headerMessage = useMemo(() => {
return OptionsListUtils.getHeaderMessage(
options.personalDetails.length + options.recentReports.length !== 0,
!!options.userToInvite,
debouncedSearchTerm.trim(),
selectedOptions.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase?.().includes(cleanSearchTerm)),
);
}, [cleanSearchTerm, debouncedSearchTerm, options.personalDetails.length, options.recentReports.length, options.userToInvite, selectedOptions]);

useEffect(() => {
if (!debouncedSearchTerm.length) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
shouldInitialize: didScreenTransitionEnd,
});

const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]);
const offlineMessage: string = isOffline ? `${translate('common.youAppearToBeOffline')} ${translate('search.resultsAreLimited')}` : '';

const isIOUSplit = iouType === CONST.IOU.TYPE.SPLIT;
Expand Down Expand Up @@ -216,7 +217,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
(chatOptions.personalDetails ?? []).length + (chatOptions.recentReports ?? []).length !== 0,
!!chatOptions?.userToInvite,
debouncedSearchTerm.trim(),
participants.some((participant) => participant?.searchText?.toLowerCase().includes(debouncedSearchTerm.trim().toLowerCase())),
participants.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(cleanSearchTerm)),
);

return [newSections, headerMessage];
Expand All @@ -230,6 +231,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
chatOptions.userToInvite,
personalDetails,
translate,
cleanSearchTerm,
]);

/**
Expand Down
Loading
Loading