Skip to content

Perf/lhn options cache #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Bar-Maz wants to merge 11 commits into from
Closed

Perf/lhn options cache #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
173d127
Add cached actions and last-message text in OptionsListUtils
Bar-Maz Feb 3, 2026
17ee5be
Fix OptionsListUtilsTest: isolate getLastMessageTextForReport tests w…
Bar-Maz Feb 4, 2026
c89e04a
Merge branch 'perf/lhn-baselines' into perf/lhn-options-cache
Bar-Maz Feb 5, 2026
e267744
Merge branch 'perf/lhn-baselines' into perf/lhn-options-cache
Bar-Maz Feb 5, 2026
bac0e47
Merge branch 'perf/lhn-sort-cache' into perf/lhn-options-cache
Bar-Maz Feb 6, 2026
832b3b6
Remove deprecated Onyx.connect calls
Bar-Maz Feb 9, 2026
89eb0f4
Merge branch 'perf/lhn-sort-cache' into perf/lhn-options-cache
Bar-Maz Feb 10, 2026
00b0a59
Merge branch 'perf/lhn-sort-cache' into perf/lhn-options-cache
Bar-Maz Feb 11, 2026
4149f87
Delete unnecessary SidebarLinks tests
Bar-Maz Feb 11, 2026
a8d5ff5
Delete unused sidebarReports test utils
Bar-Maz Feb 11, 2026
71cfc0c
Merge branch 'perf/lhn-baselines' into perf/lhn-options-cache
Bar-Maz Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 117 additions & 11 deletions src/libs/OptionsListUtils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@
isTaskAction,
isThreadParentMessage,
isUnapprovedAction,
isWhisperAction,
shouldReportActionBeVisible,
withDEWRoutedActionsArray,
} from '@libs/ReportActionsUtils';
import {computeReportName} from '@libs/ReportNameUtils';
Expand Down Expand Up @@ -197,13 +199,13 @@
*/

