[Payment due @situchan] [No QA] Remove manual memoization from 6 RC-failing files by roryabraham · Pull Request #88370 · Expensify/App

roryabraham · 2026-04-21T00:32:57Z

Explanation of Change

This PR addresses the six files that were failing the React Compiler compliance check with Existing memoization could not be preserved. Each of these had their manual memoization removed, are now compiling with React Compiler, and were organized to group like items together.

Lowered --max-warnings accordingly.

Fixed Issues

part of #68765

Tests

Create a manual distance expense
Download a wallet statement
Create two duplicate expenses and verify that you can resolve the duplicates.

Verify that no errors appear in the JS console

Offline tests

Go offline. Repeat the Manual Distance flow (tests 1–3) and the duplicates flow (test 5) and verify no JS errors and that pending states render as expected.

QA Steps

[No QA] — behavior-neutral refactor.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

melvin-bot · 2026-04-21T00:33:04Z

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

codecov · 2026-04-21T00:43:44Z

Codecov Report

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

Files with missing lines	Coverage Δ
...c/components/ReportActionItem/MoneyRequestView.tsx	`72.19% <ø> (+8.54%)`	⬆️
...ch/DeferredSearchAutocompleteList/index.native.tsx	`0.00% <0.00%> (ø)`
...c/components/WalletStatementModal/index.native.tsx	`0.00% <0.00%> (ø)`
...ages/iou/request/step/IOURequestStepAccountant.tsx	`0.00% <0.00%> (ø)`
.../iou/request/step/IOURequestStepDistanceManual.tsx	`0.00% <0.00%> (ø)`
src/pages/TransactionDuplicate/Review.tsx	`0.00% <0.00%> (ø)`
... and 10 files with indirect coverage changes

React Compiler handles the memoization; removing the manual useMemo resolves the 'Existing memoization could not be preserved' RC compliance failure. Part of #68765. Made-with: Cursor

…cture hooks Removes the useCallback wrapping renderComponent so the file passes the React Compiler 'Existing memoization could not be preserved' check, and destructures useState/useTransition from the React import for consistency with useRef. Part of #68765. Made-with: Cursor

…ssage Removes the useCallback around onMessage and restructures the try/catch so it no longer contains a logical-or value block (both unsupported by React Compiler). Also groups Onyx reads together. Part of #68765. Made-with: Cursor

…nline loop Removes four useMemos and one useCallback, replaces the map/filter/sort chain with a plain for…of loop, reorders the component body into canonical sections (onyx → refs/state → derived → effects → handlers → early returns → JSX), and drops the useCallback wraps inside useFocusEffect. Part of #68765. Made-with: Cursor

Removes all five useCallback wraps. The two Onyx selectors are now plain inline const functions (this file is their only caller), and the event handlers are plain arrow functions. Part of #68765. Made-with: Cursor

…implify effects Removes the file-wide @typescript-eslint/no-unused-vars disable and deletes the bindings it was hiding (usePolicy, getRateID, currentTransaction, lodashIsEmpty), removes three useMemos and all useCallbacks, and groups Onyx reads together. Deletes the 'clear formError when selectedTab changes' useEffect entirely: formError is event-driven (set on invalid submit, cleared by NumberWithSymbolForm's onInputChange on any keystroke), so the tab-switch reset was unnecessary and triggered react-hooks/set-state-in-effect. This also removes the unused isLoadingSelectedTab / isLoadingOnyxValue machinery. The remaining useEffect that syncs the imperative NumberWithSymbolForm child with the React-owned distance value is documented as a legitimate external-system sync. Part of #68765. Made-with: Cursor

Accounts for the 6 warnings removed by the RC memoization cleanup. Part of #68765. Made-with: Cursor

melvin-bot · 2026-04-21T20:30:50Z

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

roryabraham · 2026-04-21T20:31:14Z

@JmillsExpensify no product changes, just a code refactor

situchan · 2026-04-21T20:32:43Z

Does this require C+ review?

roryabraham · 2026-04-21T20:35:08Z

@situchan sure!

MelvinBot · 2026-04-22T14:37:25Z

🤖 Code Review — PR #88370

Overall this is a clean refactor. The React Compiler should auto-memoize the callbacks and values based on their captured dependencies, making the manual useMemo/useCallback wrappers unnecessary. A few items worth confirming:

1. `IOURequestStepAccountant.tsx` — selector reference stability

The selector and lastWorkspaceNumberWithEmailSelector are now plain arrow functions recreated each render. useOnyx internally keys its cache on options?.selector by reference (both in the App wrapper at src/hooks/useOnyx.ts:69 and upstream in react-native-onyx). If React Compiler doesn't memoize these, every render will register/deregister a new cache consumer — functionally correct (deep equality prevents spurious re-renders) but wasteful.

situchan's existing review comment on MoneyRequestView.tsx is related: the useHasMultipleSplitChildren hook at src/hooks/useHasMultipleSplitChildren.ts already exists and uses selectors internally, which would be cleaner and avoids subscribing to the full TRANSACTION and REPORT collections.

Suggestion: Confirm that the React Compiler compliance check passes for IOURequestStepAccountant.tsx. If it does, the compiler is stabilizing the selector references and this is fine.

2. `Review.tsx` — `useFocusEffect` without `useCallback`

useFocusEffect (from @react-navigation/core) passes the callback directly into React.useEffect([effect, navigation]). An unstable reference would tear down and re-register focus/blur listeners every render. The PR removes the useCallback wrapper from both useFocusEffect calls.

This should be safe if React Compiler memoizes the callbacks. The second useFocusEffect (restoring originalTransactionIDsListRef) only closes over a ref, so memoization should be trivial. The first one closes over deleteTransactionNavigateBackUrl — same deps as the old useCallback, so the compiler should produce equivalent behavior.

Suggestion: Same as above — confirm RC compliance passes for this file.

3. `IOURequestStepDistanceManual.tsx` — removed `formError`-clearing effect (behavioral change)

This useEffect was removed:

useEffect(() => {
    if (isLoadingSelectedTab) { return; }
    setFormError('');
}, [selectedTab, isLoadingSelectedTab]);

This is not a memoization removal — it's a behavioral change. Previously, switching the distance tab cleared any validation error. Now formError is only cleared via onInputChange. Is this intentional? If a user sees a "distance must be > 0" error, switches tabs, and switches back, the error would persist.

4. `IOURequestStepDistanceManual.tsx` — `currentTransaction` removal (potential split-edit regression)

The old code selected between the draft and original transaction:

const currentTransaction = isEditingSplit && !lodashIsEmpty(splitDraftTransaction) ? splitDraftTransaction : transaction;

This variable was removed and all references now use transaction (the prop). Looking at the remaining code, mileageRate and distanceInMeters are computed from transaction — but during a split edit, the draft distance lives in splitDraftTransaction. The form might display stale values from the original transaction instead of the draft.

Suggestion: Verify the split-edit flow (edit distance on a split expense) still shows the correct draft value.

5. `--max-warnings` reduction (313 → 301)

The 12-warning reduction aligns with 6 files × ~2 warnings each (react-compiler/react-compiler). Looks correct.

Bottom line: Items 1, 2, and 5 are fine assuming the React Compiler compliance check passes on these files. Items 3 and 4 are behavioral changes that go beyond memoization removal and should be confirmed as intentional.

Replaces two full-collection useOnyx subscriptions (TRANSACTION, REPORT) plus the derived boolean call with a single selector-based hook, so the view only re-renders when the hasMultipleSplits boolean actually changes. Made-with: Cursor

situchan · 2026-04-29T02:09:43Z

Sorry missed this

situchan · 2026-04-29T02:10:01Z

Signed commits failing but seems unrelated

situchan · 2026-04-29T02:10:03Z

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

android.mov

Android: mWeb Chrome

mchrome.mov

iOS: HybridApp

ios.mov

iOS: mWeb Safari

msafari.mov

MacOS: Chrome / Safari

web.mov

situchan · 2026-04-29T04:50:29Z

Sometimes resolve duplicates doesn't work. Not reliably reproduce

ios.mov

situchan · 2026-04-29T04:56:30Z

Sometimes not found page briefly shows when viewing duplicated expense. This is also not reliably reproducible.

Screen.Recording.2026-04-29.at.10.54.05.AM.mov

situchan · 2026-04-29T05:20:56Z

-            };
-        }, []),
-    );
+    useFocusEffect(() => {


Just to confirm it's safe to remove useCallback inside useFocusEffect as well?

yep - the callback appears to be correctly memoized:

situchan · 2026-04-29T05:26:41Z

I won't block on above issues sine they're not consistent and I have no reliable repro steps.

situchan · 2026-04-29T05:29:30Z

-    useEffect(() => {
-        if (isLoadingSelectedTab) {
-            return;
-        }
-        // eslint-disable-next-line react-hooks/set-state-in-effect -- Clear form error when tab changes
-        setFormError('');
-    }, [selectedTab, isLoadingSelectedTab]);


@MelvinBot find PR which added this effect. Just to make sure we don't re-introduce what this fixed.

Form error isn't cleared when tab changes

@roryabraham I think this is valid regression. This doesn't happen on staging.

Screen.Recording.2026-04-29.at.11.38.26.AM.mov

hmmm I've thought about it more and I don't think it's actually a bug. It just continues to show the same error that was already on the page. It's not actually stale - there was an error, and they didn't enter a valid amount to clear it. I'm not sure why it would be expected for the error to clear in that case.

ok but QA might report this as inconsistency as the error clears on other pages like manual request tab:

inconsistency.mov

I think it's fine from a user perspective, and shouldn't block the regular create request flow

MelvinBot · 2026-04-29T05:47:52Z

The useEffect that clears formError on tab change was added in #68402 ("[Manual distance] Create the distance creation tabs" by dominictb, merged 2025-08-25). Specifically, it was introduced in commit 97ce90caee7d ("confirmation step UI and last distance expense tab fallback").

The effect resets validation errors when the user switches between distance request tabs (e.g., Manual vs Map), so that stale errors from one tab don't persist when switching to another.

roryabraham · 2026-04-29T23:10:35Z

@situchan please take another look and let me know if you see anything else

melvin-bot · 2026-04-30T02:28:35Z

🎯 @situchan, thanks for reviewing and testing this PR! 🎉

A payment issue will be created for your review once this PR is deployed to production.
E/E issue linked to the PR - no internal issue.

If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created.

melvin-bot · 2026-04-30T04:56:19Z

@roryabraham looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

OSBotify · 2026-04-30T05:06:08Z

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

OSBotify · 2026-04-30T05:08:39Z

✋ 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-04-30T05:20:47Z

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

github-actions · 2026-04-30T05:56:26Z

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

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-20e6a09-d1461d1-fd9ab768cd78f9355046aeb1706d96e9f08aa6eb/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-20e6a09-d1461d1-9f80ea0a65f787c83cba431cb7d618776ff0d3a2/index.html

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

👀 View the workflow run that generated this build 👀

OSBotify · 2026-04-30T23:06:04Z

🚀 Deployed to staging by https://github.com/roryabraham in version: 9.3.65-0 🚀

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

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-04-30T23:07:56Z

No help site changes are needed for this PR. It's a behavior-neutral refactor (removing manual memoization for React Compiler compliance) with no user-facing changes to features, UI, or terminology.

OSBotify · 2026-05-04T13:26:38Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.3.65-6 🚀

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

melvin-bot · 2026-05-04T13:37:00Z

🤖 Payment issue created: #89504

melvin-bot Bot assigned roryabraham Apr 21, 2026

roryabraham added 5 commits April 20, 2026 18:08

refactor(MoneyRequestView): drop useMemo for hasMultipleSplits

f7727bc

React Compiler handles the memoization; removing the manual useMemo resolves the 'Existing memoization could not be preserved' RC compliance failure. Part of #68765. Made-with: Cursor

refactor(IOURequestStepAccountant): drop all useCallback wraps

78fb034

Removes all five useCallback wraps. The two Onyx selectors are now plain inline const functions (this file is their only caller), and the event handlers are plain arrow functions. Part of #68765. Made-with: Cursor

roryabraham force-pushed the rory-rc-memoization-cleanup branch from 3390092 to 829c388 Compare April 21, 2026 01:09

roryabraham added 2 commits April 20, 2026 18:11

chore: lower eslint --max-warnings to 301

59718f8

Accounts for the 6 warnings removed by the RC memoization cleanup. Part of #68765. Made-with: Cursor

roryabraham force-pushed the rory-rc-memoization-cleanup branch from 829c388 to 59718f8 Compare April 21, 2026 01:11

roryabraham mentioned this pull request Apr 21, 2026

[Tracking] Use React Compiler to enforce the Rules of React #68765

Open

roryabraham marked this pull request as ready for review April 21, 2026 20:30

roryabraham requested review from a team as code owners April 21, 2026 20:30

melvin-bot Bot requested review from JmillsExpensify and marcochavezf and removed request for a team April 21, 2026 20:30

roryabraham removed the request for review from JmillsExpensify April 21, 2026 20:31

roryabraham requested a review from situchan April 21, 2026 20:35

This comment was marked as resolved.

Sign in to view

situchan reviewed Apr 22, 2026

View reviewed changes

Comment thread src/components/ReportActionItem/MoneyRequestView.tsx Outdated

situchan reviewed Apr 29, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

roryabraham requested a review from situchan April 29, 2026 23:09

situchan approved these changes Apr 30, 2026

View reviewed changes

melvin-bot Bot changed the title ~~[No QA] Remove manual memoization from 6 RC-failing files~~ [Payment due @situchan] [No QA] Remove manual memoization from 6 RC-failing files Apr 30, 2026

marcochavezf approved these changes Apr 30, 2026

View reviewed changes

roryabraham merged commit 20e6a09 into main Apr 30, 2026
76 of 78 checks passed

roryabraham deleted the rory-rc-memoization-cleanup branch April 30, 2026 04:56

melvin-bot Bot added the Emergency label Apr 30, 2026

OSBotify mentioned this pull request Apr 30, 2026

Deploy Checklist: New Expensify 2026-04-30 #89315

Closed

melvin-bot Bot mentioned this pull request May 4, 2026

[Due for payment 2026-05-11] Payment issue for E/App PR#88370 #89504

Open

Conversation

roryabraham commented Apr 21, 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!

melvin-bot Bot commented Apr 21, 2026

Uh oh!

codecov Bot commented Apr 21, 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!

melvin-bot Bot commented Apr 21, 2026

Uh oh!

roryabraham commented Apr 21, 2026

Uh oh!

situchan commented Apr 21, 2026

Uh oh!

roryabraham commented Apr 21, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

MelvinBot commented Apr 22, 2026

1. IOURequestStepAccountant.tsx — selector reference stability

2. Review.tsx — useFocusEffect without useCallback

3. IOURequestStepDistanceManual.tsx — removed formError-clearing effect (behavioral change)

4. IOURequestStepDistanceManual.tsx — currentTransaction removal (potential split-edit regression)

5. --max-warnings reduction (313 → 301)

Uh oh!

situchan commented Apr 29, 2026

Uh oh!

situchan commented Apr 29, 2026

Uh oh!

situchan commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

situchan commented Apr 29, 2026

Uh oh!

situchan commented Apr 29, 2026

Uh oh!

situchan Apr 29, 2026 • 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!

roryabraham Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

situchan commented Apr 29, 2026

Uh oh!

situchan Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

situchan Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

roryabraham Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

situchan Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

marcochavezf Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented Apr 29, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

roryabraham commented Apr 29, 2026

Uh oh!

melvin-bot Bot commented Apr 30, 2026

Uh oh!

roryabraham commented Apr 21, 2026 •

edited

Loading

codecov Bot commented Apr 21, 2026 •

edited by github-actions Bot

Loading

1. `IOURequestStepAccountant.tsx` — selector reference stability

2. `Review.tsx` — `useFocusEffect` without `useCallback`

3. `IOURequestStepDistanceManual.tsx` — removed `formError`-clearing effect (behavioral change)

4. `IOURequestStepDistanceManual.tsx` — `currentTransaction` removal (potential split-edit regression)

5. `--max-warnings` reduction (313 → 301)

situchan commented Apr 29, 2026 •

edited

Loading

situchan Apr 29, 2026 •

edited

Loading