[web] Update AddUsersList to call useUserSearchIndex
ClosedPublic
Actions

Authored by rohan on Dec 8 2023, 5:38 AM.

Details

Reviewers

atul
ginsu
tomek

Commits

rCOMM275fee613bc1: [web] Update AddUsersList to call useUserSearchIndex

Summary

There are several search experiences in the app that do not use get/usePotentialMemberItems. This means we need to update them individually, so I'm separating out diffs to do exactly this to make it easy for reviewers. [[ https://linear.app/comm/issue/ENG-6019/[after-landing]-try-to-update-remaining-search-experiences-to-use | ENG-6019 ]] tracks attempting to update these search experiences to use usePotentialMemberItems. Each diff will affect one file ideally.

This diff affects AddUsersList.

This component is used when adding friends from friend list on web

Addresses ENG-5434

Depends on D10252

Test Plan

Please see the video below where I add friends from the friend list on web

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Dec 8 2023, 5:38 AM

Herald added a subscriber: ashoat. · View Herald TranscriptDec 8 2023, 5:38 AM

rohan edited the test plan for this revision. (Show Details)Dec 8 2023, 5:39 AM

rohan added a child revision: D10254: [lib] Deprecate userSearchIndexForPotentialMembers and userStoreSearchIndex.Dec 8 2023, 5:41 AM

rohan published this revision for review.Dec 8 2023, 6:15 AM

Harbormaster completed remote builds in B24890: Diff 34420.Dec 8 2023, 6:35 AM

ashoat added inline comments.Dec 11 2023, 9:38 AM

web/settings/relationship/add-users-list.react.js
61 ↗	(On Diff #34420)	There is a very subtle (but dangerous) flaw here You're creating a new collection on every invocation of `values`. That means that `useUserSearchIndex` will need to generate a brand new search index on every render of this component. This will definitely lead to a perf regression, and can even lead to a crash like ENG-4605 (fixed in D8760)

rohan edited the test plan for this revision. (Show Details)Dec 11 2023, 10:00 AM

rohan added inline comments.Dec 11 2023, 10:07 AM

web/settings/relationship/add-users-list.react.js
61 ↗	(On Diff #34420)	Ah yeah you're definitely right. I tested this and a new index is getting generated each time the component re-renders. Thanks for catching this!