Skip to content

[Performance] Improve getOrderedReportIDs performance #43539

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

Merged
mountiny merged 6 commits into from
Jun 13, 2024
Merged

[Performance] Improve getOrderedReportIDs performance #43539

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
59e5f83
draft: measure times
kosmydel Jun 10, 2024
57771ff
feat: do not copy reports
kosmydel Jun 10, 2024
aa2f789
cleanup: remove redundant timings
kosmydel Jun 10, 2024
25df8db
Merge branch 'main' into @kosmydel/perf/mini-reports
kosmydel Jun 12, 2024
11af4fd
cleanup: remove timings
kosmydel Jun 12, 2024
cfaaf5c
address review: add JSDoc
kosmydel Jun 12, 2024
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
39 changes: 24 additions & 15 deletions src/libs/SidebarUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ function compareStringDates(a: string, b: string): 0 | 1 | -1 {
return 0;
}

/**
* A mini report object that contains only the necessary information to sort reports.
* This is used to avoid copying the entire report object and only the necessary information.
*/
type MiniReport = {
Copy link
Copy Markdown
Contributor

@mountiny mountiny Jun 13, 2024

Choose a reason for hiding this comment

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

I think we could have called this as MinimalSidebarReport or something like that to indicate this is specifically the minimum we need for the Sidebar, but NAB

reportID?: string;
displayName: string;
lastVisibleActionCreated?: string;
};

/**
* @returns An array of reportIDs sorted in the proper order
*/
Expand Down Expand Up @@ -132,10 +142,10 @@ function getOrderedReportIDs(
// 4. Archived reports
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
const pinnedAndGBRReports: Array<OnyxEntry<Report>> = [];
const draftReports: Array<OnyxEntry<Report>> = [];
const nonArchivedReports: Array<OnyxEntry<Report>> = [];
const archivedReports: Array<OnyxEntry<Report>> = [];
const pinnedAndGBRReports: MiniReport[] = [];
const draftReports: MiniReport[] = [];
const nonArchivedReports: MiniReport[] = [];
const archivedReports: MiniReport[] = [];

if (currentPolicyID || policyMemberAccountIDs.length > 0) {
reportsToDisplay = reportsToDisplay.filter(
Expand All @@ -144,24 +154,23 @@ function getOrderedReportIDs(
}
// There are a few properties that need to be calculated for the report which are used when sorting reports.
reportsToDisplay.forEach((reportToDisplay) => {
let report = reportToDisplay as OnyxEntry<Report>;
if (report) {
report = {
...report,
displayName: ReportUtils.getReportName(report),
};
}
const report = reportToDisplay as OnyxEntry<Report>;
const miniReport: MiniReport = {
reportID: report?.reportID,
displayName: ReportUtils.getReportName(report),
lastVisibleActionCreated: report?.lastVisibleActionCreated,
};

const isPinned = report?.isPinned ?? false;
const reportAction = ReportActionsUtils.getReportAction(report?.parentReportID ?? '', report?.parentReportActionID ?? '');
if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) {
pinnedAndGBRReports.push(report);
pinnedAndGBRReports.push(miniReport);
} else if (hasValidDraftComment(report?.reportID ?? '')) {
draftReports.push(report);
draftReports.push(miniReport);
} else if (ReportUtils.isArchivedRoom(report)) {
archivedReports.push(report);
archivedReports.push(miniReport);
} else {
nonArchivedReports.push(report);
nonArchivedReports.push(miniReport);
}
});

Expand Down