refactor: extract useMoneyRequestReportPagination by sosek108 · Pull Request #100030 · Expensify/App

sosek108 · 2026-09-01T12:51:04Z

Explanation of Change

This is PR 4 of a 10-PR stack decomposing the MoneyRequestReport list components (MoneyRequestReportActionsList / MoneyRequestReportTransactionList) into focused hooks and presentational sub-components.

This slice extracts the pagination behavior into a dedicated hook.

No performance movement in this PR

Fixed Issues

$ #99919
PROPOSAL:

Tests

Open an expense report with more than one page of report actions (50+ comments) — verify the transaction table and the comments render as before
Scroll to the top of the comments — verify older comments load in as you reach the top
Scroll to the bottom — verify newer comments load in as you reach the bottom (open the report via a deep link to an old comment first, so newer actions exist)
Open a large money-request report (50+ actions with IOU actions) — verify chat messages between the oldest loaded message and the newest ones backfill automatically after the report loads
Run npx jest tests/ui/MoneyRequestReportActionsListRejectModalTest.tsx and verify it passes

Verify that no errors appear in the JS console

Offline tests

Go offline and scroll to the edges of the loaded comments — verify no request loops occur and the list stays stable
Go back online — verify loading older/newer comments resumes working

QA Steps

Open an expense report with more than one page of report actions (50+ comments) — verify the transaction table and the comments render as before
Scroll to the top of the comments — verify older comments load in as you reach the top
Scroll to the bottom — verify newer comments load in as you reach the bottom (open the report via a deep link to an old comment first, so newer actions exist)
Open a large money-request report (50+ actions with IOU actions) — verify chat messages between the oldest loaded message and the newest ones backfill automatically after the report loads

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

Nagranie.z.ekranu.2026-09-9.o.14.42.22.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

Nagranie.z.ekranu.2026-09-9.o.14.41.22.mov

codecov · 2026-09-01T12:54:31Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
...equestReportView/MoneyRequestReportActionsList.tsx	`86.22% <100.00%> (+3.70%)`	⬆️
...questReportView/useMoneyRequestReportPagination.ts	`97.67% <97.67%> (ø)`
... and 15 files with indirect coverage changes

…compose/money-request-report-transactions-a3

sosek108 · 2026-09-15T11:05:45Z

@codex review
@MelvinBot review

chatgpt-codex-connector · 2026-09-15T11:08:43Z

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 5c8d929f76

ℹ️ 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".

melvin-bot · 2026-09-15T11:12:47Z

@Pujan92 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]

MelvinBot · 2026-09-15T11:16:54Z

Review

The extraction is faithful — I verified the moved logic is behaviorally identical to what it replaced. No blocking issues. Two nits below.

Verification I ran locally on 5c8d929:

tests/unit/useMoneyRequestReportPaginationTest.ts — 20/20 pass
tests/ui/MoneyRequestReportActionsListRejectModalTest.tsx — 6/6 pass
tsc --noEmit — clean for both changed source files
eslint on all three changed files — clean, except one pre-existing react-hooks/set-state-in-effect error at MoneyRequestReportActionsList.tsx:359 that also fires on main, so dropping the file-wide rulesdir/prefer-early-return disable was safe.

1. `reportActionIDs` is now a vestigial `useMemo` in the component

MoneyRequestReportActionsList.tsx:189 computes reportActionIDs and its only consumer is the new hook — no other reference in the file. Since the hook already receives reportActions, move the useMemo into useMoneyRequestReportPagination.ts and drop the reportActionIDs param. That takes the param surface from 9 to 8 and leaves zero pagination residue in the component, which is the point of the slice.

2. One test passes vacuously

useMoneyRequestReportPaginationTest.ts — "should restart the backfill from the newest-fetched cursor when mounted for another report" uses a second renderHook, which always gets fresh refs. It passes no matter what the component does with key={reportID}, so it doesn't cover the contract its comment describes. It's still a useful guard against someone hoisting the cursor refs to module scope — consider retitling it to say that, so a future reader doesn't trust it for the remount contract.