let allPersonalDetails: OnyxEntry<PersonalDetailsList>;
Onyx.connect({
Onyx.connectWithoutView({
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => (allPersonalDetails = isEmptyObject(value) ? {} : value),
});

const policies: OnyxCollection<Policy> = {};
Onyx.connect({
Onyx.connectWithoutView({
key: ONYXKEYS.COLLECTION.POLICY,
callback: (policy, key) => {
if (!policy || !key || !policy.name) {
Expand All @@ -215,14 +217,14 @@
});

let allPolicies: OnyxCollection<Policy> = {};
Onyx.connect({
Onyx.connectWithoutView({
key: ONYXKEYS.COLLECTION.POLICY,
waitForCollectionCallback: true,
callback: (val) => (allPolicies = val),
});

let allReports: OnyxCollection<Report>;
Onyx.connect({
Onyx.connectWithoutView({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -231,18 +233,98 @@
});

let allReportNameValuePairsOnyxConnect: OnyxCollection<ReportNameValuePairs>;
Onyx.connect({

const lastReportActions: ReportActions = {};
const allSortedReportActions: Record<string, ReportAction[]> = {};
let allReportActions: OnyxCollection<ReportActions>;
const lastVisibleReportActions: ReportActions = {};
const filteredReportActionsCache: Record<string, ReportAction[]> = {};
const lastMessageTextCache: Record<string, string> = {};

/**
* Invalidates and recomputes cache entries for a specific report.
* This is called when report actions, metadata, or archived status changes.
*
* @param reportID - The ID of the report to invalidate cache for
*/
function invalidateCacheForReport(reportID: string) {
Copy link
Copy Markdown

@LukasMod LukasMod Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one also relies on allPersonalDetails, allReports, and allReportNameValuePairsOnyxConnect. Is it intentional that the cache is invalidated only for REPORT_METADATA and REPORT_ACTIONS? Will this not introduce stale data if eg only allPersonalDetails changes?

if (!reportID) {
return;
}

delete lastMessageTextCache[reportID];
delete filteredReportActionsCache[reportID];

if (allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]) {
const reportActionsArray = Object.values(allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? {});
const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`];
const transactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]);

let sortedReportActions = getSortedReportActions(withDEWRoutedActionsArray(reportActionsArray), true);
if (transactionThreadReportID) {
const transactionThreadReportActionsArray = Object.values(allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReportID}`] ?? {});
sortedReportActions = getCombinedReportActions(sortedReportActions, transactionThreadReportID, transactionThreadReportActionsArray, reportID);
}
allSortedReportActions[reportID] = sortedReportActions;

const reportNameValuePairs = allReportNameValuePairsOnyxConnect?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportID}`];
const isReportArchived = !!reportNameValuePairs?.private_isArchived;
const isWriteActionAllowed = canUserPerformWriteAction(report, isReportArchived);

const reportActionsForDisplay = sortedReportActions.filter(
(reportAction) =>
(!(isWhisperAction(reportAction) && !isReportPreviewAction(reportAction) && !isMoneyRequestAction(reportAction)) || isActionableMentionWhisper(reportAction)) &&
shouldReportActionBeVisible(reportAction, reportAction.reportActionID, isWriteActionAllowed) &&
reportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED &&
reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
);
Comment on lines +275 to +281
Copy link
Copy Markdown

@JKobrynski JKobrynski Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could make this part more readable, or maybe explain it somehow. I'm afraid it might be difficult to maintain. Wdyt?

filteredReportActionsCache[reportID] = reportActionsForDisplay;

const reportActionForDisplay = reportActionsForDisplay.at(0);
if (!reportActionForDisplay) {
delete lastVisibleReportActions[reportID];
return;
}
lastVisibleReportActions[reportID] = reportActionForDisplay;

const lastActorAccountID = report?.lastActorAccountID;
let lastActorDetails: Partial<PersonalDetails> | null = lastActorAccountID && allPersonalDetails?.[lastActorAccountID] ? allPersonalDetails[lastActorAccountID] : null;
Copy link
Copy Markdown

@JKobrynski JKobrynski Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let lastActorDetails: Partial<PersonalDetails> | null = lastActorAccountID && allPersonalDetails?.[lastActorAccountID] ? allPersonalDetails[lastActorAccountID] : null;
let lastActorDetails = allPersonalDetails?.[lastActorAccountID] ?? null;

Would that work?


if (!lastActorDetails && reportActionForDisplay) {
const lastActorDisplayName = reportActionForDisplay.person?.[0]?.text;
lastActorDetails = lastActorDisplayName
? {
displayName: lastActorDisplayName,
accountID: lastActorAccountID,
}
: null;
}
}
Comment on lines +294 to +303
Copy link
Copy Markdown

@LukasMod LukasMod Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a dead code? Is lastActorDetails used somewhere?

}

Onyx.connectWithoutView({
key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS,
waitForCollectionCallback: true,
callback: (value) => {
allReportNameValuePairsOnyxConnect = value;
},
});

const lastReportActions: ReportActions = {};
const allSortedReportActions: Record<string, ReportAction[]> = {};
let allReportActions: OnyxCollection<ReportActions>;
Onyx.connect({
Onyx.connectWithoutView({
key: ONYXKEYS.COLLECTION.REPORT_METADATA,
callback: (value, key) => {
if (!key) {
return;
}
const reportID = key.split('_').at(1);
if (reportID) {
invalidateCacheForReport(reportID);
}
},
});

Onyx.connectWithoutView({
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
waitForCollectionCallback: true,
callback: (actions) => {
Expand All @@ -265,7 +347,7 @@
const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`];

// If the report is a one-transaction report and has , we need to return the combined reportActions so that the LHN can display modifications
// If the report is a one-transaction report, we need to return the combined reportActions so that the LHN can display modifications
// to the transaction thread or the report itself
const transactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, actions[reportActions[0]]);
if (transactionThreadReportID) {
Expand All @@ -280,12 +362,13 @@
} else {
lastReportActions[reportID] = firstReportAction;
}
invalidateCacheForReport(reportID);
}
},
});

let activePolicyID: OnyxEntry<string>;
Onyx.connect({
Onyx.connectWithoutView({
key: ONYXKEYS.NVP_ACTIVE_POLICY_ID,
callback: (value) => (activePolicyID = value),
});
Expand Down Expand Up @@ -321,6 +404,27 @@
return personalDetailsForAccountIDs;
}

function getCachedReportActionsForDisplay(reportID: string): ReportAction[] {
return filteredReportActionsCache[reportID] ?? [];
}

/**
* Get cached last message text for a report, or compute and cache it if missing.
* This uses lazy computation to avoid using deprecated translateLocal in module-level Onyx.connect callbacks.
*
* @param reportID - The report ID to get cached message text for
* @param computeFn - Optional function to compute the message text if cache miss. If provided, result will be cached.
* @returns Cached message text, or undefined if cache miss and no computeFn provided
*/
function getCachedLastMessageText(reportID: string, computeFn?: () => string): string | undefined {
let cached = lastMessageTextCache[reportID];
if (!cached && computeFn) {
cached = computeFn();
lastMessageTextCache[reportID] = cached;
}
return cached;
}
Comment on lines +419 to +426
Copy link
Copy Markdown

@LukasMod LukasMod Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about empty string? Not sure if its a real issue but it would cause cache miss. Would be nice to have a unit tests for that


/**
* Return true if personal details data is ready, i.e. report list options can be created.
*/
Expand Down Expand Up @@ -467,11 +571,11 @@
const formattedLastMessageTextWithPrefix = reportPrefix + formattedLastMessageText;

if (isExpenseThread || option.isMoneyRequestReport) {
return showChatPreviewLine && formattedLastMessageText ? formattedLastMessageTextWithPrefix : translateFn('iou.expense');

Check failure on line 574 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

`translateFn` is deprecated. This function uses imperative Onyx data access patterns, similar to `Onyx.connect`. Use `useLocalize` hook instead for reactive data access in React components

Check failure on line 574 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

`translateFn` is deprecated. This function uses imperative Onyx data access patterns, similar to `Onyx.connect`. Use `useLocalize` hook instead for reactive data access in React components
}

if (option.isThread) {
return showChatPreviewLine && formattedLastMessageText ? formattedLastMessageTextWithPrefix : translateFn('threads.thread');

Check failure on line 578 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

`translateFn` is deprecated. This function uses imperative Onyx data access patterns, similar to `Onyx.connect`. Use `useLocalize` hook instead for reactive data access in React components

Check failure on line 578 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

`translateFn` is deprecated. This function uses imperative Onyx data access patterns, similar to `Onyx.connect`. Use `useLocalize` hook instead for reactive data access in React components
}

if (option.isChatRoom && !isAdminRoom && !isAnnounceRoom) {
Expand All @@ -483,11 +587,11 @@
}

if (option.isTaskReport) {
return showChatPreviewLine && formattedLastMessageText ? formattedLastMessageTextWithPrefix : translateFn('task.task');

Check failure on line 590 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

`translateFn` is deprecated. This function uses imperative Onyx data access patterns, similar to `Onyx.connect`. Use `useLocalize` hook instead for reactive data access in React components

Check failure on line 590 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

`translateFn` is deprecated. This function uses imperative Onyx data access patterns, similar to `Onyx.connect`. Use `useLocalize` hook instead for reactive data access in React components
}

if (isGroupChat) {
return showChatPreviewLine && formattedLastMessageText ? formattedLastMessageTextWithPrefix : translateFn('common.group');

Check failure on line 594 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

`translateFn` is deprecated. This function uses imperative Onyx data access patterns, similar to `Onyx.connect`. Use `useLocalize` hook instead for reactive data access in React components

Check failure on line 594 in src/libs/OptionsListUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

`translateFn` is deprecated. This function uses imperative Onyx data access patterns, similar to `Onyx.connect`. Use `useLocalize` hook instead for reactive data access in React components
}

return showChatPreviewLine && formattedLastMessageText
Expand Down Expand Up @@ -3216,6 +3320,8 @@
getLastActorDisplayName,
getLastActorDisplayNameFromLastVisibleActions,
getLastMessageTextForReport,
getCachedReportActionsForDisplay,
getCachedLastMessageText,
Comment on lines +3323 to +3324
Copy link
Copy Markdown

@LukasMod LukasMod Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see usage of these 2. Is it for follow up PRs or something?

getManagerMcTestParticipant,
getMemberInviteOptions,
getParticipantsOption,
Expand Down
Loading
Loading