Narrow per-row report actions subscriptions in ReportActionItem

src/hooks/useOriginalReportID.ts

@@ -3,13 +3,27 @@ import {getOneTransactionThreadReportID, withDEWRoutedActionsObject} from '@libs
 
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
-import type {OnyxInputOrEntry, ReportAction} from '@src/types/onyx';
+import type {OnyxInputOrEntry, Report, ReportAction, ReportActions} from '@src/types/onyx';
+
+import type {OnyxEntry} from 'react-native-onyx';
 
 import {useMemo} from 'react';
 
 import useNetwork from './useNetwork';
 import useOnyx from './useOnyx';
-import useTransactionsAndViolationsForReport from './useTransactionsAndViolationsForReport';
+import useReportTransactionsCollection from './useReportTransactionsCollection';
+
+function reportTypeSelector(report: OnyxEntry<Report>) {
+    return report?.type;
+}
+
+function chatReportIDSelector(report: OnyxEntry<Report>) {
+    return report?.chatReportID;
+}
+
+function parentReportIDSelector(report: OnyxEntry<Report>) {
+    return report?.parentReportID;
+}
 
 /**
  * This hook is for finding the "original reportID" for a given reportActionID. The reportID usually is the report we are looking at,
@@ -24,30 +38,40 @@ import useTransactionsAndViolationsForReport from './useTransactionsAndViolation
  *
  */
 function useOriginalReportID(reportID: string | undefined, reportAction: OnyxInputOrEntry<Pick<ReportAction, 'reportActionID' | 'childReportID'>>): string | undefined {
-    const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {selector: withDEWRoutedActionsObject});
-    const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
-    const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`);
-    const {isOffline} = useNetwork();
-    const {transactions: allReportTransactions} = useTransactionsAndViolationsForReport(reportID);
-
     const reportActionID = reportAction?.reportActionID;
-    const currentReportAction = reportActionID ? reportActions?.[reportActionID] : undefined;
-    const reportActionBelongsCurrentReport = Object.keys(currentReportAction ?? {}).length > 0;
+    const reportActionBelongsCurrentReportSelector = (actions: OnyxEntry<ReportActions>) => {
+        if (!reportActionID) {
+            return false;
+        }
+        if (Object.keys(actions?.[reportActionID] ?? {}).length > 0) {
+            return true;
+        }
+        return Object.keys(withDEWRoutedActionsObject(actions)?.[reportActionID] ?? {}).length > 0;
+    };
+    const [reportActionBelongsCurrentReport = false] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {selector: reportActionBelongsCurrentReportSelector});
+    const [reportType] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {selector: reportTypeSelector});
+    const [chatReportID] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {selector: chatReportIDSelector});
+    const [parentReportID] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {selector: parentReportIDSelector});
     const isThreadReportParentAction = reportAction?.childReportID?.toString() === reportID;
 
+    // The full actions, chat report and transactions are only needed to find a merged transaction thread, so rows whose action is in the current report don't subscribe to them
+    const lookupReportID = reportID && !reportActionBelongsCurrentReport && !isThreadReportParentAction && reportActionID ? reportID : undefined;
+    const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${lookupReportID}`, {selector: withDEWRoutedActionsObject});
+    const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${lookupReportID ? chatReportID : undefined}`);
+    const {isOffline} = useNetwork();
+    const allReportTransactions = useReportTransactionsCollection(lookupReportID);
+
     // This will only be found if the report with reportID is a report with a single transaction and we are merging reportActions
     const uniqueTransactionThreadReportID = useMemo(() => {
-        // These conditions are repeated with the conditions that make us return early below because there is no need to do expensive calculations
-        // on the transactions and reportActions if we are not going to use uniqueTransactionThreadReportID.
-        if (!reportID || reportActionBelongsCurrentReport || isThreadReportParentAction || !reportActionID) {
+        if (!lookupReportID) {
             return undefined;
         }
 
         const visibleTransactionsIDs = getAllNonDeletedTransactions(allReportTransactions, Object.values(reportActions ?? {}))
             .filter((transaction) => isOffline || transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)
             .map((transaction) => transaction.transactionID);
-        return getOneTransactionThreadReportID({type: report?.type}, chatReport, reportActions ?? ([] as ReportAction[]), isOffline, visibleTransactionsIDs);
-    }, [reportID, reportActionBelongsCurrentReport, isThreadReportParentAction, reportActionID, allReportTransactions, reportActions, report?.type, chatReport, isOffline]);
+        return getOneTransactionThreadReportID({type: reportType}, chatReport, reportActions ?? ([] as ReportAction[]), isOffline, visibleTransactionsIDs);
+    }, [lookupReportID, allReportTransactions, reportActions, reportType, chatReport, isOffline]);
 
     const [uniqueTransactionThreadReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${uniqueTransactionThreadReportID}`);
 
