Repository navigation
fix: Search page re-fires a first page that is already refetching - #106685
TMisiukiewicz wants to merge 1 commit into
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 366913cc94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Pagination must not restart page one. | ||
| // First open of a preloaded tab refreshes its cached results and saves the recent search, with no loading state over them. | ||
| if (isSnapshotDataLoaded && !isInitialSearchPending && !isFirstOpenOfPreloadedTab) { | ||
| shownHashRef.current = hash; |
There was a problem hiding this comment.
Avoid marking background snapshots as shown
On native, background tabs keep rendering (src/libs/Navigation/AppNavigator/FreezeWrapper/index.native.tsx). If a refetch finishes after leaving Spend, this assignment repopulates shownHashRef while another tab is active, undoing the blur cleanup. Opening the same query during Home's next unflagged refresh then satisfies the new skip guard, so the page never sends the save-recent request for that visit. The Search-internal effect also skips while loading and deliberately does not rerun on focus. Only mark the hash as shown while Search is the topmost fullscreen route, preserving the RHP case without treating an offscreen snapshot update as a user visit.
Useful? React with 👍 / 👎.
Explanation of Change
Opening an expense from Spend (or editing it in the RHP) sent two
Searchrequests for the same query: an unflagged refetch, then a copy withshouldSaveRecentSearch: true.The refetch (from
useSearchAutoRefetch,refreshSearchAfterReportActionor the totals retry) marks the snapshot pending. That reopens the guard inuseSearchPageSetup, so the page fires its own flagged request.search()sees the running request and, because only the new one has the save flag, re-sends the whole query once the first one finishes. On large accounts each response blocks the JS thread for seconds, so every edit froze the app twice.useSearchPageSetupnow skips its request when all of these hold:A user arriving on a query that another caller (for example a Home row) is refreshing still sends the flagged request, so it is still saved to recent searches. A not-yet-loaded first page still goes through
search()and claims the page for Search.search()itself is unchanged. The newisFirstPageSearchInFlightmirrors the totals, footer breakdown and trip-rooms checkssearch()uses to decide on a re-fire, and leaves out the save-flag one, which is the duplicate this PR removes.Fixed Issues
$ #106777
PROPOSAL:
Tests
SearchSearchrequest goes out for the open query (samehash,offset: 0)Searchrequest goes out for the open querySearchrequest withshouldSaveRecentSearch: truegoes out for that query and it appears in recent searchesshouldSaveRecentSearch: trueOffline tests
Same as before: going offline and back on Spend refetches the open query once.
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
🤖 Generated with Claude Code