Equivalence check on the moved logic

Auto-load-newer effect — the guard was rewritten from a positive wrapping if to early returns:

Before: hasFinishedInitialLoad && hasNewerActions && reportActions.length > 0 && !isOffline && !isLoadingNewerReportActions
After: !hasFinishedInitialLoad || !hasNewerActions || reportActions.length === 0 || isOffline || isLoadingNewerReportActions → return

De Morgan-equivalent, and the dependency array is unchanged.

Backfill effect — body and deps moved verbatim.

onStartReached / onEndReached — bodies identical; both lost their useCallback. That is not a regression: loadOlderChats / loadNewerChats are plain arrows in useLoadReportActions.ts:83 and :102, so the old useCallback(…, [loadOlderChats]) already produced a new function on every render. It also matches the surrounding file, which defines handlers like syncBottomOffset as plain functions.

Worth flagging for a later slice, though: these two handlers are props on memo(MoneyRequestReportTransactionList) → memo(MoneyRequestReportUnifiedList). Both memo boundaries are already dead anyway, because listFooterComponent={<ConciergeThinkingMessage … />} at MoneyRequestReportActionsList.tsx:581 allocates a fresh element every render. If a later PR in this stack tries to make that memo() actually bail out, the footer element plus these two handlers are all on the list of props to stabilize — and the real fix for the handlers belongs in useLoadReportActions, not here.

Remount contract — the hook's doc comment claims the consumer is keyed by reportID; confirmed at MoneyRequestReportActionsList.tsx:596.

view run · no recording available

sosek108 · 2026-09-15T12:54:45Z

Check are failing but do not seem to be related

suneox · 2026-09-15T13:15:39Z

I'm starting to take a look at this PR

suneox · 2026-09-15T14:27:50Z

@sosek108 Can you merge main into pr branch to resolve the pipeline error has been resolved at PR?

suneox · 2026-09-15T14:30:44Z

I'm still troubleshooting an issue where messages don't fully load after clearing the cache or performing a fresh login. Since this is also reproducing on latest staging (v9.4.78-0), we can track it as a separate issue.

CleanShot.2026-09-15.at.20.42.42.1.mp4

suneox · 2026-09-15T14:48:47Z

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
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 proper code patterns were followed (see Reviewing the code)
- 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 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)
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)
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately
- 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
- 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 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.
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

CleanShot.2026-09-15.at.21.14.02.4.mp4

Android: mWeb Chrome

CleanShot.2026-09-15.at.21.10.28.3.mp4

iOS: HybridApp

CleanShot.2026-09-15.at.21.07.45.2.mp4

iOS: mWeb Safari

CleanShot.2026-09-15.at.21.00.25.1.mp4

MacOS: Chrome / Safari

CleanShot.2026-09-15.at.20.53.16.1.mp4

suneox · 2026-09-15T14:54:53Z

I'm still troubleshooting an issue where messages don't fully load after clearing the cache or performing a fresh login. Since this is also reproducing on latest staging (v9.4.78-0), we can track it as a separate issue.

CleanShot.2026-09-15.at.20.42.42.1.mp4

Regarding this bug I'll investigate further and report it in the Slack bug channel soon.

suneox · 2026-09-15T14:56:27Z

Moving the component logic into the hook still keep the existing behavior LGTM. We're just waiting on a merge from main to fix the failed pipeline

sosek108 · 2026-09-16T07:33:06Z

Main is merged. Waiting for checks to pass 🤞

dylanexpensify

Product input not needed here

mountiny

Thanks, looks good

github-actions · 2026-09-17T13:35:22Z

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

OSBotify · 2026-09-17T13:36:52Z

✋ 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 · 2026-09-17T13:39:57Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #100030.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-6e977c5-9b6d912-b754c7694e3f483e3e2fa37642871e62bf70542d/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-6e977c5-9b6d912-d60230030923073f6412fd51a11ab7c47372043f/index.html

