From 82d0f7cc371601fe878e60eb020195d3e606f024 Mon Sep 17 00:00:00 2001 From: Vit Horacek <36083550+mountiny@users.noreply.github.com> Date: Fri, 3 Nov 2023 14:44:19 +0000 Subject: [PATCH] Revert "perf: refactor heavy operations when user starts to type" --- .../LHNOptionsList/OptionRowLHNData.js | 7 +---- src/libs/OptionsListUtils.js | 1 - src/libs/ReportActionsUtils.ts | 7 ++--- src/libs/ReportUtils.js | 26 +++--------------- src/libs/SidebarUtils.ts | 22 ++++++++------- src/libs/UnreadIndicatorUpdater/index.js | 27 ++----------------- src/libs/actions/Report.js | 1 - .../ComposerWithSuggestions.js | 10 +++---- src/pages/home/sidebar/SidebarLinksData.js | 5 ---- 9 files changed, 24 insertions(+), 82 deletions(-) diff --git a/src/components/LHNOptionsList/OptionRowLHNData.js b/src/components/LHNOptionsList/OptionRowLHNData.js index cb64a135b264..ebba2ffe0587 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.js +++ b/src/components/LHNOptionsList/OptionRowLHNData.js @@ -168,17 +168,14 @@ export default React.memo( }, fullReport: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, - initialValue: {}, }, reportActions: { key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, canEvict: false, - initialValue: {}, }, personalDetails: { key: ONYXKEYS.PERSONAL_DETAILS_LIST, selector: personalDetailsSelector, - initialValue: {}, }, preferredLocale: { key: ONYXKEYS.NVP_PREFERRED_LOCALE, @@ -189,17 +186,15 @@ export default React.memo( parentReportActions: { key: ({fullReport}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${fullReport.parentReportID}`, canEvict: false, - initialValue: {}, }, policy: { key: ({fullReport}) => `${ONYXKEYS.COLLECTION.POLICY}${fullReport.policyID}`, - initialValue: {}, }, // Ideally, we aim to access only the last transaction for the current report by listening to changes in reportActions. // In some scenarios, a transaction might be created after reportActions have been modified. // This can lead to situations where `lastTransaction` doesn't update and retains the previous value. // However, performance overhead of this is minimized by using memos inside the component. - receiptTransactions: {key: ONYXKEYS.COLLECTION.TRANSACTION, initialValue: {}}, + receiptTransactions: {key: ONYXKEYS.COLLECTION.TRANSACTION}, }), // eslint-disable-next-line rulesdir/no-multiple-onyx-in-file withOnyx({ diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 99853975f86a..54d09b75eff2 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -87,7 +87,6 @@ Onyx.connect({ const policyExpenseReports = {}; Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT, - waitForCollectionCallback: true, callback: (report, key) => { if (!ReportUtils.isPolicyExpenseChat(report)) { return; diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 45bdfb18b451..11e11f549682 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -406,12 +406,9 @@ function getLastVisibleMessage(reportID: string, actionsToMerge: ReportActions = }; } - let messageText = message?.text ?? ''; - if (messageText) { - messageText = String(messageText).replace(CONST.REGEX.AFTER_FIRST_LINE_BREAK, '').substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim(); - } + const messageText = message?.text ?? ''; return { - lastMessageText: messageText, + lastMessageText: String(messageText).replace(CONST.REGEX.AFTER_FIRST_LINE_BREAK, '').substring(0, CONST.REPORT.LAST_MESSAGE_TEXT_MAX_LENGTH).trim(), }; } diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 5bb8fd4ad4fc..06408dd938f6 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -818,15 +818,8 @@ function isOneOnOneChat(report) { * @returns {Object} */ function getReport(reportID) { - /** - * Using typical string concatenation here due to performance issues - * with template literals. - */ - if (!allReports) { - return {}; - } - - return allReports[ONYXKEYS.COLLECTION.REPORT + reportID] || {}; + // Deleted reports are set to null and lodashGet will still return null in that case, so we need to add an extra check + return lodashGet(allReports, `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {}) || {}; } /** @@ -1530,25 +1523,14 @@ function getMoneyRequestSpendBreakdown(report, allReportsDict = null) { * @returns {String} */ function getPolicyExpenseChatName(report, policy = undefined) { - const ownerAccountID = report.ownerAccountID; - const personalDetails = allPersonalDetails[ownerAccountID]; - const login = personalDetails ? personalDetails.login : null; - const reportOwnerDisplayName = getDisplayNameForParticipant(report.ownerAccountID) || login || report.reportName; + const reportOwnerDisplayName = getDisplayNameForParticipant(report.ownerAccountID) || lodashGet(allPersonalDetails, [report.ownerAccountID, 'login']) || report.reportName; // If the policy expense chat is owned by this user, use the name of the policy as the report name. if (report.isOwnPolicyExpenseChat) { return getPolicyName(report, false, policy); } - let policyExpenseChatRole = 'user'; - /** - * Using typical string concatenation here due to performance issues - * with template literals. - */ - const policyItem = allPolicies[ONYXKEYS.COLLECTION.POLICY + report.policyID]; - if (policyItem) { - policyExpenseChatRole = policyItem.role || 'user'; - } + const policyExpenseChatRole = lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, 'role']) || 'user'; // If this user is not admin and this policy expense chat has been archived because of account merging, this must be an old workspace chat // of the account which was merged into the current user's account. Use the name of the policy as the name of the report. diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 4951432bcd03..4aa708d5882d 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -156,6 +156,18 @@ function getOrderedReportIDs( } } + // There are a few properties that need to be calculated for the report which are used when sorting reports. + reportsToDisplay.forEach((report) => { + // Normally, the spread operator would be used here to clone the report and prevent the need to reassign the params. + // However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add + // the reportDisplayName property to the report object directly. + // eslint-disable-next-line no-param-reassign + report.displayName = ReportUtils.getReportName(report); + + // eslint-disable-next-line no-param-reassign + report.iouReportAmount = ReportUtils.getMoneyRequestReimbursableTotal(report, allReports); + }); + // The LHN is split into four distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order: // 1. Pinned/GBR - Always sorted by reportDisplayName // 2. Drafts - Always sorted by reportDisplayName @@ -169,17 +181,7 @@ function getOrderedReportIDs( const draftReports: Report[] = []; const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; - reportsToDisplay.forEach((report) => { - // Normally, the spread operator would be used here to clone the report and prevent the need to reassign the params. - // However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add - // the reportDisplayName property to the report object directly. - // eslint-disable-next-line no-param-reassign - report.displayName = ReportUtils.getReportName(report); - - // eslint-disable-next-line no-param-reassign - report.iouReportAmount = ReportUtils.getMoneyRequestReimbursableTotal(report, allReports); - const isPinned = report.isPinned ?? false; if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report)) { pinnedAndGBRReports.push(report); diff --git a/src/libs/UnreadIndicatorUpdater/index.js b/src/libs/UnreadIndicatorUpdater/index.js index bfa0cd911177..9af74f8313c3 100644 --- a/src/libs/UnreadIndicatorUpdater/index.js +++ b/src/libs/UnreadIndicatorUpdater/index.js @@ -1,4 +1,3 @@ -import {InteractionManager} from 'react-native'; import Onyx from 'react-native-onyx'; import _ from 'underscore'; import * as ReportUtils from '@libs/ReportUtils'; @@ -6,33 +5,11 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import updateUnread from './updateUnread/index'; -let previousUnreadCount = 0; - Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT, waitForCollectionCallback: true, callback: (reportsFromOnyx) => { - if (!reportsFromOnyx) { - return; - } - - /** - * We need to wait until after interactions have finished to update the unread count because otherwise - * the unread count will be updated while the interactions/animations are in progress and we don't want - * to put more work on the main thread. - * - * For eg. On web we are manipulating DOM and it makes it a better candidate to wait until after interactions - * have finished. - * - * More info: https://reactnative.dev/docs/interactionmanager - */ - InteractionManager.runAfterInteractions(() => { - const unreadReports = _.filter(reportsFromOnyx, (report) => ReportUtils.isUnread(report) && report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN); - const unreadReportsCount = _.size(unreadReports); - if (previousUnreadCount !== unreadReportsCount) { - previousUnreadCount = unreadReportsCount; - updateUnread(unreadReportsCount); - } - }); + const unreadReports = _.filter(reportsFromOnyx, (report) => ReportUtils.isUnread(report) && report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN); + updateUnread(_.size(unreadReports)); }, }); diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 4646e0e33da1..1de15c1184cb 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -65,7 +65,6 @@ Onyx.connect({ const currentReportData = {}; Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT, - waitForCollectionCallback: true, callback: (data, key) => { if (!key || !data) { return; diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index aebd70a4175f..6b375fb5ffa5 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -278,14 +278,8 @@ function ComposerWithSuggestions({ } } const newCommentConverted = convertToLTRForComposer(newComment); - const isNewCommentEmpty = !!newCommentConverted.match(/^(\s)*$/); - const isPrevCommentEmpty = !!commentRef.current.match(/^(\s)*$/); - - /** Only update isCommentEmpty state if it's different from previous one */ - if (isNewCommentEmpty !== isPrevCommentEmpty) { - setIsCommentEmpty(isNewCommentEmpty); - } emojisPresentBefore.current = emojis; + setIsCommentEmpty(!!newCommentConverted.match(/^(\s)*$/)); setValue(newCommentConverted); if (commentValue !== newComment) { const remainder = ComposerUtils.getCommonSuffixLength(commentValue, newComment); @@ -562,7 +556,9 @@ function ComposerWithSuggestions({ if (value.length === 0) { return; } + Report.setReportWithDraft(reportID, true); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 1e5e11fd9fcb..293dc3f5cd9d 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -198,28 +198,23 @@ export default compose( chatReports: { key: ONYXKEYS.COLLECTION.REPORT, selector: chatReportSelector, - initialValue: {}, }, isLoadingReportData: { key: ONYXKEYS.IS_LOADING_REPORT_DATA, }, priorityMode: { key: ONYXKEYS.NVP_PRIORITY_MODE, - initialValue: CONST.PRIORITY_MODE.DEFAULT, }, betas: { key: ONYXKEYS.BETAS, - initialValue: [], }, allReportActions: { key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, selector: reportActionsSelector, - initialValue: {}, }, policies: { key: ONYXKEYS.COLLECTION.POLICY, selector: policySelector, - initialValue: {}, }, }), )(SidebarLinksData);