Skip to content
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.

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

[NoQA] Revert "perf: refactor heavy operations when user starts to type" #30852

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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);
Loading