Web 🕸️
https://100030.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-09-17T22:20:19Z

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

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	cancelled 🔪
🍎 iOS 🍎	cancelled 🔪

MelvinBot · 2026-09-17T22:26:45Z

🤖 No help site changes required. I did not create a docs PR.

This PR is an internal refactor only. It moves the existing pagination logic out of MoneyRequestReportActionsList.tsx into a new hook, useMoneyRequestReportPagination.ts, plus a unit test. Nothing a customer sees or does changes.

Why no docs update is needed

I checked the three things that would make a help site change necessary:

No user-facing copy. I scanned the diff for translate() calls and string literals. The only match was a Jest describe('IOU backfill') label in the test file.
No behavior change. The auto-load-newer loop, the IOU backfill loop, and the onStartReached / onEndReached handlers move verbatim. The BACKFILL_MIN_ACTIONS_THRESHOLD of 50 and every cursor guard are unchanged. The one edit is inverting the auto-load-newer guard into an early return (dropping the prefer-early-return eslint disable), which is logically identical.
No new or renamed feature, tab, setting, or button. Loading older and newer comments in a report already worked this way, and the help site does not document it — nothing under docs/articles mentions comment pagination or loading older comments.

Because there is nothing to write, HELPSITE_NAMING_CONVENTIONS.md, HELP_AUTHORING_GUIDELINES.md, and TEMPLATE.md never came into play, and there was no feature name or button label to verify against the live UI.

Since no docs PR exists, there is nothing to review or mark Ready for review. If you expected a customer-visible change from this slice that I missed, reply with what it is and I will draft the article update.

view run

OSBotify · 2026-09-18T18:06:42Z

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

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

MelvinBot · 2026-09-18T18:10:55Z

🤖 No help site changes are required for this PR — so I did not create a docs draft PR.

This PR moves existing pagination logic out of MoneyRequestReportActionsList.tsx into a new hook, useMoneyRequestReportPagination.ts, and adds a unit test. Behavior is unchanged, so there is nothing for the help site to document.

What I checked

Changed files (3):

File	Change
`src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx`	Removed ~100 lines of inline pagination/backfill effects; now calls the new hook for `onStartReached` / `onEndReached`
`src/components/MoneyRequestReportView/useMoneyRequestReportPagination.ts`	New — same load-older / load-newer / IOU-backfill logic, lifted verbatim
`tests/unit/useMoneyRequestReportPaginationTest.ts`	New unit test

Why no docs are needed:

