Reports page and TransactionItemRow performance improvements[part 1] by SzymczakJ · Pull Request #65421 · Expensify/App

SzymczakJ · 2025-07-03T13:17:43Z

Explanation of Change

This PR refactors Search related components SearchContext, SearchList, SearchPage etc. to stop redundant rerenders that cascade all the way down to search list items.
Key points of the refactor:

I refactored how Onyx MOBILE_SELECTION_MODE value works, it used to be a object with single prop {isEnabled: boolean} which was causing redundant value updates in Onyx when setting this onyx key to the same value(cause of new object creation)
I refactored useMobileSelectionMode hook to not run turnOffMobileSelectionMode when selectionMode is already off, this saves rerenders all over the app as this hook is widely used.
On SearchPage: I refactored lastNonEmptySearchResults to be stored in useRef instead of useState to avoid extra rerender every time new search data comes from the BE.
In SearchContext:
- I fixed setCurrentSearchHash function to not make SearchContext rerender(which rerenders everything) when we set hash to the same hash.
- I fixed clearSelectedTransactions function to not make SearchContext rerender, when selected transactions are already cleared.
In Search/index: stoped running setSelectedTransactions in case, when there are no selectedTransactions.
... plus some minor optimizations.

Fixed Issues

$ #63672
PROPOSAL:

Tests

Go to Reports page:
Test selection mode on Reports > Expenses page

test if selecting one expense /multiple expenses works
test if deselecting one expense /multiple expenses works
test if using selection options like hold etc. works

Test selection mode on Reports > Expense Reports page

test if selecting one report /multiple reports works
test if deselecting one report /multiple reports works
test if using selection options like hold etc. works

Test switching between Reports page tabs: Expenses <-> Expense Reports, make sure that if the page was already loaded it doesn't show loading state
Test switching between Reports page tabs: Chats <-> Expenses, make sure that if the page was already loaded it doesn't show loading state
Reset cache -> go to reports page and make sure selection mode is off

Offline tests

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Screen.Recording.2025-07-07.at.12.09.45.mov

Android: mWeb Chrome

Screen.Recording.2025-07-07.at.12.14.54.mov

iOS: Native

Screen.Recording.2025-07-07.at.10.32.47.mov

iOS: mWeb Safari

Screen.Recording.2025-07-07.at.12.01.02.mov

MacOS: Chrome / Safari

Screen.Recording.2025-07-07.at.12.19.43.mov

MacOS: Desktop

…lse to false

…tations

Kicu

Very good changes 👍 I left a bunch of comments but none are super important.
Good job!

src/components/MoneyReportHeader.tsx

src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx

src/hooks/useMobileSelectionMode.ts

Kicu · 2025-07-04T07:25:01Z

src/libs/actions/MobileSelectionMode.ts


 const turnOnMobileSelectionMode = () => {
-    Onyx.merge(ONYXKEYS.MOBILE_SELECTION_MODE, {isEnabled: true});
+    Onyx.merge(ONYXKEYS.MOBILE_SELECTION_MODE, true);


src/pages/RoomMembersPage.tsx

Kicu · 2025-07-04T07:40:47Z

src/components/Search/SearchList.tsx

-        },
-        [],
-    );
-


amazing 👏
Can you tell me in short what part of code is responsible for disabling selection mode now in these cases?

For the first useEffect:
this useEffect and this one from Search/index are doing the same thing, so these effects were redundant.

For the second one: now when everything uses useMobileSelctionMode we don't need it to clear it after leaving because it is cleared on entering a screen that uses useMobileSelctionMode.

src/components/Search/SearchList.tsx

Kicu · 2025-07-04T07:44:58Z

src/components/Search/SearchPageHeader/SearchPageHeader.tsx

    const selectedTransactionsKeys = Object.keys(selectedTransactions ?? {});

-    if (shouldUseNarrowLayout && selectionMode?.isEnabled && headerButtonsOptions.length > 0) {
+    if (shouldUseNarrowLayout && !!selectionMode && headerButtonsOptions.length > 0) {


same case - selection mode is used via useOnyx and not via useSelectionMode. Why?

src/components/Search/SearchPageHeader/SearchFiltersBar.tsx

Kicu · 2025-07-04T07:49:47Z

src/pages/Search/SearchPageNarrow.tsx

+            },
        },
