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

perf: make switch between chat list and workspaces smoother #36420

Merged
merged 68 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 67 commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
3088672
perf: use context for shared reports access
hurali97 Feb 13, 2024
f33930e
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Feb 15, 2024
6921051
perf: improve renderItem and use FlatList
hurali97 Feb 15, 2024
4d267b4
Merge branch 'main' into hur/perf/issue-35704
jbroma Feb 26, 2024
dc88950
refactor: revert changes to withCurrentReportID
jbroma Feb 26, 2024
9ae8701
refactor: remove useReports context & hooks
jbroma Feb 26, 2024
b613e97
refactor: use reports from onyx in useOrderedReportIDs
jbroma Feb 28, 2024
7f15fbc
refactor: move creating orderedReport objects from getOrderedReportId…
jbroma Feb 28, 2024
7fa44fc
Revert "refactor: revert changes to withCurrentReportID"
jbroma Feb 28, 2024
ae12b00
refactor: remove comment in SidebarLinksData
jbroma Feb 28, 2024
8087e59
Merge branch 'main' into hur/perf/issue-35704
jbroma Feb 28, 2024
c87f579
refactor: rename useOrderedReportIDs to useOrderedReportListItems
jbroma Feb 28, 2024
df1069c
perf: use memo for extraData in LHNOptionList
jbroma Feb 28, 2024
1d294bc
fix: typescript fixes
jbroma Feb 28, 2024
7dfba43
fix: update reportActionsSelector to match the one from SidebarLinksData
jbroma Feb 28, 2024
696c5ee
fix: typescript issues
hurali97 Feb 29, 2024
6739b58
test: fix reassure failing test
hurali97 Feb 29, 2024
92fae0c
revert: move option item data calculation to the renderItem component
hurali97 Feb 29, 2024
596fd63
fix: linting
hurali97 Feb 29, 2024
e735658
test: fix failing test
hurali97 Mar 1, 2024
94fcc48
fix: prettier
hurali97 Mar 1, 2024
31f5ed4
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 5, 2024
e0673e0
refactor: rename to useReportIDs
hurali97 Mar 5, 2024
30ca303
refactor: don't set currentReportID if it's on workspaces screen
hurali97 Mar 5, 2024
121b537
refactor: remove dead code
hurali97 Mar 5, 2024
9e41603
fix: linting
hurali97 Mar 5, 2024
4a32841
refactor: remove irrelevant changes
hurali97 Mar 5, 2024
581d09d
fix: comments
hurali97 Mar 11, 2024
fffe41a
fix: safely check the nested properties
hurali97 Mar 11, 2024
6b41bf7
feat: add comments for extraData prop
hurali97 Mar 11, 2024
5f832f9
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 11, 2024
49e32fd
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 13, 2024
6412711
fix: reassure tests
hurali97 Mar 13, 2024
e3477b2
fix: apply prettier
hurali97 Mar 13, 2024
769f83a
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 14, 2024
30b319f
perf: check for the settings tab existence in screen params and early…
hurali97 Mar 14, 2024
71978c2
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 15, 2024
8dba89b
feat: add selector from SidebarLinksData
hurali97 Mar 15, 2024
c39f358
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 15, 2024
f333232
fix: reassure tests
hurali97 Mar 15, 2024
3557027
fix: resolve merge conflicts
hurali97 Mar 15, 2024
335f1fa
Merge branch 'main' into hur/perf/issue-35704
jbroma Mar 20, 2024
61ef588
refactor: use currentReportID from useReportIDs
hurali97 Mar 21, 2024
84aa5ac
refactor: remove unnecessary ref
hurali97 Mar 21, 2024
2d8f88a
Merge branch 'hur/perf/issue-35704' of https://github.com/callstack-i…
hurali97 Mar 21, 2024
8532c08
update comment is updateCurrentReportID function
hurali97 Mar 21, 2024
ac465f3
update comment in useReportIDs hook
hurali97 Mar 21, 2024
886622e
refactor: make comment less verbose
hurali97 Mar 21, 2024
6d17116
refactor: remove irrelevant comment
hurali97 Mar 21, 2024
91ed2e6
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 21, 2024
4e1fbd2
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 22, 2024
2ef5c7f
refactor: reduce memo usage
hurali97 Mar 22, 2024
3bcc037
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Mar 27, 2024
ab5bce3
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Apr 16, 2024
9e4f464
fix: typecheck
hurali97 Apr 16, 2024
20d798e
refactor: remove unnecessary diff
hurali97 Apr 16, 2024
6f20396
refactor: leverage useOnyx hook instead of withOnyx HOC
hurali97 Apr 16, 2024
33fe888
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Apr 18, 2024
c791be0
fix: max depth exceeded error by not mutating allReports object
hurali97 Apr 18, 2024
4aa6b99
fix: use inline options and relevant initialValue
hurali97 Apr 18, 2024
459e86c
fix: remove not needed prop
hurali97 Apr 18, 2024
61fc9ce
fix: use correct prop in memo check
hurali97 Apr 18, 2024
489c652
fix: use policies instead of policyMembers
hurali97 Apr 18, 2024
10c0c04
Apply workaround for initialValue
fabioh8010 Apr 18, 2024
e075aee
fix: failing SidebarOrderTest
hurali97 Apr 19, 2024
0d4637e
fix: typecheck
hurali97 Apr 19, 2024
42068cc
Merge branch 'main' of https://github.com/callstack-internal/Expensif…
hurali97 Apr 25, 2024
821e4e0
refactor: remove initialValues
hurali97 Apr 25, 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
2 changes: 2 additions & 0 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {KeyboardStateProvider} from './components/withKeyboardState';
import {WindowDimensionsProvider} from './components/withWindowDimensions';
import Expensify from './Expensify';
import useDefaultDragAndDrop from './hooks/useDefaultDragAndDrop';
import {ReportIDsContextProvider} from './hooks/useReportIDs';
import OnyxUpdateManager from './libs/actions/OnyxUpdateManager';
import {ReportAttachmentsProvider} from './pages/home/report/ReportAttachmentsContext';
import type {Route} from './ROUTES';
Expand Down Expand Up @@ -78,6 +79,7 @@ function App({url}: AppProps) {
CustomStatusBarAndBackgroundContextProvider,
ActiveElementRoleProvider,
ActiveWorkspaceContextProvider,
ReportIDsContextProvider,
PlaybackContextProvider,
FullScreenContextProvider,
VolumeContextProvider,
Expand Down
12 changes: 11 additions & 1 deletion src/components/withCurrentReportID.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,17 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro
*/
const updateCurrentReportID = useCallback(
(state: NavigationState) => {
setCurrentReportID(Navigation.getTopmostReportId(state) ?? '');
const reportID = Navigation.getTopmostReportId(state) ?? '';

/*
* Make sure we don't make the reportID undefined when switching between the chat list and settings tab.
* This helps prevent unnecessary re-renders.
*/
const params = state?.routes?.[state.index]?.params;
if (params && 'screen' in params && typeof params.screen === 'string' && params.screen.indexOf('Settings_') !== -1) {
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
return;
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
}
setCurrentReportID(reportID);
},
[setCurrentReportID],
);
Expand Down
174 changes: 174 additions & 0 deletions src/hooks/useReportIDs.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
import React, {createContext, useCallback, useContext, useMemo} from 'react';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import {getPolicyEmployeeListByIdWithoutCurrentUser} from '@libs/PolicyUtils';
import * as ReportUtils from '@libs/ReportUtils';
import SidebarUtils from '@libs/SidebarUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type * as OnyxTypes from '@src/types/onyx';
import type {Message} from '@src/types/onyx/ReportAction';
import useActiveWorkspace from './useActiveWorkspace';
import useCurrentReportID from './useCurrentReportID';
import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails';

type ChatReportSelector = OnyxTypes.Report & {isUnreadWithMention: boolean};
type PolicySelector = Pick<OnyxTypes.Policy, 'type' | 'name' | 'avatar' | 'employeeList'>;
type ReportActionsSelector = Array<Pick<OnyxTypes.ReportAction, 'reportActionID' | 'actionName' | 'errors' | 'message' | 'originalMessage'>>;

type ReportIDsContextProviderProps = {
children: React.ReactNode;
currentReportIDForTests?: string;
};

type ReportIDsContextValue = {
orderedReportIDs: string[];
currentReportID: string;
};

const ReportIDsContext = createContext<ReportIDsContextValue>({
orderedReportIDs: [],
currentReportID: '',
});

/**
* This function (and the few below it), narrow down the data from Onyx to just the properties that we want to trigger a re-render of the component. This helps minimize re-rendering
* and makes the entire component more performant because it's not re-rendering when a bunch of properties change which aren't ever used in the UI.
*/
const chatReportSelector = (report: OnyxEntry<OnyxTypes.Report>): ChatReportSelector =>
(report && {
reportID: report.reportID,
participantAccountIDs: report.participantAccountIDs,
isPinned: report.isPinned,
isHidden: report.isHidden,
notificationPreference: report.notificationPreference,
errorFields: {
addWorkspaceRoom: report.errorFields?.addWorkspaceRoom,
},
lastMessageText: report.lastMessageText,
lastVisibleActionCreated: report.lastVisibleActionCreated,
iouReportID: report.iouReportID,
total: report.total,
nonReimbursableTotal: report.nonReimbursableTotal,
hasOutstandingChildRequest: report.hasOutstandingChildRequest,
isWaitingOnBankAccount: report.isWaitingOnBankAccount,
statusNum: report.statusNum,
stateNum: report.stateNum,
chatType: report.chatType,
type: report.type,
policyID: report.policyID,
visibility: report.visibility,
lastReadTime: report.lastReadTime,
// Needed for name sorting:
reportName: report.reportName,
policyName: report.policyName,
oldPolicyName: report.oldPolicyName,
// Other less obvious properites considered for sorting:
ownerAccountID: report.ownerAccountID,
currency: report.currency,
managerID: report.managerID,
// Other important less obivous properties for filtering:
parentReportActionID: report.parentReportActionID,
parentReportID: report.parentReportID,
isDeletedParentAction: report.isDeletedParentAction,
isUnreadWithMention: ReportUtils.isUnreadWithMention(report),
}) as ChatReportSelector;

const reportActionsSelector = (reportActions: OnyxEntry<OnyxTypes.ReportActions>): ReportActionsSelector =>
(reportActions &&
Object.values(reportActions).map((reportAction) => {
const {reportActionID, actionName, errors = [], originalMessage} = reportAction;
const decision = reportAction.message?.[0]?.moderationDecision?.decision;

return {
reportActionID,
actionName,
errors,
message: [
{
moderationDecision: {decision},
},
] as Message[],
originalMessage,
};
})) as ReportActionsSelector;

const policySelector = (policy: OnyxEntry<OnyxTypes.Policy>): PolicySelector =>
(policy && {
type: policy.type,
name: policy.name,
avatar: policy.avatar,
employeeList: policy.employeeList,
}) as PolicySelector;

function ReportIDsContextProvider({
children,
/**
* Only required to make unit tests work, since we
* explicitly pass the currentReportID in LHNTestUtils
* to SidebarLinksData, so this context doesn't have
* access to currentReportID in that case.
*
* This is a workaround to have currentReportID available in testing environment.
*/
currentReportIDForTests,
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
}: ReportIDsContextProviderProps) {
const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, {initialValue: CONST.PRIORITY_MODE.DEFAULT});
const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {selector: chatReportSelector, initialValue: {} as ChatReportSelector});
const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: policySelector, initialValue: {} as PolicySelector});
const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {selector: reportActionsSelector, initialValue: {} as ReportActionsSelector});
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {initialValue: {} as OnyxCollection<OnyxTypes.TransactionViolations>});
const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {initialValue: {} as OnyxCollection<string>});
hurali97 marked this conversation as resolved.
Show resolved Hide resolved
const [betas] = useOnyx(ONYXKEYS.BETAS, {initialValue: [] as OnyxEntry<OnyxTypes.Beta[]>});