No user-facing copy. No src/languages/*.ts files touched, no new strings, no new buttons, labels, tabs, or settings.
No behavior change. The PR body states "No performance movement in this PR", and the Tests / QA Steps sections all describe verifying that the report list "render[s] as before". The moved code (the BACKFILL_MIN_ACTIONS_THRESHOLD guard, the cursor-advance safety check, the getOlderActions backfill loop) is identical to what already shipped.
Nothing documented to update. Comment pagination in an expense report is invisible infrastructure — it has no entry in App/docs/articles. I searched the help site for pagination and comment-loading content and found no article that describes this behavior.

This is PR 4 of the 10-PR stack under [Perf] Investigate Money Request Report performance. Refactor-only slices in this stack won't need docs updates; if a later PR in the stack changes what a user actually sees or does, tag me on that one and I'll draft the help site changes.

sosek108 — there is no linked help site PR to review, because no help site changes are needed. If you disagree and think something here is user-visible, reply with @MelvinBot and what you'd like documented, and I'll create the draft PR.

view run

OSBotify · 2026-09-18T19:23:27Z

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

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

OSBotify · 2026-09-18T19:58:41Z

🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.89-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	failure ❌

Bundle Size Analysis (Sentry):

Android

Extract useMoneyRequestReportPagination

d66bf4b

melvin-bot Bot assigned sosek108 Sep 1, 2026

Merge branch 'decompose/money-request-report-transactions-a2' into de…

5afaa14

…compose/money-request-report-transactions-a3

sosek108 mentioned this pull request Sep 8, 2026

refactor: extract transaction selection and pending expense hooks, finish wiring #100040

Closed

40 tasks

sosek108 added 5 commits September 9, 2026 14:06

Merge branch 'decompose/money-request-report-transactions-a2' into de…

55abfc3

…compose/money-request-report-transactions-a3

Add tests for new hook

eee226d

Merge branch 'decompose/money-request-report-transactions-a2' into de…

9cc7473

…compose/money-request-report-transactions-a3

Merge branch 'main' into decompose/money-request-report-transactions-a3

6cb174f

Merge branch 'main' into decompose/money-request-report-transactions-a3

5c8d929

sosek108 marked this pull request as ready for review September 15, 2026 11:12

sosek108 requested review from a team as code owners September 15, 2026 11:12

melvin-bot Bot requested review from Pujan92 and dylanexpensify and removed request for a team September 15, 2026 11:12

melvin-bot Bot requested review from a team and removed request for a team September 15, 2026 11:12

sosek108 mentioned this pull request Sep 15, 2026

[Due for payment 2026-09-25] perf: decompose MoneyRequestReportView list components into compiler compliant hooks #99919

Open

github-actions Bot reviewed Sep 15, 2026

View reviewed changes

Comment thread src/components/MoneyRequestReportView/useMoneyRequestReportPagination.ts Outdated

sosek108 mentioned this pull request Sep 15, 2026

refactor: extract useMoneyRequestReportColumns and useMoneyRequestReportLayout #101200

Open

39 tasks

mountiny removed the request for review from Pujan92 September 15, 2026 12:42

Merge branch 'main' into decompose/money-request-report-transactions-a3

7af0ca7

mountiny requested a review from suneox September 15, 2026 12:46

apply comments fixes

e295f62

Merge branch 'main' into decompose/money-request-report-transactions-a3

95c259d

suneox approved these changes Sep 16, 2026

View reviewed changes

melvin-bot Bot requested a review from mountiny September 16, 2026 10:08

dylanexpensify reviewed Sep 16, 2026

View reviewed changes

Merge branch 'main' into decompose/money-request-report-transactions-a3

ce6de99

mountiny approved these changes Sep 17, 2026

View reviewed changes

mountiny merged commit 6e977c5 into Expensify:main Sep 17, 2026
34 of 35 checks passed

OSBotify mentioned this pull request Sep 17, 2026

Deploy Checklist: New Expensify 2026-09-17 #101475

Closed

82 tasks

Conversation

sosek108 commented Sep 1, 2026 • 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!

codecov Bot commented Sep 1, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sosek108 commented Sep 15, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 15, 2026

Uh oh!

melvin-bot Bot commented Sep 15, 2026

Uh oh!

MelvinBot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

1. reportActionIDs is now a vestigial useMemo in the component

2. One test passes vacuously

Uh oh!

Uh oh!

sosek108 commented Sep 15, 2026

Uh oh!

suneox commented Sep 15, 2026

Uh oh!

suneox commented Sep 15, 2026

Uh oh!

suneox commented Sep 15, 2026

Uh oh!

suneox commented Sep 15, 2026

Reviewer Checklist

Screenshots/Videos

Uh oh!

suneox commented Sep 15, 2026

Uh oh!

suneox commented Sep 15, 2026

Uh oh!

sosek108 commented Sep 16, 2026

Uh oh!

dylanexpensify 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!

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

OSBotify commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

OSBotify commented Sep 17, 2026

Uh oh!

MelvinBot commented Sep 17, 2026

Uh oh!

OSBotify commented Sep 18, 2026

Uh oh!

MelvinBot commented Sep 18, 2026

Uh oh!

OSBotify commented Sep 18, 2026

Uh oh!

OSBotify commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sosek108 commented Sep 1, 2026 •

edited

Loading

codecov Bot commented Sep 1, 2026 •

edited by github-actions Bot

Loading

MelvinBot commented Sep 15, 2026 •

edited

Loading

1. `reportActionIDs` is now a vestigial `useMemo` in the component