Skip to content

Commit

Permalink
Merge pull request #10239 from Expensify/marcaaron-memoizeGetOptions
Browse files Browse the repository at this point in the history 
Memoize some functions in `OptionsListUtils`
  • Loading branch information
@luacmartins
luacmartins authored Aug 8, 2022
2 parents 6e3bba4 + 1ef1980 commit c15c2e7
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 76 deletions.
17 changes: 11 additions & 6 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"htmlparser2": "^7.2.0",
"localforage": "^1.10.0",
"lodash": "4.17.21",
"memoize-one": "^6.0.0",
"metro-config": "^0.71.3",
"moment": "^2.29.4",
"moment-timezone": "^0.5.31",
Expand All @@ -93,7 +94,7 @@
"react-native-image-picker": "^4.7.3",
"react-native-image-size": "git+https://github.com/Expensify/react-native-image-size#6b5ab5110dc3ed554f8eafbc38d7d87c17147972",
"react-native-modal": "^13.0.0",
"react-native-onyx": "1.0.10",
"react-native-onyx": "1.0.12",
"react-native-pdf": "^6.6.2",
"react-native-performance": "^2.0.0",
"react-native-permissions": "^3.0.1",
Expand Down
96 changes: 55 additions & 41 deletions src/libs/OptionsListUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import _ from 'underscore';
import Onyx from 'react-native-onyx';
import lodashGet from 'lodash/get';
import lodashOrderBy from 'lodash/orderBy';
import memoizeOne from 'memoize-one';
import Str from 'expensify-common/lib/str';
import ONYXKEYS from '../ONYXKEYS';
import CONST from '../CONST';
Expand All @@ -11,6 +12,8 @@ import * as Localize from './Localize';
import Permissions from './Permissions';
import * as CollectionUtils from './CollectionUtils';

const memoizedOrderBy = memoizeOne(lodashOrderBy);