const {accountID} = useCurrentUserPersonalDetails();
const currentReportIDValue = useCurrentReportID();
hurali97 marked this conversation as resolved.
Show resolved Hide resolved
const derivedCurrentReportID = currentReportIDForTests ?? currentReportIDValue?.currentReportID;
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
const {activeWorkspaceID} = useActiveWorkspace();

const policyMemberAccountIDs = getPolicyEmployeeListByIdWithoutCurrentUser(policies, activeWorkspaceID, accountID);

const getOrderedReportIDs = useCallback(
hurali97 marked this conversation as resolved.
Show resolved Hide resolved
(currentReportID?: string) =>
SidebarUtils.getOrderedReportIDs(
currentReportID ?? null,
chatReports,
betas,
policies,
priorityMode,
allReportActions,
transactionViolations,
activeWorkspaceID,
policyMemberAccountIDs,
),
// we need reports draft in deps array for reloading of list when reportsDrafts will change
// eslint-disable-next-line react-hooks/exhaustive-deps
[chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, reportsDrafts],
);

const orderedReportIDs = useMemo(() => getOrderedReportIDs(), [getOrderedReportIDs]);
const contextValue: ReportIDsContextValue = useMemo(() => {
// We need to make sure the current report is in the list of reports, but we do not want
// to have to re-generate the list every time the currentReportID changes. To do that
// we first generate the list as if there was no current report, then we check if
// the current report is missing from the list, which should very rarely happen. In this
// case we re-generate the list a 2nd time with the current report included.
if (derivedCurrentReportID && !orderedReportIDs.includes(derivedCurrentReportID)) {
return {orderedReportIDs: getOrderedReportIDs(derivedCurrentReportID), currentReportID: derivedCurrentReportID ?? ''};
}

return {
orderedReportIDs,
currentReportID: derivedCurrentReportID ?? '',
};
}, [getOrderedReportIDs, orderedReportIDs, derivedCurrentReportID]);

