refactor: split SearchContext into narrow Query, Results, and Selection providers#91303
Conversation
…-context # Conflicts: # Mobile-Expensify
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@mananjadhav Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppSimulator.Screen.Recording.-.iPhone.15.Pro.-.2026-05-22.at.21.30.58.moviOS: mWeb SafariSimulator.Screen.Recording.-.iPhone.15.Pro.-.2026-05-22.at.21.33.09.movMacOS: Chrome / SafariScreen.Recording.2026-05-22.at.19.35.22.mov |
This comment was marked as outdated.
This comment was marked as outdated.
|
@MelvinBot review |
Code ReviewWell-structured refactor. The split into three narrow providers (Query, Results, Selection) is a solid approach to reducing unnecessary re-renders. The circular dependency handling via Architecture looks good:
Issues to address:
Minor nits (non-blocking):
Overall this is a well-executed refactor with clear motivation and good test coverage. The main actionable feedback is around memoizing context value objects (point 3) — without that, the re-render reduction benefit is partially lost. |
I think this point makes sense, we can dedupe the defined hooks from provider files. |
…-context # Conflicts: # config/eslint/eslint.seatbelt.tsv # src/components/MoneyReportHeaderPrimaryAction/SubmitPrimaryAction.tsx
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.81-0 🚀
|
|
After reviewing the changes in this PR, no help site documentation updates are required. This PR is a pure internal code refactoring that splits |
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.3.81-2 🚀
|
Explanation of Change
The monolithic
SearchContextwas serving three unrelated concerns (query state, results/data, and selection state) through a single context object. Any component consuming even one field caused re-renders on all three axes of change.This refactor splits
SearchContextinto three narrow providers:SearchQueryProvider— owns query string and filter stateSearchResultsProvider— owns results data, loading state, and sort configSearchSelectionProvider— owns selection set and bulk-action stateAll 90+ consumers across components, hooks, pages, and tests are migrated to import from the matching narrow hook (
useSearchQuery,useSearchResults,useSearchSelection). The oldSearchContextshim and its re-exports are removed. A newSearchContextDefinitions.tsfile holds shared type definitions to avoid circular imports.The public API of the
Searchcomponent is unchanged. Tests are updated to use the newMockSearchContextProviderutility.Since the PR looks pretty big, I prepared a list of files that are must-review, all the rest is just a swap for one of the three narrow hooks, a spot-check of few is enough:
Architecture — start here (new providers + the split)
These are the heart of the refactor. If these are right, the rest is mostly grep-and-replace.
src/components/Search/SearchContextDefinitions.ts— bare context objects + defaults. Extracted to break the circular import between providers and useOnyx.src/components/Search/SearchQueryProvider.tsx— owns currentSearchHash, currentSearchKey, query JSON, shouldResetSearchQuery.src/components/Search/SearchResultsProvider.tsx— owns snapshot/live-data switching (shouldUseLiveData), sortedReportIDs, filter-bar loading.src/components/Search/SearchSelectionProvider.tsx— owns selectedTransactions/selectedReports, contains the atomic useSyncSelectedReports hook and deriveSelectedReports helper.src/components/Search/SearchContext.tsx— thin shim: composes the 3 providers and re-exports the narrow hooks for the public API.src/components/Search/types.ts— adds per-bucket types (SearchQuery/Results/SelectionContextValue + …ActionsValue); keeps SearchStateContextValue & SearchActionsContextValue as composed unions.Cross-bucket consumer (worth a careful read)
src/hooks/useOnyx.ts— only call-site that reads from two buckets at once (SearchQueryContext + SearchResultsContext). Pattern reference for any future cross-bucket consumers.The orchestrator using all three buckets
src/components/Search/index.tsx— heaviest consumer; now callsuseSearchQueryContext,useSearchResultsContext,useSearchSelectionContext+ their actions separately. Worth checking the imports / dep arrays.Tests
tests/utils/MockSearchContextProvider.tsx(new) — test harness mirroring the new 3-provider tree.tests/unit/Search/useSyncSelectedReportsTest.tsx(new) — covers the atomic-sync behavior.Fixed Issues
$ #91423
PROPOSAL:
Tests
Search query & results
Filters
Selection
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mov