@@ -61,7 +85,7 @@ function useOriginalReportID(reportID: string | undefined, reportAction: OnyxInp
 
     if (isThreadReportParentAction) {
         // This reportAction is the parent action of a thread report, so the original reportID is the parentReportID
-        return report?.parentReportID;
+        return parentReportID;
     }
 
     // If we have a uniqueTransactionThreadReportID, then we are viewing an expense report with a single transaction and merging reportActions

src/hooks/useReportsParentHierarchy.ts

@@ -1,7 +1,8 @@
 import type {ReportHierarchyInfo} from '@userActions/ClearReportActionErrors';
 
 import ONYXKEYS from '@src/ONYXKEYS';
-import {reportsParentHierarchySelector} from '@src/selectors/Report';
+import {reportAncestorsHierarchySelector, reportsParentHierarchySelector} from '@src/selectors/Report';
+import type {Report} from '@src/types/onyx';
 
 import type {OnyxCollection} from 'react-native-onyx';
 
@@ -18,4 +19,15 @@ function useReportsParentHierarchy(): OnyxCollection<ReportHierarchyInfo> {
     return reportsParentHierarchy;
 }
 
+/**
+ * Subscribes to the parent hierarchy links of the given report and its ancestors only. Use this instead of
+ * `useReportsParentHierarchy` when the caller only walks up from one known report, e.g. in list rows.
+ */
+function useReportAncestorsHierarchy(reportID: string | undefined): OnyxCollection<ReportHierarchyInfo> {
+    const ancestorsSelector = (reports: OnyxCollection<Report>) => reportAncestorsHierarchySelector(reports, reportID);
+    const [reportAncestorsHierarchy] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {selector: ancestorsSelector});
+    return reportAncestorsHierarchy;
+}
+
 export default useReportsParentHierarchy;
+export {useReportAncestorsHierarchy};

src/pages/inbox/report/ReportActionItem.tsx

@@ -19,7 +19,7 @@ import useNetwork from '@hooks/useNetwork';
 import useOnyx from '@hooks/useOnyx';
 import useOriginalReportID from '@hooks/useOriginalReportID';
 import {usePersonalDetail} from '@hooks/usePersonalDetails';
-import useReportsParentHierarchy from '@hooks/useReportsParentHierarchy';
+import {useReportAncestorsHierarchy} from '@hooks/useReportsParentHierarchy';
 import useReportTransactionsCollection from '@hooks/useReportTransactionsCollection';
 import useResponsiveLayout from '@hooks/useResponsiveLayout';
 import useStyleUtils from '@hooks/useStyleUtils';