return <ReportIDsContext.Provider value={contextValue}>{children}</ReportIDsContext.Provider>;
}

function useReportIDs() {
return useContext(ReportIDsContext);
}

export {ReportIDsContext, ReportIDsContextProvider, policySelector, useReportIDs};
export type {ChatReportSelector, PolicySelector, ReportActionsSelector};
30 changes: 15 additions & 15 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import Str from 'expensify-common/lib/str';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {ChatReportSelector, PolicySelector, ReportActionsSelector} from '@hooks/useReportIDs';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetails, PersonalDetailsList, TransactionViolation} from '@src/types/onyx';
import type {PersonalDetails, PersonalDetailsList, ReportActions, TransactionViolation} from '@src/types/onyx';
import type Beta from '@src/types/onyx/Beta';
import type Policy from '@src/types/onyx/Policy';
import type PriorityMode from '@src/types/onyx/PriorityMode';
import type Report from '@src/types/onyx/Report';
import type {ReportActions} from '@src/types/onyx/ReportAction';
import type ReportAction from '@src/types/onyx/ReportAction';
import type DeepValueOf from '@src/types/utils/DeepValueOf';
import * as CollectionUtils from './CollectionUtils';
Expand Down Expand Up @@ -61,11 +61,11 @@ function compareStringDates(a: string, b: string): 0 | 1 | -1 {
*/
function getOrderedReportIDs(
currentReportId: string | null,
allReports: OnyxCollection<Report>,
allReports: OnyxCollection<ChatReportSelector>,
betas: OnyxEntry<Beta[]>,
policies: OnyxCollection<Policy>,
priorityMode: OnyxEntry<ValueOf<typeof CONST.PRIORITY_MODE>>,
allReportActions: OnyxCollection<ReportAction[]>,
policies: OnyxCollection<PolicySelector>,
priorityMode: OnyxEntry<PriorityMode>,
allReportActions: OnyxCollection<ReportActionsSelector>,
transactionViolations: OnyxCollection<TransactionViolation[]>,
currentPolicyID = '',
policyMemberAccountIDs: number[] = [],
Expand All @@ -87,7 +87,7 @@ function getOrderedReportIDs(
const doesReportHaveViolations = !!(
betas?.includes(CONST.BETAS.VIOLATIONS) &&
!!parentReportAction &&
ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction)
ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction as OnyxEntry<ReportAction>)
);
const isHidden = report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
const isFocused = report.reportID === currentReportId;
Expand All @@ -103,7 +103,7 @@ function getOrderedReportIDs(
currentReportId: currentReportId ?? '',
isInGSDMode,
betas,
policies,
policies: policies as OnyxCollection<Policy>,
excludeEmptyChats: true,
doesReportHaveViolations,
includeSelfDM: true,
Expand All @@ -130,13 +130,13 @@ 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.
reportsToDisplay.forEach((reportToDisplay) => {
let report = reportToDisplay as OnyxEntry<Report>;
if (report) {
// eslint-disable-next-line no-param-reassign
report.displayName = ReportUtils.getReportName(report);
report = {
...report,
displayName: ReportUtils.getReportName(report),
};
}

const isPinned = report?.isPinned ?? false;
Expand Down
Loading
Loading