-    });
+        [],


the change with useAnimatedScrollHandler is that you are now passing [] as a dependency list, right?
Does that mean we will do less re-renders now because in the past we'd do more?

Previously useAnimatedScrollHandler would recreate in every rerender causing additional overhead. Passing empty dependency array stops that.

…irst time

melvin-bot · 2025-07-07T10:36:56Z

@suneox 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

Overall, the change set looks good. There are just a few points that need clarification and improvement. I’m currently stuck with the iOS build but will resolve it soon to complete the checklist.

suneox · 2025-07-08T19:15:21Z

src/components/Search/SearchPageHeader/SearchFiltersBar.tsx

 type SearchFiltersBarProps = {
    queryJSON: SearchQueryJSON;
    headerButtonsOptions: Array<DropdownOption<SearchHeaderOptionValue>>;
+    isMobileSelectionModeEnabled: boolean;


NAB: I'm not sure if we need to introduce this prop, since all the places using this component already pass the value from the useMobileSelectionMode hook

Each useMobileSelectionMode is adding additional overhead with useOnyx, useRef and the useEfffect which it calls.
Since the performance is our number one concern and it's so easy to pass isMobileSelectionModeEnabled as it is in the SearchFiltersBar parent I think we should keep it that way.

suneox · 2025-07-08T19:26:14Z

src/components/Search/SearchPageHeader/SearchPageHeader.tsx

    onSearchRouterFocus?: () => void;
    headerButtonsOptions: Array<DropdownOption<SearchHeaderOptionValue>>;
    handleSearch: (value: string) => void;
+    isMobileSelectionModeEnabled: boolean;


NAB: same above all the places using this component already pass the value from the useMobileSelectionMode hook so i think we don't need to introduce this props

The same answear as above ☝️

suneox · 2025-07-08T19:32:19Z

src/components/SelectionList/Search/TransactionListItem.tsx

-                            handleActionButtonPress(currentSearchHash, transactionItem, () => onSelectRow(item), shouldUseNarrowLayout && !!canSelectMultiple);
-                        }}
+                        onButtonPress={handleActionButtonPress}
                        onCheckboxPress={() => onCheckboxPress?.(item)}


If this component has a re-render issue, I think we should wrap all calback functions with useCallback, rather than just wrapping onButtonPress

suneox · 2025-07-09T09:47:14Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified there are no new alerts related to the canBeMissing param for useOnyx
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

Screen.Recording.2025-07-09.at.16.39.55.mp4

Android: mWeb Chrome

Screen.Recording.2025-07-09.at.16.36.02.mp4

iOS: HybridApp

Screen.Recording.2025-07-09.at.16.27.48.mp4

iOS: mWeb Safari

Screen.Recording.2025-07-09.at.16.32.49.mp4

MacOS: Chrome / Safari

Screen.Recording.2025-07-09.at.16.18.51.mp4

MacOS: Desktop

Screen.Recording.2025-07-09.at.16.30.48.mp4

suneox

The current change doesn't break existing functionality, so i think we can proceed with this PR. Based on my test data, I've only seen a slight improvement. Looking forward to the next investigation around flattening the view hierarchy it may bring even more improvements!

mountiny

Lets get this one out, thanks!

mountiny · 2025-07-09T12:15:04Z

src/ONYXKEYS.ts

    [ONYXKEYS.REVIEW_DUPLICATES]: OnyxTypes.ReviewDuplicates;
    [ONYXKEYS.ADD_NEW_COMPANY_CARD]: OnyxTypes.AddNewCompanyCardFeed;
    [ONYXKEYS.ASSIGN_CARD]: OnyxTypes.AssignCard;
-    [ONYXKEYS.MOBILE_SELECTION_MODE]: OnyxTypes.MobileSelectionMode;


Did you remove this type as its not used anymore?

mountiny · 2025-07-09T12:22:05Z

src/components/Search/index.tsx

