[CP Staging] Re-fire search on Spend tab focus after cache clear#88994
[CP Staging] Re-fire search on Spend tab focus after cache clear#88994luacmartins merged 18 commits intomainfrom
Conversation
The TabNavigator introduced in #85234 keeps the Search screen mounted with freezeOnBlur. After "Clear cache and restart", Onyx snapshot data is wiped but the useEffect deps [hash, isOffline, shouldUseLiveData, queryJSON] don't change, so search() never re-fires. Adding a useFocusEffect ensures search() runs when the tab regains focus and data is missing. Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
Move ref assignments into useEffect to avoid accessing refs during render, which violates React rules and breaks React Compiler. Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the React Compiler compliance failure: moved ref assignments from render phase into |
|
@MelvinBot don't change anything else, just add the useFocus callback. Additionally, remove any refs added in this PR |
Per review feedback: restore the original inline useEffect for firing search (without refs), and add a separate useFocusEffect callback to re-fire search when the Spend tab regains focus after cache clear. Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Done — removed all refs and their sync |
The fireSearchOnFocus callback included currentSearchResults, currentSearchKey, and shouldCalculateTotals in its useCallback deps. When search() fires and updates currentSearchResults via Onyx, the callback identity changes, causing useFocusEffect to re-run while focused — creating an infinite loop that exhausts JS heap memory. Use refs (synced via useEffect) for these values so the callback can read them without depending on them, matching the pattern of the existing useEffect that intentionally omits these deps. Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the failing The |
|
The This PR only modifies The only other failing check is |
Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the failing Prettier check: added a missing semicolon in |
|
The failing Analysis: The error is in Evidence: This PR only modifies The fix for this typecheck error belongs in a follow-up to #78995, not in this PR. Re-running CI after that fix lands on main should make this check pass. |
Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Both failing checks are unrelated to this PR's changes (only
Pushed an empty commit to trigger a fresh CI run against the updated |
…loop The inline function passed to useFocusEffect created a new reference on every render, causing the effect to re-fire continuously (render → search → Onyx update → re-render → new callback → search again), leading to OOM in SearchPageTest. Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the failing Root cause: The Fix: Wrapped the callback in |
Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the failing React Compiler Compliance check: removed an unnecessary |
…n SearchPageTest The useEffect that fires search() had currentSearchResults added to its dependency array, creating an infinite loop: search() updates results in Onyx, which triggers the effect again. Also guard the useFocusEffect search() call with isSearchDataLoaded so it only re-fires when data is actually missing (e.g. after cache clear). Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the failing
Fix: Restored the original deps array |
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Co-authored-by: Carlos Martins <luacmartins@users.noreply.github.com>
|
Fixed the failing Prettier check: ran prettier on |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-mweb.movAndroid: mWeb Chromeandroid-mweb.moviOS: HybridAppios.moviOS: mWeb Safariios-mweb.movMacOS: Chrome / Safariweb.mov |
|
The Evidence: The error occurs in the Fix: Re-run the failed |
|
The Action needed: Re-run the |
| }, []); | ||
| }); | ||
|
|
||
| useFocusEffect(() => { |
There was a problem hiding this comment.
Is this focus effect needed? Looking at the initial useEffect, wouldn't the issue be solved by adding isSnapshotDataLoaded, isSnapshotSearchLoading to the useEffect deps? Since those depend on the data, so if its missing, the effect will run again?
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
…adAfterReset [CP Staging] Re-fire search on Spend tab focus after cache clear (cherry picked from commit b77d56b) (cherry-picked to staging by arosiclair)
|
🚧 @luacmartins 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! 🧪🧪
|
|
🚀 Cherry-picked to staging by https://github.com/arosiclair in version: 9.3.64-18 🚀
Bundle Size Analysis (Sentry): |
|
I reviewed the changes in this PR. It modifies internal React hook logic in
These are purely internal bug fixes with no user-facing feature changes, no new UI elements, no renamed settings/tabs/buttons, and no workflow changes. No help site documentation changes are required. |
Explanation of Change
After PR #85234 introduced a bottom TabNavigator with
freezeOnBlur: true, the Search/Spend screen stays mounted across tab switches. When the user does "Clear cache and restart",Onyx.clear()wipes all snapshot data, but theuseEffectinuseSearchPageSetupthat firessearch()does not re-run because its deps (hash,isOffline,shouldUseLiveData,queryJSON) haven't changed. The skeleton loading state persists forever since no newsearch()call is made.This PR extracts the search-firing logic into a
useCallbackand adds auseFocusEffectso that when the Spend tab regains focus after a cache clear,search()is re-fired if the snapshot data is missing.Fixed Issues
$ #88961
PROPOSAL: #88961 (comment)
Tests
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
Screen.Recording.2026-04-27.at.3.47.56.PM.mov
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari