Conversation
Remove useMemo/useCallback wrappers from 12 files where the compiler reported "Existing memoization could not be preserved" errors. The compiler's inferred dependencies didn't match the manually specified ones, causing it to bail on those functions entirely. With the manual memoization removed, the compiler can now auto-memoize these files. DebugReportPage was excluded because its DebugDetailsTab useCallback wraps an inline component used as a route — removing it triggers react/no-unstable-nested-components errors that require a larger refactoring. Co-authored-by: Cursor <cursoragent@cursor.com>
React Compiler automatically memoizes inline useOnyx selectors, making the rulesdir/no-inline-useOnyx-selector lint rule unnecessary for files that compile successfully. Extend the compat processor to suppress it. Co-authored-by: Cursor <cursoragent@cursor.com>
These files compile with React Compiler, so the compat processor now automatically suppresses rulesdir/no-inline-useOnyx-selector for them. The manual eslint-disable-next-line comments are no longer needed. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the useEffect that initialized selectedOptions with the React-recommended pattern of adjusting state during render. The effect was synchronously calling setState to pre-select all members on first load, which triggers cascading renders. The inline state adjustment achieves the same result without the extra render cycle. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
Requested AdHoc testing: https://expensify.slack.com/archives/C09V78U42D8/p1771464087542039 |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment was marked as outdated.
This comment was marked as outdated.
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # src/components/ReportActionItem/MoneyRequestView.tsx # src/components/SelectionListWithSections/Search/TransactionGroupListItem.tsx # src/pages/OnboardingWorkspaces/BaseOnboardingWorkspaces.tsx
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # src/components/Navigation/NavigationTabBar/index.tsx
Remove manual memoization (useCallback, useMemo) from SearchTabButton and WorkspacesTabButton, and use the shared TabBarItem component internally to eliminate duplicated Icon+Text rendering logic. Remove the now-unused getTabIconFill utility. Fix nested ternary lint error in index.tsx. Co-authored-by: Cursor <cursoragent@cursor.com>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # src/components/Navigation/NavigationTabBar/SearchTabButton.tsx # src/components/Navigation/NavigationTabBar/WorkspacesTabButton.tsx # src/components/ReportActionItem/MoneyRequestView.tsx
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
npm has a |
|
@mjasikowski 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] |
Restore react-native-live-markdown, react-native-onyx, app version, and prettier script to match main. Only the --max-warnings lint count should differ. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@joekaufmanexpensify no product considerations |
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # src/pages/workspace/WorkspacesListPage.tsx
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
mountiny
left a comment
There was a problem hiding this comment.
QA completed successfully on the PR, to avoid conflicts, Rory asked me to review and merge
|
The android build seems to come from main |
|
@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
Explained above, we just merged changes to the deploy / build flow so that is leading to some build issues |
|
✋ 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! 🧪🧪
|
Explanation of Change
Second batch of refactors to make components compliant with React Compiler / React best practices.
This batch focuses on components which only fail to compile because manual memoization could not be preserved, but which couldn't be refactored in batch 1 because of conflicting ESLint rules (fixed by our custom ESLint processor that runs React Compiler and conditionally suppresses certain rules that don't make sense in a component that's compiled successfully).
Fixed Issues
(partial) #68765
Tests
This is a refactoring PR that removes manual memoization (useMemo/useCallback/React.memo) from components that successfully compile with React Compiler, and improves adherence to React best practices (removing unnecessary effects, adjusting state during rendering). No behavioral changes are intended.
Domain pages (DomainAddPrimaryContactPage, DomainAdminDetailsPage, BaseDomainMemberDetailsComponent):
Report search grouping (TransactionGroupListItem):
Company cards (SelectCountryStep, InviteNewMemberStep):
Workspace selection/creation (NewReportWorkspaceSelectionPage, WorkspacesListPage):
Onboarding (BaseOnboardingWorkspaces):
Money request / IOU (MoneyRequestView, IOURequestStepReport):
Share (ShareTab):
Accounting integrations (NetSuiteExportConfigurationPage):
TC-5161: Accounting Integration Status - Subtitle Logo Verification
Verify that no errors appear in the JS console
Offline tests
n/a
QA Steps
None, covered by regression tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari