-
Notifications
You must be signed in to change notification settings - Fork 3.7k
83207: debounce autocomplete query and memoize search options #86925
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
base: main
Are you sure you want to change the base?
Changes from all commits
e676af6
3bbdbca
4a55f1a
9b669fe
11e34d5
0c69c3e
3e4067c
4e8ba5a
c4c2f9a
0648ce9
8e65cd2
8d71e2c
e1d2ccc
81ffc89
c447ddc
e5e514d
4bbbec9
429042f
e66d380
f246760
59bbc7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -214,7 +214,7 @@ | |
| */ | ||
|
|
||
| let allReports: OnyxCollection<Report>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -230,7 +230,7 @@ | |
| const deprecatedCachedOneTransactionThreadReportIDs: Record<string, string | undefined> = {}; | ||
| /** @deprecated Use sortedReportActionsData from ONYXKEYS.DERIVED.RAM_ONLY_SORTED_REPORT_ACTIONS instead. Will be removed once all flows are migrated. */ | ||
| let deprecatedAllReportActions: OnyxCollection<ReportActions>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, | ||
| waitForCollectionCallback: true, | ||
| callback: (actions) => { | ||
|
|
@@ -280,7 +280,7 @@ | |
| }); | ||
|
|
||
| let activePolicyID: OnyxEntry<string>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.NVP_ACTIVE_POLICY_ID, | ||
| callback: (value) => (activePolicyID = value), | ||
| }); | ||
|
|
@@ -507,18 +507,15 @@ | |
| * Searches for a match when provided with a value | ||
| */ | ||
| function isSearchStringMatch(searchValue: string, searchText?: string | null, participantNames = new Set<string>(), isReportChatRoom = false): boolean { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please explain changes in this function?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aimane-chnaif This change is mainly a perf cleanup in a hot path, not intended to change behavior. Before, What I changed:
So matching logic stays the same, but we avoid repeated regex allocations and reduce JS work during search filtering.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, please add unit test (not perf-test) for this function
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aimane-chnaif Added unit test in |
||
| const searchWords = new Set(searchValue.replaceAll(',', ' ').split(/\s+/)); | ||
| const searchWords = Array.from(new Set(searchValue.replaceAll(',', ' ').split(/\s+/).filter(Boolean))); | ||
| const valueToSearch = searchText?.replaceAll(new RegExp(/ /g), ''); | ||
| let matching = true; | ||
| for (const word of searchWords) { | ||
| // if one of the word is not matching, we don't need to check further | ||
| if (!matching) { | ||
| continue; | ||
| const compiledRegexes = searchWords.map((word) => ({word, regex: new RegExp(Str.escapeForRegExp(word), 'i')})); | ||
| for (const {word, regex} of compiledRegexes) { | ||
| if (!(regex.test(valueToSearch ?? '') || (!isReportChatRoom && participantNames.has(word)))) { | ||
| return false; | ||
| } | ||
| const matchRegex = new RegExp(Str.escapeForRegExp(word), 'i'); | ||
| matching = matchRegex.test(valueToSearch ?? '') || (!isReportChatRoom && participantNames.has(word)); | ||
| } | ||
| return matching; | ||
| return true; | ||
| } | ||
|
|
||
| function isSearchStringMatchUserDetails(personalDetail: PersonalDetails, searchValue: string) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.