Skip to content

Commit

Permalink
Merge pull request #30852 from Expensify/revert-28469-perf/refactor-h…
Browse files Browse the repository at this point in the history
…eavy-operations-for-draft

[NoQA] Revert "perf: refactor heavy operations when user starts to type"
  • Loading branch information
mountiny authored Nov 3, 2023
2 parents 17ce0c2 + 82d0f7c commit 2133bef
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 82 deletions.
7 changes: 1 addition & 6 deletions src/components/LHNOptionsList/OptionRowLHNData.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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({
Expand Down
1 change: 0 additions & 1 deletion src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ Onyx.connect({
const policyExpenseReports = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (report, key) => {
if (!ReportUtils.isPolicyExpenseChat(report)) {
return;
Expand Down
7 changes: 2 additions & 5 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
}

Expand Down
26 changes: 4 additions & 22 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`, {}) || {};
}

/**
Expand Down Expand Up @@ -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.
Expand Down
22 changes: 12 additions & 10 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
27 changes: 2 additions & 25 deletions src/libs/UnreadIndicatorUpdater/index.js
Original file line number Diff line number Diff line change
@@ -1,38 +1,15 @@
import {InteractionManager} from 'react-native';
import Onyx from 'react-native-onyx';
import _ from 'underscore';
import * as ReportUtils from '@libs/ReportUtils';
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));
},
});
1 change: 0 additions & 1 deletion src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ Onyx.connect({
const currentReportData = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (data, key) => {
if (!key || !data) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -562,7 +556,9 @@ function ComposerWithSuggestions({
if (value.length === 0) {
return;
}

Report.setReportWithDraft(reportID, true);

// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

Expand Down
5 changes: 0 additions & 5 deletions src/pages/home/sidebar/SidebarLinksData.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit 2133bef

Please sign in to comment.