/**
* OptionsListUtils is used to build a list options passed to the OptionsList component. Several different UI views can
* be configured to display different results based on the options passed to the private getOptions() method. Public
Expand Down Expand Up @@ -41,18 +44,6 @@ Onyx.connect({
callback: val => preferredLocale = val || CONST.DEFAULT_LOCALE,
});

const reportsWithDraft = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT,
callback: (hasDraft, key) => {
if (!key) {
return;
}

reportsWithDraft[key] = hasDraft;
},
});

const policies = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY,
Expand Down Expand Up @@ -126,6 +117,8 @@ function getPersonalDetailsForLogins(logins, personalDetails) {
return personalDetailsForLogins;
}

const memoizedGetPersonalDetailsForLogins = memoizeOne(getPersonalDetailsForLogins);

/**
* Constructs a Set with all possible names (displayName, firstName, lastName, email) for all participants in a report,
* to be used in isSearchStringMatch.
Expand Down Expand Up @@ -189,15 +182,17 @@ function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolic
return _.unique(searchTerms).join(' ');
}

const memoizedGetSearchText = memoizeOne(getSearchText);

/**
* Determines whether a report has a draft comment.
*
* @param {Object} report
* @param {Object} reportsWithDraft
* @return {Boolean}
*/
function hasReportDraftComment(report) {
function hasReportDraftComment(report, reportsWithDraft = {}) {
return report
&& reportsWithDraft
&& lodashGet(reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${report.reportID}`, false);
}

Expand All @@ -207,22 +202,24 @@ function hasReportDraftComment(report) {
* @param {Array<String>} logins
* @param {Object} personalDetails
* @param {Object} report
* @param {Object} reportsWithDraft
* @param {Object} options
* @param {Boolean} [options.showChatPreviewLine]
* @param {Boolean} [options.forcePolicyNamePreview]
* @returns {Object}
*/
function createOption(logins, personalDetails, report, {
showChatPreviewLine = false, forcePolicyNamePreview = false,
function createOption(logins, personalDetails, report, reportsWithDraft, {
showChatPreviewLine = false,
forcePolicyNamePreview = false,
}) {
const isChatRoom = ReportUtils.isChatRoom(report);
const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report);
const personalDetailMap = getPersonalDetailsForLogins(logins, personalDetails);
const personalDetailMap = memoizedGetPersonalDetailsForLogins(logins, personalDetails);
const personalDetailList = _.values(personalDetailMap);
const isArchivedRoom = ReportUtils.isArchivedRoom(report);
const hasMultipleParticipants = personalDetailList.length > 1 || isChatRoom || isPolicyExpenseChat;
const personalDetail = personalDetailList[0];
const hasDraftComment = hasReportDraftComment(report);
const hasDraftComment = hasReportDraftComment(report, reportsWithDraft);
const hasOutstandingIOU = lodashGet(report, 'hasOutstandingIOU', false);
const iouReport = hasOutstandingIOU
? lodashGet(iouReports, `${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`, {})
Expand Down Expand Up @@ -276,7 +273,7 @@ function createOption(logins, personalDetails, report, {
isUnread: report ? report.unreadActionCount > 0 : null,
hasDraftComment,
keyForList: report ? String(report.reportID) : personalDetail.login,
searchText: getSearchText(report, reportName, personalDetailList, isChatRoom || isPolicyExpenseChat),
searchText: memoizedGetSearchText(report, reportName, personalDetailList, isChatRoom || isPolicyExpenseChat),
isPinned: lodashGet(report, 'isPinned', false),
hasOutstandingIOU,
iouReportID: lodashGet(report, 'iouReportID'),
Expand All @@ -289,6 +286,8 @@ function createOption(logins, personalDetails, report, {
};
}

const memoizedCreateOption = memoizeOne(createOption);

/**
* Searches for a match when provided with a value
*
Expand Down Expand Up @@ -342,6 +341,10 @@ function isCurrentUser(userDetails) {
return result;
}

// We are storing a map of logins in the format {[login]: [login]} so that the memoized functions looking for an array with a login in it
// treat this like the same argument (because it will use the same reference). Memoization for personalDetails won't work properly without this.
const loginArrayMap = {};

/**
* Build the options
*
Expand All @@ -353,6 +356,7 @@ function isCurrentUser(userDetails) {
* @private
*/
function getOptions(reports, personalDetails, activeReportID, {
reportsWithDraft = {},
betas = [],
selectedOptions = [],
maxRecentReportsToShow = 0,
Expand Down Expand Up @@ -390,8 +394,9 @@ function getOptions(reports, personalDetails, activeReportID, {
if (sortByAlphaAsc) {
sortProperty = ['reportName'];
}

const sortDirection = [sortByAlphaAsc ? 'asc' : 'desc'];
let orderedReports = lodashOrderBy(reports, sortProperty, sortDirection);
let orderedReports = memoizedOrderBy(reports, sortProperty, sortDirection);

// Move the archived Rooms to the last
orderedReports = _.sortBy(orderedReports, report => ReportUtils.isArchivedRoom(report));
Expand All @@ -409,7 +414,7 @@ function getOptions(reports, personalDetails, activeReportID, {
return;
}

const hasDraftComment = hasReportDraftComment(report);
const hasDraftComment = hasReportDraftComment(report, reportsWithDraft);
const iouReportOwner = lodashGet(report, 'hasOutstandingIOU', false)
? lodashGet(iouReports, [`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`, 'ownerEmail'], '')
: '';
Expand Down Expand Up @@ -456,22 +461,33 @@ function getOptions(reports, personalDetails, activeReportID, {
reportMapForLogins[logins[0]] = report;
}
const isSearchingSomeonesPolicyExpenseChat = !report.isOwnPolicyExpenseChat && searchValue !== '';
allReportOptions.push(createOption(logins, personalDetails, report, {
allReportOptions.push(memoizedCreateOption(logins, personalDetails, report, reportsWithDraft, {
showChatPreviewLine,
forcePolicyNamePreview: isPolicyExpenseChat ? isSearchingSomeonesPolicyExpenseChat : forcePolicyNamePreview,
}));
});

let allPersonalDetailsOptions = _.map(personalDetails, personalDetail => (
createOption([personalDetail.login], personalDetails, reportMapForLogins[personalDetail.login], {
showChatPreviewLine,
forcePolicyNamePreview,
})
));
let allPersonalDetailsOptions = _.map(personalDetails, (personalDetail) => {
// We want to use the same argument reference when creating the personalDetails option as memoization won't work properly
// if we passed [personalDetail.login] directly (that would be a new argument since we'd initialize a new array)
if (!loginArrayMap[personalDetail.login]) {
loginArrayMap[personalDetail.login] = [personalDetail.login];
}
return memoizedCreateOption(
loginArrayMap[personalDetail.login],
personalDetails,
reportMapForLogins[personalDetail.login],
reportsWithDraft,
{
showChatPreviewLine,
forcePolicyNamePreview,
},
);
});

if (sortPersonalDetailsByAlphaAsc) {
// PersonalDetails should be ordered Alphabetically by default - https://github.com/Expensify/App/issues/8220#issuecomment-1104009435
allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [personalDetail => personalDetail.text.toLowerCase()], 'asc');
allPersonalDetailsOptions = memoizedOrderBy(allPersonalDetailsOptions, [personalDetail => personalDetail.text.toLowerCase()], 'asc');
}

// Always exclude already selected options and the currently logged in user
Expand Down Expand Up @@ -529,20 +545,20 @@ function getOptions(reports, personalDetails, activeReportID, {
// If we are prioritizing reports with draft comments, add them before the normal recent report options
// and sort them by report name.
if (prioritizeReportsWithDraftComments) {
const sortedDraftReports = lodashOrderBy(draftReportOptions, ['text'], ['asc']);
const sortedDraftReports = memoizedOrderBy(draftReportOptions, ['text'], ['asc']);
recentReportOptions = sortedDraftReports.concat(recentReportOptions);
}

// If we are prioritizing IOUs the user owes, add them before the normal recent report options and reports
// with draft comments.
if (prioritizeIOUDebts) {
const sortedIOUReports = lodashOrderBy(iouDebtReportOptions, ['iouReportAmount'], ['desc']);
const sortedIOUReports = memoizedOrderBy(iouDebtReportOptions, ['iouReportAmount'], ['desc']);
recentReportOptions = sortedIOUReports.concat(recentReportOptions);
}

// If we are prioritizing our pinned reports then shift them to the front and sort them by report name
if (prioritizePinnedReports) {
const sortedPinnedReports = lodashOrderBy(pinnedReportOptions, ['text'], ['asc']);
const sortedPinnedReports = memoizedOrderBy(pinnedReportOptions, ['text'], ['asc']);
recentReportOptions = sortedPinnedReports.concat(recentReportOptions);
}

Expand Down Expand Up @@ -584,7 +600,7 @@ function getOptions(reports, personalDetails, activeReportID, {
const login = (Str.isValidPhone(searchValue) && !searchValue.includes('+'))
? `+${countryCodeByIP}${searchValue}`
: searchValue;
userToInvite = createOption([login], personalDetails, null, {
userToInvite = memoizedCreateOption([login], personalDetails, null, reportsWithDraft, {
showChatPreviewLine,
});
userToInvite.icons = [ReportUtils.getDefaultAvatar(login)];
Expand All @@ -595,7 +611,7 @@ function getOptions(reports, personalDetails, activeReportID, {
// When sortByReportTypeInSearch is true, recentReports will be returned with all the reports including personalDetailsOptions in the correct Order.
recentReportOptions.push(...personalDetailsOptions);
personalDetailsOptions = [];
recentReportOptions = lodashOrderBy(recentReportOptions, [(option) => {
recentReportOptions = memoizedOrderBy(recentReportOptions, [(option) => {
if (option.isChatRoom || option.isArchivedRoom) {
return 3;
}
Expand Down Expand Up @@ -746,15 +762,10 @@ function getMemberInviteOptions(
* @param {Number} activeReportID
* @param {String} priorityMode
* @param {Array<String>} betas
* @param {Object} reportsWithDraft
* @returns {Object}
*/
function getSidebarOptions(
reports,
personalDetails,
activeReportID,
priorityMode,
betas,
) {
function calculateSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportsWithDraft) {
let sideBarOptions = {
prioritizeIOUDebts: true,
prioritizeReportsWithDraftComments: true,
Expand All @@ -774,9 +785,12 @@ function getSidebarOptions(
showChatPreviewLine: true,
prioritizePinnedReports: true,
...sideBarOptions,
reportsWithDraft,
});
}

const getSidebarOptions = memoizeOne(calculateSidebarOptions);

/**
* Helper method that returns the text to be used for the header's message and title (if any)
*
Expand Down
8 changes: 7 additions & 1 deletion src/libs/actions/App.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ Onyx.connect({
let myPersonalDetails;
Onyx.connect({
key: ONYXKEYS.PERSONAL_DETAILS,
callback: val => myPersonalDetails = val[currentUserEmail],
callback: (val) => {
if (!val || !currentUserEmail) {
return;
}

myPersonalDetails = val[currentUserEmail];
},
});

const allPolicies = {};
Expand Down
3 changes: 2 additions & 1 deletion src/pages/home/ReportScreen.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const propTypes = {

/** The type of the policy */
type: PropTypes.string,
})).isRequired,
})),

/** Information about the network */
network: networkPropTypes.isRequired,
Expand All @@ -99,6 +99,7 @@ const defaultProps = {
isComposerFullSize: false,
betas: [],
isLoadingInitialReportActions: false,
policies: {},
};

/**
Expand Down
Loading

0 comments on commit c15c2e7

Please sign in to comment.