Decompose Expensify.tsx from delegate access, Fullstory init, screen share and priority mode#83307
Conversation
…Handler, ScreenShareRequestModal
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Replace `import * as User` with named imports to comply with the no-restricted-syntax rule for @userActions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
||
| if (splashScreenState === CONST.BOOT_SPLASH_STATE.VISIBLE) { | ||
| const propsToLog: Omit<ExpensifyProps & {isAuthenticated: boolean}, 'children' | 'session'> = { | ||
| const propsToLog = { |
There was a problem hiding this comment.
I highly doubt this is useful to anyone. We can completely remove it or accept this strip.
|
@Krishna2323 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] |
|
@suneox @Julesssss One of you needs to 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] |
|
Will be reviewing this in 30mins. |
|
Reviewing... |
Isolate NVP_PRIORITY_MODE and COLLECTION.REPORT_DRAFT_COMMENT Onyx subscriptions into a render-null handler component so changes to these keys no longer re-render the entire Expensify component tree. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| const [account] = useOnyx(ONYXKEYS.ACCOUNT); | ||
| const [session] = useOnyx(ONYXKEYS.SESSION); | ||
| const {preferredLocale} = useLocalize(); | ||
| const [accountID] = useOnyx(ONYXKEYS.SESSION, {selector: accountIDSelector}); |
There was a problem hiding this comment.
We are now using selector for accountID but the component uses useIsAuthenticated on line 121 which subscribes to the full SESSION object. We can update useIsAuthenticated to use a selector (e.g., session?.authToken) so this component only re-renders when authToken actually changes.
This isn't something introduced by this PR, but it's a missed opportunity since the PR's goal is reducing root-level re-renders.
|
Sorry for last minute changes, pushed just one small further improvement that removes even more renders (PriorityModeHandler). No more changes to come. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4199d88516
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Add authTokenSelector to Session selectors and use it in useIsAuthenticated to avoid re-renders on unrelated session field changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid_native.mp4Android: mWeb Chromeandroid_chrome.mp4iOS: HybridAppios_native.mp4iOS: mWeb Safariios_safari.mp4MacOS: Chrome / Safariweb_chrome_1.mp4web_chrome_3.mp4web_chrome_2.mp4 |
|
I'm still working on the platform videos, will be done in 15mins |
Krishna2323
left a comment
There was a problem hiding this comment.
LGTM and tests well! 🚀
The failing test doesn’t seem related. @adhorodyski, could you please confirm? Thanks!
|
yeah just github's issues, resyncing now |
mountiny
left a comment
There was a problem hiding this comment.
LGTM, thank you, going to move this ahead to avoid conflicts
|
✋ 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.26-0 🚀
|
|
Hi @adhorodyski QA team can't check Fullstory. Could you please validate it internally? Thank you in advance |
|
@izarutskaya Yeah will do |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.26-8 🚀
|
|
Everything operational so far, thanks |
@mountiny @rlinoz
Explanation of Change
TLDR: this change improves React rendering at the root of the app by ~5%, making eg. report opening faster.
Tests I conducted: refreshed the app (from scratch) while on the same report.
This PR reduces root-level Onyx subscriptions in
Expensify.tsxfrom 14 down to 6 by extracting 8 subscriptions into three isolated renderless components. Previously, every change to keys likeACCOUNT,USER_METADATA, orSCREEN_SHARE_REQUESTwould cause a full re-render of the rootExpensifycomponent — including the entire navigation tree (NavigationRoot). By moving these subscriptions into dedicated components, their state changes are now scoped and do not bubble up to trigger unnecessary re-renders of the navigation tree.Three new renderless components were created:
src/DelegateAccessHandler.tsx— Owns 5 subscriptions (ACCOUNT,STASHED_CREDENTIALS,STASHED_SESSION,HAS_LOADED_APP,IS_LOADING_APP) plus the delegate disconnect/mismatch logging andisLoadingApprecovery effects that previously lived inExpensify.tsx.src/FullstoryInitHandler.tsx— Owns theUSER_METADATAsubscription and handles Fullstory initialization and Sentry context setting.src/components/ScreenShareRequestModal.tsx— Owns theSCREEN_SHARE_REQUESTsubscription and renders theConfirmModalfor screen-share requests from GuidesPlus agents.src/PriorityModeHandler.tsx— Decompose Expensify.tsx: extract PriorityModeHandler.Expensify.tsxwas updated to use a narrowedaccountIDSelectoronSESSION(instead of subscribing to the full session object), remove all moved subscriptions, removeuseNetwork(), drop now-unused imports, and clean up thepropsToLogobject andExpensifyPropstype.This follows the same pattern as the prior
DeepLinkHandlerextraction, which demonstrated a 10–35% render performance improvement.trace-report.txt
before:

after:

Fixed Issues
$ #74367
$ #77173
$ #84149
PROPOSAL:
Tests
ScreenShareRequestModalopens up correctly with a guide request.openApp.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