+                const baseKey = isChat
+                    ? `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${(item as ReportActionListItemType).reportActionID}`
+                    : `${ONYXKEYS.COLLECTION.TRANSACTION}${(item as TransactionListItemType).transactionID}`;
+                // Check if the base key matches the newSearchResultKey (TransactionListItemType)


Suggested change

// Check if the base key matches the newSearchResultKey (TransactionListItemType)

// Check if the base key matches the newSearchResultKey (TransactionListItemType)

mountiny · 2025-07-09T12:22:10Z

src/components/Search/index.tsx

+                    : `${ONYXKEYS.COLLECTION.TRANSACTION}${(item as TransactionListItemType).transactionID}`;
+                // Check if the base key matches the newSearchResultKey (TransactionListItemType)
+                const isBaseKeyMatch = baseKey === newSearchResultKey;
+                // Check if any transaction within the transactions array (TransactionGroupListItemType) matches the newSearchResultKey


Suggested change

// Check if any transaction within the transactions array (TransactionGroupListItemType) matches the newSearchResultKey

// Check if any transaction within the transactions array (TransactionGroupListItemType) matches the newSearchResultKey

mountiny · 2025-07-09T12:22:16Z

src/components/Search/index.tsx

+                        const transactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`;
+                        return transactionKey === newSearchResultKey;
+                    });
+                // Determine if either the base key or any transaction key matches


Suggested change

// Determine if either the base key or any transaction key matches

// Determine if either the base key or any transaction key matches

OSBotify · 2025-07-09T12:41:09Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2025-07-10T07:26:33Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.79-0 🚀

platform	result
🖥 desktop 🖥	success ✅
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

…/@szymczak/performance-improvements" This reverts commit 8956422, reversing changes made to fda04c5.

…k/performance-improvements" This reverts commit 8956422, reversing changes made to fda04c5.

github-actions · 2025-07-11T17:38:36Z

🚀 Deployed to production by https://github.com/Julesssss in version: 9.1.79-11 🚀

platform	result
🖥 desktop 🖥	success ✅
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

suneox · 2025-07-26T08:16:03Z

src/components/Search/SearchList.tsx

-        } else if (selectedItemsLength === 0 && selectionMode?.isEnabled && !wasSelectionOnRef.current) {
-            turnOffMobileSelectionMode();
-        }


Coming from the #66027 checklist: we’ve removed this logic, so in case the user navigates back using the Android back button, we need to call turnOffMobileSelectionMode

bernhardoj · 2025-12-10T08:22:22Z

src/components/Search/index.tsx

            });
        }
+        if (isEmptyObject(newTransactionList)) {
+            return;


This early return causes an issue where the selected transactions are not cleared when we previously have selected transactions, but then all selected items are removed from the list.

SzymczakJ added 10 commits July 1, 2025 15:36

fix animatedScrollHandler recomputations

4bf02fb

fix setCurrentSearchHash to not cause redundant rerenders

9384ded

fix selectionMode caused rerenders when selection mode is set from fa…

3d004eb

…lse to false

stop SearchPage rerenders caused by lastNonEmptySearchResults recompu…

7f416c6

…tations

stop redudndant rerenders caused by SearchContext

196c2a7

delete redundant disable compiler case

572bdde

Merge branch 'main' into @szymczak/performance-improvements

3eeb275

Merge branch 'main' into @szymczak/performance-improvements

d98d2da

fix failing linter

3fbda01

Merge branch 'main' into @szymczak/performance-improvements

b4141c0

Kicu approved these changes Jul 4, 2025

View reviewed changes

fix selectionMode turing on when user entering reports page for the f…

3029843

…irst time

SzymczakJ mentioned this pull request Jul 4, 2025

[Due for payment 2025-08-13] [Due for payment 2025-08-07] Improve TransactionItemRow component performance #63672

Closed

SzymczakJ added 2 commits July 7, 2025 09:51

Merge branch 'main' into @szymczak/performance-improvements

79795df

fix PR comments

f8fddc9

SzymczakJ marked this pull request as ready for review July 7, 2025 10:36

SzymczakJ requested a review from a team as a code owner July 7, 2025 10:36

melvin-bot bot requested review from suneox and removed request for a team July 7, 2025 10:36

Merge branch 'main' into @szymczak/performance-improvements

53c9ce8

suneox reviewed Jul 8, 2025

View reviewed changes

mountiny assigned SzymczakJ Jul 8, 2025

fixPR comments

af258b3

suneox approved these changes Jul 9, 2025

View reviewed changes

melvin-bot bot requested a review from mountiny July 9, 2025 10:49

mountiny approved these changes Jul 9, 2025

View reviewed changes

mountiny merged commit 8956422 into Expensify:main Jul 9, 2025
20 checks passed

github-actions bot mentioned this pull request Jul 10, 2025

Deploy Checklist: New Expensify 2025-07-10 #65809

Closed

77 tasks

jponikarchuk mentioned this pull request Jul 10, 2025

[Due for payment 2025-07-22] [Due for payment 2025-07-18] Reports - "x selected" dropdown button does not disappear after deleting expense #65824

Closed

8 tasks

mitarachim mentioned this pull request Jul 10, 2025

[$250] Reports - Nothing happens after triggering Download for the second time #65844

Closed

8 tasks

grgia mentioned this pull request Jul 10, 2025

[Due for payment 2025-07-22] Expense - Tag detail page is not open to edit #65821

Closed

8 tasks

BlameGPT mentioned this pull request Jul 10, 2025

[Due for payment 2025-08-05] [$250] Report - When workspace member adds expense to report, error is displayed #65870

Closed

16 tasks

suneox added a commit to suneox/ExpensifyApp that referenced this pull request Jul 10, 2025

Revert "Merge pull request Expensify#65421 from software-mansion-labs…

dd5ccf2

…/@szymczak/performance-improvements" This reverts commit 8956422, reversing changes made to fda04c5.

grgia added a commit that referenced this pull request Jul 11, 2025

Revert "Merge pull request #65421 from software-mansion-labs/@szymcza…

dd3f267

…k/performance-improvements" This reverts commit 8956422, reversing changes made to fda04c5.

BlameGPT mentioned this pull request Jul 11, 2025

Deleted by User A expense remains in User B's report, leads to Not here page #65914

Closed

8 tasks

SzymczakJ mentioned this pull request Jul 15, 2025

[Due for payment 2025-07-25] Report - "Select Multiple" still seen on top of report, after selecting and removing expense #66111

Closed

8 tasks

suneox reviewed Jul 26, 2025

View reviewed changes

waterim mentioned this pull request Jul 28, 2025

Feat: Add ability to unreport transactions v3 #67048

Merged

58 tasks

trjExpensify mentioned this pull request Sep 5, 2025

Reports - Fallback WS avatar shown when clearing the "From" filter on "Submit" while offline #69869

Merged

51 tasks

paulnjs mentioned this pull request Nov 25, 2025

[Due for payment 2025-12-16] [$250] Reports - Selected state remains after the expense disappears from the page #74822

Closed

16 tasks

bernhardoj reviewed Dec 10, 2025

View reviewed changes

	// Check if the base key matches the newSearchResultKey (TransactionListItemType)

	// Check if the base key matches the newSearchResultKey (TransactionListItemType)

	// Check if any transaction within the transactions array (TransactionGroupListItemType) matches the newSearchResultKey

	// Check if any transaction within the transactions array (TransactionGroupListItemType) matches the newSearchResultKey

	// Determine if either the base key or any transaction key matches

	// Determine if either the base key or any transaction key matches

Conversation

SzymczakJ commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

Kicu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melvin-bot bot commented Jul 7, 2025

Uh oh!

suneox left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suneox Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suneox commented Jul 9, 2025

Reviewer Checklist

Screenshots/Videos

Uh oh!

suneox left a comment

Choose a reason for hiding this comment

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

OSBotify commented Jul 9, 2025

Uh oh!

github-actions bot commented Jul 10, 2025

Uh oh!

github-actions bot commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

SzymczakJ commented Jul 3, 2025 •

edited

Loading

suneox Jul 8, 2025 •

edited

Loading