@@ -192,7 +192,7 @@ function ReportActionItem({
     const reportID = report?.reportID ?? action?.reportID;
     const originalReportID = useOriginalReportID(report?.reportID, action);
     const {isOffline} = useNetwork();
-    const reportsParentHierarchy = useReportsParentHierarchy();
+    const reportsParentHierarchy = useReportAncestorsHierarchy(reportID);
     const [iouReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getIOUReportIDFromReportActionPreview(action)}`, {selector: getStableReportSelector});
     const [iouPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${iouReport?.policyID}`);
 

src/selectors/Report.ts

@@ -379,6 +379,25 @@ function reportHierarchySelector(report: OnyxEntry<Report>): OnyxEntry<ReportHie
 function reportsParentHierarchySelector(reports: OnyxCollection<Report>): OnyxCollection<ReportHierarchyInfo> {
     return mapOnyxCollectionItems(reports, reportHierarchySelector);
 }
+
+/**
+ * Returns the parent hierarchy links of the given report and its ancestors only, which is all that walking up an
+ * ancestor chain reads, so the output stays the same when unrelated reports are added or removed.
+ */
+function reportAncestorsHierarchySelector(reports: OnyxCollection<Report>, reportID: string | undefined): OnyxCollection<ReportHierarchyInfo> {
+    const result: OnyxCollection<ReportHierarchyInfo> = {};
+    let currentReportID = reportID;
+    while (currentReportID) {
+        const key = `${ONYXKEYS.COLLECTION.REPORT}${currentReportID}` as const;
+        const report = reports?.[key];
+        if (!report || key in result) {
+            break;
+        }
+        result[key] = reportHierarchySelector(report);
+        currentReportID = report.parentReportID;
+    }
+    return result;
+}
 /**
  * Creates a selector returning only the reports for the given IDs, so a consumer that knows the exact
  * reports it needs doesn't re-render when any other report in the account changes.
@@ -395,6 +414,7 @@ const reportsByIDsSelector =
     };
 
 export {
+    reportAncestorsHierarchySelector,
     getArchiveReason,
     getReportChatType,
     groupChatAvatarReportSelector,

tests/perf-test/ReportActionsListRowWrites.perf-test.tsx

@@ -0,0 +1,189 @@
+import {act, screen} from '@testing-library/react-native';
+
+import OnyxListItemProvider from '@components/OnyxListItemProvider';
+
+import type Navigation from '@libs/Navigation/Navigation';
+import navigationRef from '@libs/Navigation/navigationRef';
+import {setHasRadio} from '@libs/NetworkState';
+
+import {ActionListContext} from '@pages/inbox/ActionListContext';
+import {ReactionListContext} from '@pages/inbox/ReactionListContext';
+import ReportActionsList from '@pages/inbox/report/ReportActionsList';
+import {AttachmentModalContextProvider} from '@pages/media/AttachmentModalScreen/AttachmentModalContext';
+
+import ComposeProviders from '@src/components/ComposeProviders';
+import {LocaleContextProvider} from '@src/components/LocaleContextProvider';
+import CONST from '@src/CONST';
+import ONYXKEYS from '@src/ONYXKEYS';
+import type {ReportAction, ReportActions} from '@src/types/onyx';
+
+import {NavigationContainer} from '@react-navigation/native';
+import Onyx from 'react-native-onyx';
+import {measureRenders} from 'reassure';
+
+import * as ReportTestUtils from '../utils/ReportTestUtils';
+import * as TestHelper from '../utils/TestHelper';
+import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
+import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatchedUpdates';
+
+const mockedNavigate = jest.fn();
+
+jest.mock('@react-navigation/native', () => {
+    const actualNav = jest.requireActual<typeof Navigation>('@react-navigation/native');
+    return {
+        ...actualNav,
+        useRoute: () => mockedNavigate,
+        useIsFocused: () => true,
+    };
+});
+
+beforeAll(() => {
+    Onyx.init({
+        keys: ONYXKEYS,
+        evictableKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS],
+    });
+    // Required lazily so the navigation mock is registered before the engine's imports load it
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
+    require('@userActions/OnyxDerived').default();
+});
+
+const mockOnLayout = jest.fn();
+// Built via a function so the value isn't an inline literal the context-split lint rule would flag; these are all refs/accessors with no re-render concern.
+function buildActionListContextValue() {
+    return {scrollOffsetRef: {current: 0}, getScrollOffset: () => 0, registerListRef: () => {}, getListRef: () => null};
+}
+const actionListContextValue = buildActionListContextValue();
+const mockReactionListContextValue = {
+    showReactionList: () => {},
+    hideReactionList: () => {},
+    isActiveReportAction: () => false,
+};
+
+const TEST_USER_ACCOUNT_ID = 1;
+const TEST_USER_LOGIN = 'test@test.com';
+const REPORT_ID = '1';
+
+const signUpWithTestUser = () => {
+    TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN);
+};
+
+const sortedReportActions = ReportTestUtils.getMockedSortedReportActions(500);
+const reportActions: ReportActions = Object.fromEntries(sortedReportActions.map((action: ReportAction) => [action.reportActionID, action]));
+const POLICY_ID = 'P1';
+const report = ReportTestUtils.createMockReport({reportID: REPORT_ID, policyID: POLICY_ID, lastVisibleActionCreated: sortedReportActions.at(0)?.created});
+const WRITES_PER_SCENARIO = 10;
+
+// All mock actions share one timestamp, so the initial window is ordered by reportActionID as a string and starts at action 1, then 10..
+const VISIBLE_ACTION_ID = '10';
+
+beforeEach(async () => {
+    // Initialize the network key for OfflineWithFeedback
+    setHasRadio(true);
+    wrapOnyxWithWaitForBatchedUpdates(Onyx);
+    // Pre-seed the locale so LocaleContextProvider's mount effect is a no-op (setLocale early-returns),
+    // avoiding post-mount Onyx writes that would re-render outside act().
+    await act(async () => {
+        signUpWithTestUser();
+        await Onyx.merge(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.DEFAULT);
+
+        // Seed the report under test: the report, its 500 actions, and a settled loading state.
+        await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
+        await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, {
+            id: POLICY_ID,
+            name: 'Policy',
+            type: CONST.POLICY.TYPE.TEAM,
+            role: CONST.POLICY.ROLE.ADMIN,
+            owner: TEST_USER_LOGIN,
+            outputCurrency: CONST.CURRENCY.USD,
+        });
+        await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, reportActions);
+        await Onyx.set(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${REPORT_ID}`, {
+            isLoadingInitialReportActions: false,
+            hasOnceLoadedReportActions: true,
+            isLoadingOlderReportActions: false,
+            hasLoadingOlderReportActionsError: false,
+            isLoadingNewerReportActions: false,
+            hasLoadingNewerReportActionsError: false,
+        });
+        await waitForBatchedUpdates();
+    });
+});
+
+afterEach(async () => {
+    // Await the clear so its broadcasts settle in teardown instead of leaking into the next test.
+    await Onyx.clear();
+    await waitForBatchedUpdates();
+});
+
+// Mirror the production data flow (usePaginatedReportActions): the body reads REPORT_ACTIONS from Onyx and
+// sorts inside a useOnyx selector, then runs it through the real continuous-chain pagination and the
+// visibility filter.
+function ReportActionsListWrapper() {
+    return (
+        <NavigationContainer ref={navigationRef}>
+            <ComposeProviders components={[OnyxListItemProvider, LocaleContextProvider, AttachmentModalContextProvider]}>
+                <ReactionListContext.Provider value={mockReactionListContextValue}>
+                    <ActionListContext.Provider value={actionListContextValue}>
+                        <ReportActionsList
+                            reportID={REPORT_ID}
+                            conciergeChat={undefined}
+                            onLayout={mockOnLayout}
+                        />
+                    </ActionListContext.Provider>
+                </ReactionListContext.Provider>
+            </ComposeProviders>
+        </NavigationContainer>
+    );
+}
+
+async function measureWrites(write: (run: number, index: number) => Promise<unknown>) {
+    // Reassure runs the scenario several times against one seeded Onyx, so every run writes a value that doesn't exist yet
+    let run = 0;
+    const scenario = async () => {
+        await screen.findByTestId('report-actions-list');
+        const currentRun = run;
+        for (let index = 0; index < WRITES_PER_SCENARIO; index++) {
+            await act(async () => {
+                await write(currentRun, index);
+                await waitForBatchedUpdates();
+            });
+        }
+        run++;
+    };
+    await waitForBatchedUpdates();
+    await measureRenders(<ReportActionsListWrapper />, {scenario});
+}
+
+const getTime = (run: number, index: number) => `2030-01-01 00:${String(run % 60).padStart(2, '0')}:${String(index).padStart(2, '0')}.000`;
+
+test('[ReportActionsList] should re-render when one visible action is edited', async () => {
+    // Given 500 actions in the report, when the message of one visible action is edited repeatedly
+    // Then reassure measures what an edit of one action costs the list
+    await measureWrites((run, index) =>
+        Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {[VISIBLE_ACTION_ID]: {message: [{type: 'TEXT', html: `edited ${run}-${index}`, text: `edited ${run}-${index}`}]}}),
+    );
+});
+
+test('[ReportActionsList] should re-render when only a heartbeat field of the report changes', async () => {
+    // Given 500 actions in the report, when only lastReadTime of the report changes
+    // Then reassure measures what a field the rows don't display costs the list
+    await measureWrites((run, index) => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, {lastReadTime: getTime(run, index)}));
+});
+
+test('[ReportActionsList] should re-render when an unrelated report is written', async () => {
+    // Given 500 actions in the report, when another report in the account is created or updated
+    // Then reassure measures what a write outside the open report costs the list
+    await measureWrites((run, index) => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}unrelated${run}_${index}`, {reportID: `unrelated${run}_${index}`, reportName: 'Unrelated'}));
+});
+
+test('[ReportActionsList] should re-render when an unrelated transaction is written', async () => {
+    // Given 500 actions in the report, when a transaction of another report changes
+    // Then reassure measures what a write outside the open report costs the list
+    await measureWrites((run, index) => Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}T1`, {transactionID: 'T1', reportID: 'unrelated', amount: run * 100 + index}));
+});
+
+test('[ReportActionsList] should re-render when the report name changes', async () => {
+    // Given 500 actions in the report, when a field every row reads through the stable report changes
+    // Then reassure measures the cost of a write that legitimately re-renders all rows
+    await measureWrites((run, index) => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, {reportName: `Renamed ${run}-${index}`}));
+});