From 3088672dda44f8eb7173c0545ef1c1adfebd4498 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 13 Feb 2024 18:44:45 +0500 Subject: [PATCH 01/51] perf: use context for shared reports access --- src/App.tsx | 4 + .../LHNOptionsList/LHNOptionsList.tsx | 6 +- src/components/LHNOptionsList/types.ts | 3 - src/hooks/useOrderedReportIDs.tsx | 145 +++++++++++ src/hooks/useReports.tsx | 44 ++++ .../AppNavigator/ReportScreenIDSetter.ts | 13 +- src/libs/SidebarUtils.ts | 57 ++--- src/pages/home/sidebar/SidebarLinksData.js | 228 +----------------- src/pages/workspace/WorkspacesListPage.tsx | 12 +- src/types/onyx/PriorityMode.ts | 6 + 10 files changed, 228 insertions(+), 290 deletions(-) create mode 100644 src/hooks/useOrderedReportIDs.tsx create mode 100644 src/hooks/useReports.tsx create mode 100644 src/types/onyx/PriorityMode.ts diff --git a/src/App.tsx b/src/App.tsx index 7c1ead1d86d3..712b82c21291 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -26,6 +26,8 @@ import {KeyboardStateProvider} from './components/withKeyboardState'; import {WindowDimensionsProvider} from './components/withWindowDimensions'; import Expensify from './Expensify'; import useDefaultDragAndDrop from './hooks/useDefaultDragAndDrop'; +import {OrderedReportIDsContextProvider} from './hooks/useOrderedReportIDs'; +import {ReportsContextProvider} from './hooks/useReports'; import OnyxUpdateManager from './libs/actions/OnyxUpdateManager'; import * as Session from './libs/actions/Session'; import * as Environment from './libs/Environment/Environment'; @@ -79,6 +81,8 @@ function App({url}: AppProps) { EnvironmentProvider, CustomStatusBarAndBackgroundContextProvider, ActiveWorkspaceContextProvider, + ReportsContextProvider, + OrderedReportIDsContextProvider, ]} > diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index a55d329f39ee..5ab105eda8df 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -5,6 +5,7 @@ import {StyleSheet, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import withCurrentReportID from '@components/withCurrentReportID'; import usePermissions from '@hooks/usePermissions'; +import {useReports} from '@hooks/useReports'; import useThemeStyles from '@hooks/useThemeStyles'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import variables from '@styles/variables'; @@ -22,7 +23,6 @@ function LHNOptionsList({ onSelectRow, optionMode, shouldDisableFocusOptions = false, - reports = {}, reportActions = {}, policy = {}, preferredLocale = CONST.LOCALES.DEFAULT, @@ -33,6 +33,7 @@ function LHNOptionsList({ transactionViolations = {}, onFirstItemRendered = () => {}, }: LHNOptionsListProps) { + const reports = useReports(); const styles = useThemeStyles(); const {canUseViolations} = usePermissions(); @@ -134,9 +135,6 @@ LHNOptionsList.displayName = 'LHNOptionsList'; export default withCurrentReportID( withOnyx({ - reports: { - key: ONYXKEYS.COLLECTION.REPORT, - }, reportActions: { key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, }, diff --git a/src/components/LHNOptionsList/types.ts b/src/components/LHNOptionsList/types.ts index 58bea97f04c9..c58770c8383f 100644 --- a/src/components/LHNOptionsList/types.ts +++ b/src/components/LHNOptionsList/types.ts @@ -15,9 +15,6 @@ type LHNOptionsListOnyxProps = { /** The policy which the user has access to and which the report could be tied to */ policy: OnyxCollection; - /** All reports shared with the user */ - reports: OnyxCollection; - /** Array of report actions for this report */ reportActions: OnyxCollection; diff --git a/src/hooks/useOrderedReportIDs.tsx b/src/hooks/useOrderedReportIDs.tsx new file mode 100644 index 000000000000..3217830054de --- /dev/null +++ b/src/hooks/useOrderedReportIDs.tsx @@ -0,0 +1,145 @@ +import React, {createContext, useContext, useMemo} from 'react'; +import {withOnyx} from 'react-native-onyx'; +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; +import {getCurrentUserAccountID} from '@libs/actions/Report'; +import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; +import SidebarUtils from '@libs/SidebarUtils'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Beta, Policy, PolicyMembers, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; +import type PriorityMode from '@src/types/onyx/PriorityMode'; +import useActiveWorkspace from './useActiveWorkspace'; +import useCurrentReportID from './useCurrentReportID'; +import {useReports} from './useReports'; + +type OnyxProps = { + betas: OnyxEntry; + policies: OnyxCollection; + allReportActions: OnyxCollection; + transactionViolations: OnyxCollection; + policyMembers: OnyxCollection; + priorityMode: OnyxEntry; +}; + +type WithOrderedReportIDsContextProviderProps = OnyxProps & { + children: React.ReactNode; +}; + +const OrderedReportIDsContext = createContext({}); + +function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextProviderProps) { + const chatReports = useReports(); + const currentReportIDValue = useCurrentReportID(); + const {activeWorkspaceID} = useActiveWorkspace(); + + const policyMemberAccountIDs = useMemo( + () => getPolicyMembersByIdWithoutCurrentUser(props.policyMembers, activeWorkspaceID, getCurrentUserAccountID()), + [activeWorkspaceID, props.policyMembers], + ); + + const optionListItems = useMemo( + () => + SidebarUtils.getOrderedReportIDs( + null, + chatReports, + props.betas ?? [], + props.policies, + props.priorityMode, + props.allReportActions, + props.transactionViolations, + activeWorkspaceID, + policyMemberAccountIDs, + ), + [chatReports, props.betas, props.policies, props.priorityMode, props.allReportActions, props.transactionViolations, activeWorkspaceID, policyMemberAccountIDs], + ); + + // 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 here 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. + const optionListItemsWithCurrentReport = useMemo(() => { + if (currentReportIDValue?.currentReportID && !optionListItems.includes(currentReportIDValue.currentReportID)) { + return SidebarUtils.getOrderedReportIDs( + currentReportIDValue.currentReportID, + chatReports, + props.betas ?? [], + props.policies, + props.priorityMode, + props.allReportActions, + props.transactionViolations, + activeWorkspaceID, + policyMemberAccountIDs, + ); + } + return optionListItems; + }, [ + activeWorkspaceID, + chatReports, + currentReportIDValue?.currentReportID, + optionListItems, + policyMemberAccountIDs, + props.allReportActions, + props.betas, + props.policies, + props.priorityMode, + props.transactionViolations, + ]); + + return {props.children}; +} + +const reportActionsSelector = (reportActions: OnyxEntry) => { + if (!reportActions) { + return []; + } + + return Object.values(reportActions).map((reportAction) => { + const {reportActionID, actionName, originalMessage} = reportAction ?? {}; + const decision = reportAction?.message?.[0]?.moderationDecision?.decision; + return { + reportActionID, + actionName, + originalMessage, + message: [ + { + moderationDecision: {decision}, + }, + ], + }; + }); +}; + +const OrderedReportIDsContextProvider = withOnyx({ + priorityMode: { + key: ONYXKEYS.NVP_PRIORITY_MODE, + initialValue: CONST.PRIORITY_MODE.DEFAULT, + }, + betas: { + key: ONYXKEYS.BETAS, + initialValue: [], + }, + allReportActions: { + key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, + // @ts-expect-error Need some help in determining the correct type for this selector + selector: (actions) => reportActionsSelector(actions), + initialValue: {}, + }, + policies: { + key: ONYXKEYS.COLLECTION.POLICY, + initialValue: {}, + }, + policyMembers: { + key: ONYXKEYS.COLLECTION.POLICY_MEMBERS, + }, + transactionViolations: { + key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, + initialValue: {}, + }, +})(WithOrderedReportIDsContextProvider); + +function useOrderedReportIDs() { + return useContext(OrderedReportIDsContext); +} + +export {OrderedReportIDsContextProvider, OrderedReportIDsContext, useOrderedReportIDs}; diff --git a/src/hooks/useReports.tsx b/src/hooks/useReports.tsx new file mode 100644 index 000000000000..c4082149cbf4 --- /dev/null +++ b/src/hooks/useReports.tsx @@ -0,0 +1,44 @@ +import React, {createContext, useContext, useEffect, useMemo, useState} from 'react'; +import Onyx from 'react-native-onyx'; +import type {OnyxCollection} from 'react-native-onyx'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Report} from '@src/types/onyx'; + +type Reports = OnyxCollection; +type ReportsContextValue = Reports; + +type ReportsContextProviderProps = { + children: React.ReactNode; +}; + +const ReportsContext = createContext(null); + +function ReportsContextProvider(props: ReportsContextProviderProps) { + const [reports, setReports] = useState(null); + + useEffect(() => { + // eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs + const connID = Onyx.connect({ + key: ONYXKEYS.COLLECTION.REPORT, + waitForCollectionCallback: true, + callback: (val) => { + setReports(val); + }, + }); + return () => { + Onyx.disconnect(connID); + }; + }, []); + + const contextValue = useMemo(() => reports ?? {}, [reports]); + + return {props.children}; +} + +function useReports() { + return useContext(ReportsContext); +} + +ReportsContextProvider.displayName = 'ReportsContextProvider'; + +export {ReportsContextProvider, ReportsContext, useReports}; diff --git a/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts b/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts index b4bb56262860..83df18d5492d 100644 --- a/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts +++ b/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts @@ -3,6 +3,7 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {withOnyx} from 'react-native-onyx'; import useActiveWorkspace from '@hooks/useActiveWorkspace'; import usePermissions from '@hooks/usePermissions'; +import {useReports} from '@hooks/useReports'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; import * as App from '@userActions/App'; @@ -11,9 +12,6 @@ import type {Policy, PolicyMembers, Report, ReportMetadata} from '@src/types/ony import type {ReportScreenWrapperProps} from './ReportScreenWrapper'; type ReportScreenIDSetterComponentProps = { - /** Available reports that would be displayed in this navigator */ - reports: OnyxCollection; - /** The policies which the user has access to */ policies: OnyxCollection; @@ -59,9 +57,10 @@ const getLastAccessedReportID = ( }; // This wrapper is reponsible for opening the last accessed report if there is no reportID specified in the route params -function ReportScreenIDSetter({route, reports, policies, policyMembers = {}, navigation, isFirstTimeNewExpensifyUser = false, reportMetadata, accountID}: ReportScreenIDSetterProps) { +function ReportScreenIDSetter({route, policies, policyMembers = {}, navigation, isFirstTimeNewExpensifyUser = false, reportMetadata, accountID}: ReportScreenIDSetterProps) { const {canUseDefaultRooms} = usePermissions(); const {activeWorkspaceID} = useActiveWorkspace(); + const reports = useReports(); useEffect(() => { // Don't update if there is a reportID in the params already @@ -83,7 +82,7 @@ function ReportScreenIDSetter({route, reports, policies, policyMembers = {}, nav !canUseDefaultRooms, policies, isFirstTimeNewExpensifyUser, - !!reports?.params?.openOnAdminRoom, + !!route?.params?.openOnAdminRoom, reportMetadata, activeWorkspaceID, policyMemberAccountIDs, @@ -106,10 +105,6 @@ function ReportScreenIDSetter({route, reports, policies, policyMembers = {}, nav ReportScreenIDSetter.displayName = 'ReportScreenIDSetter'; export default withOnyx({ - reports: { - key: ONYXKEYS.COLLECTION.REPORT, - allowStaleData: true, - }, policies: { key: ONYXKEYS.COLLECTION.POLICY, allowStaleData: true, diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 2347f5b9f5c5..e51276c5c28a 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -2,12 +2,12 @@ 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 CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {PersonalDetails, PersonalDetailsList, 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'; @@ -78,59 +78,23 @@ function setIsSidebarLoadedReady() { resolveSidebarIsReadyPromise(); } -// Define a cache object to store the memoized results -const reportIDsCache = new Map(); - -// Function to set a key-value pair while maintaining the maximum key limit -function setWithLimit(map: Map, key: TKey, value: TValue) { - if (map.size >= 5) { - // If the map has reached its limit, remove the first (oldest) key-value pair - const firstKey = map.keys().next().value; - map.delete(firstKey); - } - map.set(key, value); -} - -// Variable to verify if ONYX actions are loaded -let hasInitialReportActions = false; - /** * @returns An array of reportIDs sorted in the proper order */ function getOrderedReportIDs( currentReportId: string | null, - allReports: Record, + allReports: OnyxCollection, betas: Beta[], - policies: Record, - priorityMode: ValueOf, + policies: OnyxCollection, + priorityMode: OnyxEntry, allReportActions: OnyxCollection, transactionViolations: OnyxCollection, currentPolicyID = '', policyMemberAccountIDs: number[] = [], ): string[] { - const currentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${currentReportId}`]; - let reportActionCount = currentReportActions?.length ?? 0; - reportActionCount = Math.max(reportActionCount, 1); - - // Generate a unique cache key based on the function arguments - const cachedReportsKey = JSON.stringify( - [currentReportId, allReports, betas, policies, priorityMode, reportActionCount, currentPolicyID, policyMemberAccountIDs], - // Exclude some properties not to overwhelm a cached key value with huge data, which we don't need to store in a cacheKey - (key, value: unknown) => (['participantAccountIDs', 'participants', 'lastMessageText', 'visibleChatMemberAccountIDs'].includes(key) ? undefined : value), - ); - - // Check if the result is already in the cache - const cachedIDs = reportIDsCache.get(cachedReportsKey); - if (cachedIDs && hasInitialReportActions) { - return cachedIDs; - } - - // This is needed to prevent caching when Onyx is empty for a second render - hasInitialReportActions = Object.values(lastReportActions).length > 0; - const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; const isInDefaultMode = !isInGSDMode; - const allReportsDictValues = Object.values(allReports); + const allReportsDictValues = Object.values(allReports ?? {}); // Filter out all the reports that shouldn't be displayed let reportsToDisplay = allReportsDictValues.filter((report) => { @@ -173,10 +137,18 @@ function getOrderedReportIDs( const archivedReports: Report[] = []; if (currentPolicyID || policyMemberAccountIDs.length > 0) { - reportsToDisplay = reportsToDisplay.filter((report) => ReportUtils.doesReportBelongToWorkspace(report, policyMemberAccountIDs, currentPolicyID)); + reportsToDisplay = reportsToDisplay.filter((report) => { + if (!report) { + return false; + } + return ReportUtils.doesReportBelongToWorkspace(report, policyMemberAccountIDs, currentPolicyID); + }); } // There are a few properties that need to be calculated for the report which are used when sorting reports. reportsToDisplay.forEach((report) => { + if (!report) { + return; + } // 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. @@ -219,7 +191,6 @@ function getOrderedReportIDs( // Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID. // The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar. const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); - setWithLimit(reportIDsCache, cachedReportsKey, LHNReports); return LHNReports; } diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 3bd538e8beab..42681fea451b 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -1,11 +1,8 @@ import {deepEqual} from 'fast-equals'; -import lodashGet from 'lodash/get'; -import lodashMap from 'lodash/map'; import PropTypes from 'prop-types'; import React, {useCallback, useEffect, useMemo, useRef} from 'react'; import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; -import _ from 'underscore'; import networkPropTypes from '@components/networkPropTypes'; import {withNetwork} from '@components/OnyxProvider'; import withCurrentReportID from '@components/withCurrentReportID'; @@ -13,13 +10,11 @@ import withCurrentUserPersonalDetails from '@components/withCurrentUserPersonalD import withNavigationFocus from '@components/withNavigationFocus'; import useActiveWorkspace from '@hooks/useActiveWorkspace'; import useLocalize from '@hooks/useLocalize'; +import {useOrderedReportIDs} from '@hooks/useOrderedReportIDs'; import usePrevious from '@hooks/usePrevious'; import useThemeStyles from '@hooks/useThemeStyles'; import compose from '@libs/compose'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; -import * as ReportUtils from '@libs/ReportUtils'; -import SidebarUtils from '@libs/SidebarUtils'; -import reportPropTypes from '@pages/reportPropTypes'; import * as Policy from '@userActions/Policy'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -28,107 +23,30 @@ import SidebarLinks, {basePropTypes} from './SidebarLinks'; const propTypes = { ...basePropTypes, - /* Onyx Props */ - /** List of reports */ - chatReports: PropTypes.objectOf(reportPropTypes), - - /** All report actions for all reports */ - - /** Object of report actions for this report */ - allReportActions: PropTypes.objectOf( - PropTypes.arrayOf( - PropTypes.shape({ - error: PropTypes.string, - message: PropTypes.arrayOf( - PropTypes.shape({ - moderationDecision: PropTypes.shape({ - decision: PropTypes.string, - }), - }), - ), - }), - ), - ), - /** Whether the reports are loading. When false it means they are ready to be used. */ isLoadingApp: PropTypes.bool, /** The chat priority mode */ priorityMode: PropTypes.string, - /** Beta features list */ - betas: PropTypes.arrayOf(PropTypes.string), - network: networkPropTypes.isRequired, - /** The policies which the user has access to */ - // eslint-disable-next-line react/forbid-prop-types - policies: PropTypes.object, - - // eslint-disable-next-line react/forbid-prop-types - policyMembers: PropTypes.object, - /** Session info for the currently logged in user. */ session: PropTypes.shape({ /** Currently logged in user accountID */ accountID: PropTypes.number, }), - /** All of the transaction violations */ - transactionViolations: PropTypes.shape({ - violations: PropTypes.arrayOf( - PropTypes.shape({ - /** The transaction ID */ - transactionID: PropTypes.number, - - /** The transaction violation type */ - type: PropTypes.string, - - /** The transaction violation message */ - message: PropTypes.string, - - /** The transaction violation data */ - data: PropTypes.shape({ - /** The transaction violation data field */ - field: PropTypes.string, - - /** The transaction violation data value */ - value: PropTypes.string, - }), - }), - ), - }), }; const defaultProps = { - chatReports: {}, - allReportActions: {}, isLoadingApp: true, priorityMode: CONST.PRIORITY_MODE.DEFAULT, - betas: [], - policies: {}, - policyMembers: {}, session: { accountID: '', }, - transactionViolations: {}, }; -function SidebarLinksData({ - isFocused, - allReportActions, - betas, - chatReports, - currentReportID, - insets, - isLoadingApp, - onLinkClick, - policies, - priorityMode, - network, - policyMembers, - session: {accountID}, - transactionViolations, -}) { +function SidebarLinksData({isFocused, currentReportID, insets, isLoadingApp, onLinkClick, priorityMode, network, policyMembers, session: {accountID}}) { const styles = useThemeStyles(); const {activeWorkspaceID} = useActiveWorkspace(); const {translate} = useLocalize(); @@ -141,19 +59,8 @@ function SidebarLinksData({ const reportIDsRef = useRef(null); const isLoading = isLoadingApp; + const reportIDs = useOrderedReportIDs(); const optionListItems = useMemo(() => { - const reportIDs = SidebarUtils.getOrderedReportIDs( - null, - chatReports, - betas, - policies, - priorityMode, - allReportActions, - transactionViolations, - activeWorkspaceID, - policyMemberAccountIDs, - ); - if (deepEqual(reportIDsRef.current, reportIDs)) { return reportIDsRef.current; } @@ -165,29 +72,7 @@ function SidebarLinksData({ reportIDsRef.current = reportIDs; } return reportIDsRef.current || []; - }, [chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, isLoading, network.isOffline, prevPriorityMode]); - - // 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 here 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. - const optionListItemsWithCurrentReport = useMemo(() => { - if (currentReportID && !_.contains(optionListItems, currentReportID)) { - return SidebarUtils.getOrderedReportIDs( - currentReportID, - chatReports, - betas, - policies, - priorityMode, - allReportActions, - transactionViolations, - activeWorkspaceID, - policyMemberAccountIDs, - ); - } - return optionListItems; - }, [currentReportID, optionListItems, chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs]); + }, [reportIDs, isLoading, network.isOffline, prevPriorityMode, priorityMode]); const currentReportIDRef = useRef(currentReportID); currentReportIDRef.current = currentReportID; @@ -207,8 +92,8 @@ function SidebarLinksData({ // Data props: isActiveReport={isActiveReport} isLoading={isLoading} - optionListItems={optionListItemsWithCurrentReport} activeWorkspaceID={activeWorkspaceID} + optionListItems={optionListItems} /> ); @@ -218,97 +103,12 @@ SidebarLinksData.propTypes = propTypes; SidebarLinksData.defaultProps = defaultProps; SidebarLinksData.displayName = 'SidebarLinksData'; -/** - * 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. - * @param {Object} [report] - * @returns {Object|undefined} - */ -const chatReportSelector = (report) => - report && { - reportID: report.reportID, - participantAccountIDs: report.participantAccountIDs, - hasDraft: report.hasDraft, - isPinned: report.isPinned, - isHidden: report.isHidden, - notificationPreference: report.notificationPreference, - errorFields: { - addWorkspaceRoom: report.errorFields && 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), - }; - -/** - * @param {Object} [reportActions] - * @returns {Object|undefined} - */ -const reportActionsSelector = (reportActions) => - reportActions && - lodashMap(reportActions, (reportAction) => { - const {reportActionID, parentReportActionID, actionName, errors = []} = reportAction; - const decision = lodashGet(reportAction, 'message[0].moderationDecision.decision'); - - return { - reportActionID, - parentReportActionID, - actionName, - errors, - message: [ - { - moderationDecision: {decision}, - }, - ], - }; - }); - -/** - * @param {Object} [policy] - * @returns {Object|undefined} - */ -const policySelector = (policy) => - policy && { - type: policy.type, - name: policy.name, - avatar: policy.avatar, - }; - export default compose( withCurrentReportID, withCurrentUserPersonalDetails, withNavigationFocus, withNetwork(), withOnyx({ - chatReports: { - key: ONYXKEYS.COLLECTION.REPORT, - selector: chatReportSelector, - initialValue: {}, - }, isLoadingApp: { key: ONYXKEYS.IS_LOADING_APP, }, @@ -316,26 +116,8 @@ export default compose( 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: {}, - }, policyMembers: { key: ONYXKEYS.COLLECTION.POLICY_MEMBERS, }, - transactionViolations: { - key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, - initialValue: {}, - }, }), )(SidebarLinksData); diff --git a/src/pages/workspace/WorkspacesListPage.tsx b/src/pages/workspace/WorkspacesListPage.tsx index aa4f1df1ba7a..5efc94ba5974 100755 --- a/src/pages/workspace/WorkspacesListPage.tsx +++ b/src/pages/workspace/WorkspacesListPage.tsx @@ -20,6 +20,7 @@ import ScreenWrapper from '@components/ScreenWrapper'; import Text from '@components/Text'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; +import {useReports} from '@hooks/useReports'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; @@ -33,7 +34,7 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import SCREENS from '@src/SCREENS'; -import type {PolicyMembers, Policy as PolicyType, ReimbursementAccount, Report} from '@src/types/onyx'; +import type {PolicyMembers, Policy as PolicyType, ReimbursementAccount} from '@src/types/onyx'; import type * as OnyxCommon from '@src/types/onyx/OnyxCommon'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import withPolicyAndFullscreenLoading from './withPolicyAndFullscreenLoading'; @@ -72,9 +73,6 @@ type WorkspaceListPageOnyxProps = { /** A collection of objects for all policies which key policy member objects by accountIDs */ allPolicyMembers: OnyxCollection; - - /** All reports shared with the user (coming from Onyx) */ - reports: OnyxCollection; }; type WorkspaceListPageProps = WithPolicyAndFullscreenLoadingProps & WorkspaceListPageOnyxProps; @@ -110,7 +108,8 @@ function dismissWorkspaceError(policyID: string, pendingAction: OnyxCommon.Pendi throw new Error('Not implemented'); } -function WorkspacesListPage({policies, allPolicyMembers, reimbursementAccount, reports}: WorkspaceListPageProps) { +function WorkspacesListPage({policies, allPolicyMembers, reimbursementAccount}: WorkspaceListPageProps) { + const reports = useReports(); const theme = useTheme(); const styles = useThemeStyles(); const {translate} = useLocalize(); @@ -407,8 +406,5 @@ export default withPolicyAndFullscreenLoading( reimbursementAccount: { key: ONYXKEYS.REIMBURSEMENT_ACCOUNT, }, - reports: { - key: ONYXKEYS.COLLECTION.REPORT, - }, })(WorkspacesListPage), ); diff --git a/src/types/onyx/PriorityMode.ts b/src/types/onyx/PriorityMode.ts new file mode 100644 index 000000000000..5fef300ed014 --- /dev/null +++ b/src/types/onyx/PriorityMode.ts @@ -0,0 +1,6 @@ +import type {ValueOf} from 'type-fest'; +import type CONST from '@src/CONST'; + +type PriorityMode = ValueOf; + +export default PriorityMode; From 6921051449b209820347247511a943d8b4624ffb Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 15 Feb 2024 19:19:58 +0500 Subject: [PATCH 02/51] perf: improve renderItem and use FlatList --- .../LHNOptionsList/LHNOptionsList.tsx | 114 +++--------------- .../LHNOptionsList/OptionRowLHNData.tsx | 51 +------- src/components/LHNOptionsList/types.ts | 3 +- src/components/withCurrentReportID.tsx | 10 +- src/hooks/useOrderedReportIDs.tsx | 38 +++++- src/libs/SidebarUtils.ts | 40 +++++- src/pages/home/sidebar/SidebarLinks.js | 4 +- src/pages/home/sidebar/SidebarLinksData.js | 2 +- src/pages/workspace/WorkspacesListPage.tsx | 4 +- 9 files changed, 111 insertions(+), 155 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index e0a869f5222a..c70e355c46c5 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -1,41 +1,27 @@ -import {FlashList} from '@shopify/flash-list'; import type {ReactElement} from 'react'; -import React, {memo, useCallback} from 'react'; -import {StyleSheet, View} from 'react-native'; -import {withOnyx} from 'react-native-onyx'; -import withCurrentReportID from '@components/withCurrentReportID'; -import usePermissions from '@hooks/usePermissions'; -import {useReports} from '@hooks/useReports'; +import React, {useCallback, memo} from 'react'; +import {FlatList, StyleSheet, View} from 'react-native'; import useThemeStyles from '@hooks/useThemeStyles'; -import * as ReportActionsUtils from '@libs/ReportActionsUtils'; -import variables from '@styles/variables'; -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; import OptionRowLHNData from './OptionRowLHNData'; -import type {LHNOptionsListOnyxProps, LHNOptionsListProps, RenderItemProps} from './types'; +import type {LHNOptionsListProps, RenderItemProps} from './types'; +import { OrderedReports } from '@libs/SidebarUtils'; +import CONST from '@src/CONST'; +import variables from '@styles/variables'; +import { FlashList } from '@shopify/flash-list'; -const keyExtractor = (item: string) => `report_${item}`; +const keyExtractor = (item: OrderedReports) => `report_${item?.reportID}`; function LHNOptionsList({ style, contentContainerStyles, data, onSelectRow, - optionMode, shouldDisableFocusOptions = false, - reportActions = {}, - policy = {}, - preferredLocale = CONST.LOCALES.DEFAULT, - personalDetails = {}, - transactions = {}, currentReportID = '', - draftComments = {}, - transactionViolations = {}, + optionMode, onFirstItemRendered = () => {}, }: LHNOptionsListProps) { - const reports = useReports(); const styles = useThemeStyles(); - const {canUseViolations} = usePermissions(); // When the first item renders we want to call the onFirstItemRendered callback. // At this point in time we know that the list is actually displaying items. @@ -53,69 +39,29 @@ function LHNOptionsList({ * Function which renders a row in the list */ const renderItem = useCallback( - ({item: reportID}: RenderItemProps): ReactElement => { - const itemFullReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? null; - const itemReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? null; - const itemParentReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null; - const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null; - const itemPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null; - const transactionID = itemParentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? itemParentReportAction.originalMessage.IOUTransactionID ?? '' : ''; - const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? null; - const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? ''; - const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions); - const lastReportAction = sortedReportActions[0]; - - // Get the transaction for the last report action - let lastReportActionTransactionID = ''; - - if (lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU) { - lastReportActionTransactionID = lastReportAction.originalMessage?.IOUTransactionID ?? ''; - } - const lastReportActionTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${lastReportActionTransactionID}`] ?? {}; + ({item}: RenderItemProps): ReactElement => { return ( ); }, [ currentReportID, - draftComments, onSelectRow, - optionMode, - personalDetails, - policy, - preferredLocale, - reportActions, - reports, - shouldDisableFocusOptions, - transactions, - transactionViolations, - canUseViolations, onLayoutItem, ], ); return ( - @@ -133,30 +79,6 @@ function LHNOptionsList({ LHNOptionsList.displayName = 'LHNOptionsList'; -export default withCurrentReportID( - withOnyx({ - reportActions: { - key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, - }, - policy: { - key: ONYXKEYS.COLLECTION.POLICY, - }, - preferredLocale: { - key: ONYXKEYS.NVP_PREFERRED_LOCALE, - }, - personalDetails: { - key: ONYXKEYS.PERSONAL_DETAILS_LIST, - }, - transactions: { - key: ONYXKEYS.COLLECTION.TRANSACTION, - }, - draftComments: { - key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, - }, - transactionViolations: { - key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, - }, - })(memo(LHNOptionsList)), -); +export default memo(LHNOptionsList); export type {LHNOptionsListProps}; diff --git a/src/components/LHNOptionsList/OptionRowLHNData.tsx b/src/components/LHNOptionsList/OptionRowLHNData.tsx index a18d5a8ec1ec..622d7b9d9123 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.tsx +++ b/src/components/LHNOptionsList/OptionRowLHNData.tsx @@ -16,61 +16,12 @@ import type {OptionRowLHNDataProps} from './types'; */ function OptionRowLHNData({ isFocused = false, - fullReport, - reportActions, - personalDetails = {}, - preferredLocale = CONST.LOCALES.DEFAULT, comment, - policy, - receiptTransactions, - parentReportAction, - transaction, - lastReportActionTransaction = {}, - transactionViolations, - canUseViolations, + optionItem, ...propsToForward }: OptionRowLHNDataProps) { const reportID = propsToForward.reportID; - const optionItemRef = useRef(); - - const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(fullReport, transactionViolations, parentReportAction ?? null); - - const optionItem = useMemo(() => { - // Note: ideally we'd have this as a dependent selector in onyx! - const item = SidebarUtils.getOptionData({ - report: fullReport, - reportActions, - personalDetails, - preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT, - policy, - parentReportAction, - hasViolations: !!hasViolations, - }); - if (deepEqual(item, optionItemRef.current)) { - return optionItemRef.current; - } - - optionItemRef.current = item; - - return item; - // Listen parentReportAction to update title of thread report when parentReportAction changed - // Listen to transaction to update title of transaction report when transaction changed - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ - fullReport, - lastReportActionTransaction, - reportActions, - personalDetails, - preferredLocale, - policy, - parentReportAction, - transaction, - transactionViolations, - canUseViolations, - receiptTransactions, - ]); - useEffect(() => { if (!optionItem || !!optionItem.hasDraftComment || !comment || comment.length <= 0 || isFocused) { return; diff --git a/src/components/LHNOptionsList/types.ts b/src/components/LHNOptionsList/types.ts index c58770c8383f..4c79536571bf 100644 --- a/src/components/LHNOptionsList/types.ts +++ b/src/components/LHNOptionsList/types.ts @@ -8,6 +8,7 @@ import type CONST from '@src/CONST'; import type {OptionData} from '@src/libs/ReportUtils'; import type {Locale, PersonalDetailsList, Policy, Report, ReportAction, ReportActions, Transaction, TransactionViolation} from '@src/types/onyx'; import type {EmptyObject} from '@src/types/utils/EmptyObject'; +import { OrderedReports } from '@libs/SidebarUtils'; type OptionMode = ValueOf; @@ -134,6 +135,6 @@ type OptionRowLHNProps = { onLayout?: (event: LayoutChangeEvent) => void; }; -type RenderItemProps = {item: string}; +type RenderItemProps = {item: OrderedReports}; export type {LHNOptionsListProps, OptionRowLHNDataProps, OptionRowLHNProps, LHNOptionsListOnyxProps, RenderItemProps}; diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index a452e7565b4e..cc49c44e0e77 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -39,7 +39,15 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro */ const updateCurrentReportID = useCallback( (state: NavigationState) => { - setCurrentReportID(Navigation.getTopmostReportId(state) ?? ''); + const reportID = Navigation.getTopmostReportId(state) ?? ''; + /** + * This is to make sure we don't set the undefined as reportID when + * switching between chat list and settings->workspaces tab. + * and doing so avoid unnecessary re-render of `useOrderedReportIDs`. + */ + if (reportID) { + setCurrentReportID(reportID); + } }, [setCurrentReportID], ); diff --git a/src/hooks/useOrderedReportIDs.tsx b/src/hooks/useOrderedReportIDs.tsx index 3217830054de..01dab5a3231a 100644 --- a/src/hooks/useOrderedReportIDs.tsx +++ b/src/hooks/useOrderedReportIDs.tsx @@ -1,6 +1,7 @@ import React, {createContext, useContext, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; +import {usePersonalDetails} from '@components/OnyxProvider'; import {getCurrentUserAccountID} from '@libs/actions/Report'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import SidebarUtils from '@libs/SidebarUtils'; @@ -10,6 +11,7 @@ import type {Beta, Policy, PolicyMembers, ReportAction, ReportActions, Transacti import type PriorityMode from '@src/types/onyx/PriorityMode'; import useActiveWorkspace from './useActiveWorkspace'; import useCurrentReportID from './useCurrentReportID'; +import usePermissions from './usePermissions'; import {useReports} from './useReports'; type OnyxProps = { @@ -31,6 +33,8 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP const chatReports = useReports(); const currentReportIDValue = useCurrentReportID(); const {activeWorkspaceID} = useActiveWorkspace(); + const personalDetails = usePersonalDetails(); + const {canUseViolations} = usePermissions(); const policyMemberAccountIDs = useMemo( () => getPolicyMembersByIdWithoutCurrentUser(props.policyMembers, activeWorkspaceID, getCurrentUserAccountID()), @@ -49,8 +53,25 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP props.transactionViolations, activeWorkspaceID, policyMemberAccountIDs, + personalDetails, + props.preferredLocale, + canUseViolations, + props.draftComments, ), - [chatReports, props.betas, props.policies, props.priorityMode, props.allReportActions, props.transactionViolations, activeWorkspaceID, policyMemberAccountIDs], + [ + chatReports, + props.betas, + props.policies, + props.priorityMode, + props.allReportActions, + props.transactionViolations, + activeWorkspaceID, + policyMemberAccountIDs, + personalDetails, + props.preferredLocale, + canUseViolations, + props.draftComments, + ], ); // We need to make sure the current report is in the list of reports, but we do not want @@ -70,6 +91,10 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP props.transactionViolations, activeWorkspaceID, policyMemberAccountIDs, + personalDetails, + props.preferredLocale, + canUseViolations, + props.draftComments, ); } return optionListItems; @@ -84,6 +109,10 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP props.policies, props.priorityMode, props.transactionViolations, + personalDetails, + props.preferredLocale, + canUseViolations, + props.draftComments, ]); return {props.children}; @@ -136,6 +165,13 @@ const OrderedReportIDsContextProvider = withOnyx, currentPolicyID = '', policyMemberAccountIDs: number[] = [], -): string[] { + personalDetails: OnyxEntry, + preferredLocale: DeepValueOf, + canUseViolations: boolean, + draftComments: OnyxCollection, +): OrderedReports[] { const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; const isInDefaultMode = !isInGSDMode; const allReportsDictValues = Object.values(allReports ?? {}); @@ -190,7 +200,33 @@ function getOrderedReportIDs( // Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID. // The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar. - const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); + const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => { + const itemFullReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.reportID}`] ?? null; + const itemReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.reportID}`] ?? null; + const itemParentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null; + const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null; + const itemPolicy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null; + const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report?.reportID}`] ?? ''; + + const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(itemFullReport, transactionViolations, itemParentReportAction ?? null); + + const item = getOptionData({ + report: itemFullReport, + reportActions: itemReportActions, + personalDetails, + preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT, + policy: itemPolicy, + parentReportAction: itemParentReportAction, + hasViolations: !!hasViolations, + }); + + return { + reportID: report.reportID, + optionItem: item, + comment: itemComment, + } + }); + return LHNReports; } diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index e78e656e0b7e..4c05c3d44ae8 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -1,7 +1,7 @@ /* eslint-disable rulesdir/onyx-props-must-have-default */ import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React, {useCallback, useEffect, useMemo, useRef} from 'react'; +import React, {memo, useCallback, useEffect, useMemo, useRef} from 'react'; import {InteractionManager, StyleSheet, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; @@ -179,5 +179,5 @@ export default withOnyx({ activePolicy: { key: ({activeWorkspaceID}) => `${ONYXKEYS.COLLECTION.POLICY}${activeWorkspaceID}`, }, -})(SidebarLinks); +})(memo(SidebarLinks)); export {basePropTypes}; diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 42681fea451b..87006d3debe2 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -46,7 +46,7 @@ const defaultProps = { }, }; -function SidebarLinksData({isFocused, currentReportID, insets, isLoadingApp, onLinkClick, priorityMode, network, policyMembers, session: {accountID}}) { +function SidebarLinksData({isFocused, currentReportID, insets, isLoadingApp, onLinkClick, priorityMode, network, policyMembers, session: {accountID}}) { const styles = useThemeStyles(); const {activeWorkspaceID} = useActiveWorkspace(); const {translate} = useLocalize(); diff --git a/src/pages/workspace/WorkspacesListPage.tsx b/src/pages/workspace/WorkspacesListPage.tsx index d778648ea998..968148a8becb 100755 --- a/src/pages/workspace/WorkspacesListPage.tsx +++ b/src/pages/workspace/WorkspacesListPage.tsx @@ -107,6 +107,8 @@ function dismissWorkspaceError(policyID: string, pendingAction: OnyxCommon.Pendi throw new Error('Not implemented'); } +const stickyHeaderIndices = [0]; + function WorkspacesListPage({policies, allPolicyMembers, reimbursementAccount}: WorkspaceListPageProps) { const reports = useReports(); const theme = useTheme(); @@ -375,7 +377,7 @@ function WorkspacesListPage({policies, allPolicyMembers, reimbursementAccount}: data={workspaces} renderItem={getMenuItem} ListHeaderComponent={listHeaderComponent} - stickyHeaderIndices={[0]} + stickyHeaderIndices={stickyHeaderIndices} /> Date: Mon, 26 Feb 2024 17:58:22 +0100 Subject: [PATCH 03/51] refactor: revert changes to withCurrentReportID --- src/components/withCurrentReportID.tsx | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index cc49c44e0e77..a452e7565b4e 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -39,15 +39,7 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro */ const updateCurrentReportID = useCallback( (state: NavigationState) => { - const reportID = Navigation.getTopmostReportId(state) ?? ''; - /** - * This is to make sure we don't set the undefined as reportID when - * switching between chat list and settings->workspaces tab. - * and doing so avoid unnecessary re-render of `useOrderedReportIDs`. - */ - if (reportID) { - setCurrentReportID(reportID); - } + setCurrentReportID(Navigation.getTopmostReportId(state) ?? ''); }, [setCurrentReportID], ); From 9ae8701c7ed4a5c98e69d689e63c4ac19694ebf2 Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Mon, 26 Feb 2024 18:48:44 +0100 Subject: [PATCH 04/51] refactor: remove useReports context & hooks --- src/App.tsx | 2 - src/hooks/useReports.tsx | 44 ------------------- .../AppNavigator/ReportScreenIDSetter.ts | 13 ++++-- src/pages/workspace/WorkspacesListPage.tsx | 10 +++-- 4 files changed, 16 insertions(+), 53 deletions(-) delete mode 100644 src/hooks/useReports.tsx diff --git a/src/App.tsx b/src/App.tsx index 7cbf3478d16e..eb1750a7fe5f 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -30,7 +30,6 @@ import {WindowDimensionsProvider} from './components/withWindowDimensions'; import Expensify from './Expensify'; import useDefaultDragAndDrop from './hooks/useDefaultDragAndDrop'; import {OrderedReportIDsContextProvider} from './hooks/useOrderedReportIDs'; -import {ReportsContextProvider} from './hooks/useReports'; import OnyxUpdateManager from './libs/actions/OnyxUpdateManager'; import InitialUrlContext from './libs/InitialUrlContext'; import {ReportAttachmentsProvider} from './pages/home/report/ReportAttachmentsContext'; @@ -77,7 +76,6 @@ function App({url}: AppProps) { CustomStatusBarAndBackgroundContextProvider, ActiveElementRoleProvider, ActiveWorkspaceContextProvider, - ReportsContextProvider, OrderedReportIDsContextProvider, PlaybackContextProvider, VolumeContextProvider, diff --git a/src/hooks/useReports.tsx b/src/hooks/useReports.tsx deleted file mode 100644 index c4082149cbf4..000000000000 --- a/src/hooks/useReports.tsx +++ /dev/null @@ -1,44 +0,0 @@ -import React, {createContext, useContext, useEffect, useMemo, useState} from 'react'; -import Onyx from 'react-native-onyx'; -import type {OnyxCollection} from 'react-native-onyx'; -import ONYXKEYS from '@src/ONYXKEYS'; -import type {Report} from '@src/types/onyx'; - -type Reports = OnyxCollection; -type ReportsContextValue = Reports; - -type ReportsContextProviderProps = { - children: React.ReactNode; -}; - -const ReportsContext = createContext(null); - -function ReportsContextProvider(props: ReportsContextProviderProps) { - const [reports, setReports] = useState(null); - - useEffect(() => { - // eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs - const connID = Onyx.connect({ - key: ONYXKEYS.COLLECTION.REPORT, - waitForCollectionCallback: true, - callback: (val) => { - setReports(val); - }, - }); - return () => { - Onyx.disconnect(connID); - }; - }, []); - - const contextValue = useMemo(() => reports ?? {}, [reports]); - - return {props.children}; -} - -function useReports() { - return useContext(ReportsContext); -} - -ReportsContextProvider.displayName = 'ReportsContextProvider'; - -export {ReportsContextProvider, ReportsContext, useReports}; diff --git a/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts b/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts index 297313e513f6..529f0f3d31a7 100644 --- a/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts +++ b/src/libs/Navigation/AppNavigator/ReportScreenIDSetter.ts @@ -3,7 +3,6 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {withOnyx} from 'react-native-onyx'; import useActiveWorkspace from '@hooks/useActiveWorkspace'; import usePermissions from '@hooks/usePermissions'; -import {useReports} from '@hooks/useReports'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -11,6 +10,9 @@ import type {Policy, PolicyMembers, Report, ReportMetadata} from '@src/types/ony import type {ReportScreenWrapperProps} from './ReportScreenWrapper'; type ReportScreenIDSetterComponentProps = { + /** Available reports that would be displayed in this navigator */ + reports: OnyxCollection; + /** The policies which the user has access to */ policies: OnyxCollection; @@ -56,10 +58,9 @@ const getLastAccessedReportID = ( }; // This wrapper is reponsible for opening the last accessed report if there is no reportID specified in the route params -function ReportScreenIDSetter({route, policies, policyMembers = {}, navigation, isFirstTimeNewExpensifyUser = false, reportMetadata, accountID}: ReportScreenIDSetterProps) { +function ReportScreenIDSetter({route, reports, policies, policyMembers = {}, navigation, isFirstTimeNewExpensifyUser = false, reportMetadata, accountID}: ReportScreenIDSetterProps) { const {canUseDefaultRooms} = usePermissions(); const {activeWorkspaceID} = useActiveWorkspace(); - const reports = useReports(); useEffect(() => { // Don't update if there is a reportID in the params already @@ -80,7 +81,7 @@ function ReportScreenIDSetter({route, policies, policyMembers = {}, navigation, !canUseDefaultRooms, policies, isFirstTimeNewExpensifyUser, - !!route?.params?.openOnAdminRoom, + !!reports?.params?.openOnAdminRoom, reportMetadata, activeWorkspaceID, policyMemberAccountIDs, @@ -101,6 +102,10 @@ function ReportScreenIDSetter({route, policies, policyMembers = {}, navigation, ReportScreenIDSetter.displayName = 'ReportScreenIDSetter'; export default withOnyx({ + reports: { + key: ONYXKEYS.COLLECTION.REPORT, + allowStaleData: true, + }, policies: { key: ONYXKEYS.COLLECTION.POLICY, allowStaleData: true, diff --git a/src/pages/workspace/WorkspacesListPage.tsx b/src/pages/workspace/WorkspacesListPage.tsx index f646366360eb..a51efd608444 100755 --- a/src/pages/workspace/WorkspacesListPage.tsx +++ b/src/pages/workspace/WorkspacesListPage.tsx @@ -20,7 +20,6 @@ import ScreenWrapper from '@components/ScreenWrapper'; import Text from '@components/Text'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; -import {useReports} from '@hooks/useReports'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; @@ -73,6 +72,9 @@ type WorkspaceListPageOnyxProps = { /** A collection of objects for all policies which key policy member objects by accountIDs */ allPolicyMembers: OnyxCollection; + + /** All reports shared with the user (coming from Onyx) */ + reports: OnyxCollection; }; type WorkspaceListPageProps = WithPolicyAndFullscreenLoadingProps & WorkspaceListPageOnyxProps; @@ -110,8 +112,7 @@ function dismissWorkspaceError(policyID: string, pendingAction: OnyxCommon.Pendi const stickyHeaderIndices = [0]; -function WorkspacesListPage({policies, allPolicyMembers, reimbursementAccount}: WorkspaceListPageProps) { - const reports = useReports(); +function WorkspacesListPage({policies, allPolicyMembers, reimbursementAccount, reports}: WorkspaceListPageProps) { const theme = useTheme(); const styles = useThemeStyles(); const {translate} = useLocalize(); @@ -409,5 +410,8 @@ export default withPolicyAndFullscreenLoading( reimbursementAccount: { key: ONYXKEYS.REIMBURSEMENT_ACCOUNT, }, + reports: { + key: ONYXKEYS.COLLECTION.REPORT, + }, })(WorkspacesListPage), ); From b613e974a2f8e8b7c07e893e64bc6fdf377d08d1 Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Wed, 28 Feb 2024 15:54:37 +0100 Subject: [PATCH 05/51] refactor: use reports from onyx in useOrderedReportIDs --- src/hooks/useOrderedReportIDs.tsx | 69 +++++++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/src/hooks/useOrderedReportIDs.tsx b/src/hooks/useOrderedReportIDs.tsx index 01dab5a3231a..6b3ebd8d9e28 100644 --- a/src/hooks/useOrderedReportIDs.tsx +++ b/src/hooks/useOrderedReportIDs.tsx @@ -4,23 +4,26 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {usePersonalDetails} from '@components/OnyxProvider'; import {getCurrentUserAccountID} from '@libs/actions/Report'; import {getPolicyMembersByIdWithoutCurrentUser} 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 {Beta, Policy, PolicyMembers, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; +import type {Beta, Locale, Policy, PolicyMembers, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; import type PriorityMode from '@src/types/onyx/PriorityMode'; import useActiveWorkspace from './useActiveWorkspace'; import useCurrentReportID from './useCurrentReportID'; import usePermissions from './usePermissions'; -import {useReports} from './useReports'; type OnyxProps = { + chatReports: OnyxCollection; betas: OnyxEntry; policies: OnyxCollection; allReportActions: OnyxCollection; transactionViolations: OnyxCollection; policyMembers: OnyxCollection; priorityMode: OnyxEntry; + preferredLocale: OnyxEntry; + draftComments: OnyxCollection; }; type WithOrderedReportIDsContextProviderProps = OnyxProps & { @@ -30,7 +33,6 @@ type WithOrderedReportIDsContextProviderProps = OnyxProps & { const OrderedReportIDsContext = createContext({}); function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextProviderProps) { - const chatReports = useReports(); const currentReportIDValue = useCurrentReportID(); const {activeWorkspaceID} = useActiveWorkspace(); const personalDetails = usePersonalDetails(); @@ -45,7 +47,7 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP () => SidebarUtils.getOrderedReportIDs( null, - chatReports, + props.chatReports, props.betas ?? [], props.policies, props.priorityMode, @@ -59,7 +61,7 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP props.draftComments, ), [ - chatReports, + props.chatReports, props.betas, props.policies, props.priorityMode, @@ -83,7 +85,7 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP if (currentReportIDValue?.currentReportID && !optionListItems.includes(currentReportIDValue.currentReportID)) { return SidebarUtils.getOrderedReportIDs( currentReportIDValue.currentReportID, - chatReports, + props.chatReports, props.betas ?? [], props.policies, props.priorityMode, @@ -100,7 +102,7 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP return optionListItems; }, [ activeWorkspaceID, - chatReports, + props.chatReports, currentReportIDValue?.currentReportID, optionListItems, policyMemberAccountIDs, @@ -118,6 +120,52 @@ function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextP return {props.children}; } +/** + * 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. + * @param {Object} [report] + * @returns {Object|undefined} + */ +const chatReportSelector = (report) => + report && { + reportID: report.reportID, + participantAccountIDs: report.participantAccountIDs, + hasDraft: report.hasDraft, + isPinned: report.isPinned, + isHidden: report.isHidden, + notificationPreference: report.notificationPreference, + errorFields: { + addWorkspaceRoom: report.errorFields && 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), + }; + const reportActionsSelector = (reportActions: OnyxEntry) => { if (!reportActions) { return []; @@ -140,6 +188,11 @@ const reportActionsSelector = (reportActions: OnyxEntry) => { }; const OrderedReportIDsContextProvider = withOnyx({ + chatReports: { + key: ONYXKEYS.COLLECTION.REPORT, + selector: chatReportSelector, + initialValue: {}, + }, priorityMode: { key: ONYXKEYS.NVP_PRIORITY_MODE, initialValue: CONST.PRIORITY_MODE.DEFAULT, @@ -151,7 +204,7 @@ const OrderedReportIDsContextProvider = withOnyx reportActionsSelector(actions), + selector: reportActionsSelector, initialValue: {}, }, policies: { From 7f15fbc34bdc512c6c72fe8b15232dc1ba53e756 Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Wed, 28 Feb 2024 17:32:07 +0100 Subject: [PATCH 06/51] refactor: move creating orderedReport objects from getOrderedReportIds to the context itself --- .../LHNOptionsList/LHNOptionsList.tsx | 48 +++--- src/hooks/useOrderedReportIDs.tsx | 140 ++++++++---------- src/libs/SidebarUtils.ts | 46 +----- src/pages/home/sidebar/SidebarLinksData.js | 18 ++- 4 files changed, 100 insertions(+), 152 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index c70e355c46c5..8b18630a9646 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -1,13 +1,13 @@ +import {FlashList} from '@shopify/flash-list'; import type {ReactElement} from 'react'; -import React, {useCallback, memo} from 'react'; -import {FlatList, StyleSheet, View} from 'react-native'; +import React, {memo, useCallback} from 'react'; +import {StyleSheet, View} from 'react-native'; import useThemeStyles from '@hooks/useThemeStyles'; +import {OrderedReports} from '@libs/SidebarUtils'; +import variables from '@styles/variables'; +import CONST from '@src/CONST'; import OptionRowLHNData from './OptionRowLHNData'; import type {LHNOptionsListProps, RenderItemProps} from './types'; -import { OrderedReports } from '@libs/SidebarUtils'; -import CONST from '@src/CONST'; -import variables from '@styles/variables'; -import { FlashList } from '@shopify/flash-list'; const keyExtractor = (item: OrderedReports) => `report_${item?.reportID}`; @@ -39,29 +39,23 @@ function LHNOptionsList({ * Function which renders a row in the list */ const renderItem = useCallback( - ({item}: RenderItemProps): ReactElement => { - - return ( - - ); - }, - [ - currentReportID, - onSelectRow, - onLayoutItem, - ], + ({item}: RenderItemProps): ReactElement => ( + + ), + [shouldDisableFocusOptions, currentReportID, onSelectRow, onLayoutItem, optionMode], ); return ( - diff --git a/src/hooks/useOrderedReportIDs.tsx b/src/hooks/useOrderedReportIDs.tsx index 6b3ebd8d9e28..8f071f036fc7 100644 --- a/src/hooks/useOrderedReportIDs.tsx +++ b/src/hooks/useOrderedReportIDs.tsx @@ -1,4 +1,4 @@ -import React, {createContext, useContext, useMemo} from 'react'; +import React, {createContext, useCallback, useContext, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {usePersonalDetails} from '@components/OnyxProvider'; @@ -32,101 +32,90 @@ type WithOrderedReportIDsContextProviderProps = OnyxProps & { const OrderedReportIDsContext = createContext({}); -function WithOrderedReportIDsContextProvider(props: WithOrderedReportIDsContextProviderProps) { +function WithOrderedReportIDsContextProvider({ + children, + chatReports, + betas, + policies, + allReportActions, + transactionViolations, + policyMembers, + priorityMode, + preferredLocale, + draftComments, +}: WithOrderedReportIDsContextProviderProps) { const currentReportIDValue = useCurrentReportID(); - const {activeWorkspaceID} = useActiveWorkspace(); const personalDetails = usePersonalDetails(); const {canUseViolations} = usePermissions(); + const {activeWorkspaceID} = useActiveWorkspace(); - const policyMemberAccountIDs = useMemo( - () => getPolicyMembersByIdWithoutCurrentUser(props.policyMembers, activeWorkspaceID, getCurrentUserAccountID()), - [activeWorkspaceID, props.policyMembers], - ); + const policyMemberAccountIDs = useMemo(() => getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, getCurrentUserAccountID()), [activeWorkspaceID, policyMembers]); - const optionListItems = useMemo( - () => + const getOrderedReportIDs = useCallback( + (currentReportID?: string) => SidebarUtils.getOrderedReportIDs( - null, - props.chatReports, - props.betas ?? [], - props.policies, - props.priorityMode, - props.allReportActions, - props.transactionViolations, + currentReportID ?? null, + chatReports, + betas ?? [], + policies, + priorityMode, + allReportActions, + transactionViolations, activeWorkspaceID, policyMemberAccountIDs, - personalDetails, - props.preferredLocale, - canUseViolations, - props.draftComments, ), - [ - props.chatReports, - props.betas, - props.policies, - props.priorityMode, - props.allReportActions, - props.transactionViolations, - activeWorkspaceID, - policyMemberAccountIDs, - personalDetails, - props.preferredLocale, - canUseViolations, - props.draftComments, - ], + [chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs], ); + const orderedReportIDs = useMemo(() => getOrderedReportIDs(), [getOrderedReportIDs]); + // 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 here 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. - const optionListItemsWithCurrentReport = useMemo(() => { - if (currentReportIDValue?.currentReportID && !optionListItems.includes(currentReportIDValue.currentReportID)) { - return SidebarUtils.getOrderedReportIDs( - currentReportIDValue.currentReportID, - props.chatReports, - props.betas ?? [], - props.policies, - props.priorityMode, - props.allReportActions, - props.transactionViolations, - activeWorkspaceID, - policyMemberAccountIDs, - personalDetails, - props.preferredLocale, - canUseViolations, - props.draftComments, - ); + const orderedReportIDsWithCurrentReport = useMemo(() => { + if (currentReportIDValue?.currentReportID && !orderedReportIDs.includes(currentReportIDValue.currentReportID)) { + return getOrderedReportIDs(currentReportIDValue.currentReportID); } - return optionListItems; - }, [ - activeWorkspaceID, - props.chatReports, - currentReportIDValue?.currentReportID, - optionListItems, - policyMemberAccountIDs, - props.allReportActions, - props.betas, - props.policies, - props.priorityMode, - props.transactionViolations, - personalDetails, - props.preferredLocale, - canUseViolations, - props.draftComments, - ]); - - return {props.children}; + return orderedReportIDs; + }, [getOrderedReportIDs, currentReportIDValue?.currentReportID, orderedReportIDs]); + + const orderedReportListItems = useMemo( + () => + orderedReportIDsWithCurrentReport.map((reportID) => { + const itemFullReport = chatReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? null; + const itemReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? null; + const itemParentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null; + const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null; + const itemPolicy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null; + const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? ''; + + const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(itemFullReport, transactionViolations, itemParentReportAction ?? null); + + const item = SidebarUtils.getOptionData({ + report: itemFullReport, + reportActions: itemReportActions, + personalDetails, + preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT, + policy: itemPolicy, + parentReportAction: itemParentReportAction, + hasViolations: !!hasViolations, + }); + + return {reportID, optionItem: item, comment: itemComment}; + }), + [orderedReportIDsWithCurrentReport, canUseViolations, personalDetails, draftComments, preferredLocale, chatReports, allReportActions, policies, transactionViolations], + ); + + return {children}; } /** * 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. - * @param {Object} [report] - * @returns {Object|undefined} */ -const chatReportSelector = (report) => +const chatReportSelector = (report: OnyxEntry) => report && { reportID: report.reportID, participantAccountIDs: report.participantAccountIDs, @@ -134,9 +123,7 @@ const chatReportSelector = (report) => isPinned: report.isPinned, isHidden: report.isHidden, notificationPreference: report.notificationPreference, - errorFields: { - addWorkspaceRoom: report.errorFields && report.errorFields.addWorkspaceRoom, - }, + errorFields: {addWorkspaceRoom: report.errorFields?.addWorkspaceRoom}, lastMessageText: report.lastMessageText, lastVisibleActionCreated: report.lastVisibleActionCreated, iouReportID: report.iouReportID, @@ -188,6 +175,7 @@ const reportActionsSelector = (reportActions: OnyxEntry) => { }; const OrderedReportIDsContextProvider = withOnyx({ + // @ts-expect-error Need some help in determining the correct type for this selector chatReports: { key: ONYXKEYS.COLLECTION.REPORT, selector: chatReportSelector, diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index f99384487650..f36becf40715 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -59,30 +59,20 @@ function compareStringDates(a: string, b: string): 0 | 1 | -1 { return 0; } -export type OrderedReports = { - reportID: string; - optionItem: ReportUtils.OptionData | undefined; - comment: string; -}; - /** * @returns An array of reportIDs sorted in the proper order */ function getOrderedReportIDs( currentReportId: string | null, allReports: OnyxCollection, - betas: Beta[], + betas: OnyxEntry, policies: OnyxCollection, priorityMode: OnyxEntry, allReportActions: OnyxCollection, transactionViolations: OnyxCollection, currentPolicyID = '', policyMemberAccountIDs: number[] = [], - personalDetails: OnyxEntry, - preferredLocale: DeepValueOf, - canUseViolations: boolean, - draftComments: OnyxCollection, -): OrderedReports[] { +): string[] { const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD; const isInDefaultMode = !isInGSDMode; const allReportsDictValues = Object.values(allReports ?? {}); @@ -93,12 +83,12 @@ function getOrderedReportIDs( const parentReportActions = allReportActions?.[parentReportActionsKey]; const parentReportAction = parentReportActions?.find((action) => action && report && action?.reportActionID === report?.parentReportActionID); const doesReportHaveViolations = - betas.includes(CONST.BETAS.VIOLATIONS) && !!parentReportAction && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction); + !!betas && betas.includes(CONST.BETAS.VIOLATIONS) && !!parentReportAction && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction); return ReportUtils.shouldReportBeInOptionList({ report, currentReportId: currentReportId ?? '', isInGSDMode, - betas, + betas: betas ?? [], policies, excludeEmptyChats: true, doesReportHaveViolations, @@ -178,33 +168,7 @@ function getOrderedReportIDs( // Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID. // The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar. - const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => { - const itemFullReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.reportID}`] ?? null; - const itemReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.reportID}`] ?? null; - const itemParentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null; - const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null; - const itemPolicy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null; - const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report?.reportID}`] ?? ''; - - const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(itemFullReport, transactionViolations, itemParentReportAction ?? null); - - const item = getOptionData({ - report: itemFullReport, - reportActions: itemReportActions, - personalDetails, - preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT, - policy: itemPolicy, - parentReportAction: itemParentReportAction, - hasViolations: !!hasViolations, - }); - - return { - reportID: report.reportID, - optionItem: item, - comment: itemComment, - }; - }); - + const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); return LHNReports; } diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 6d63a8a44fe1..4966a2414e97 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -51,22 +51,24 @@ function SidebarLinksData({isFocused, currentReportID, insets, isLoadingApp, onL // eslint-disable-next-line react-hooks/exhaustive-deps useEffect(() => Policy.openWorkspace(activeWorkspaceID, policyMemberAccountIDs), [activeWorkspaceID]); - const reportIDsRef = useRef(null); + const orderedReportListItemsRef = useRef(null); const isLoading = isLoadingApp; - const reportIDs = useOrderedReportIDs(); + const orderedReportListItems = useOrderedReportIDs(); + const optionListItems = useMemo(() => { - if (deepEqual(reportIDsRef.current, reportIDs)) { - return reportIDsRef.current; + // this can be very heavy because we are no longer comapring the IDS but the whole objects for the list + if (deepEqual(orderedReportListItemsRef.current, orderedReportListItems)) { + return orderedReportListItemsRef.current; } // 1. We need to update existing reports only once while loading because they are updated several times during loading and causes this regression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 // 2. If the user is offline, we need to update the reports unconditionally, since the loading of report data might be stuck in this case. // 3. Changing priority mode to Most Recent will call OpenApp. If there is an existing reports and the priority mode is updated, we want to immediately update the list instead of waiting the OpenApp request to complete - if (!isLoading || !reportIDsRef.current || network.isOffline || (reportIDsRef.current && prevPriorityMode !== priorityMode)) { - reportIDsRef.current = reportIDs; + if (!isLoading || !orderedReportListItemsRef.current || network.isOffline || (orderedReportListItemsRef.current && prevPriorityMode !== priorityMode)) { + orderedReportListItemsRef.current = orderedReportListItems; } - return reportIDsRef.current || []; - }, [reportIDs, isLoading, network.isOffline, prevPriorityMode, priorityMode]); + return orderedReportListItemsRef.current || []; + }, [orderedReportListItems, isLoading, network.isOffline, prevPriorityMode, priorityMode]); const currentReportIDRef = useRef(currentReportID); currentReportIDRef.current = currentReportID; From 7fa44fc71ce4eb20d70b4da92094b9a5b6d50394 Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Wed, 28 Feb 2024 17:34:55 +0100 Subject: [PATCH 07/51] Revert "refactor: revert changes to withCurrentReportID" This reverts commit dc88950ac4dbc552e4ca8b705ae4faff05dd70da. --- src/components/withCurrentReportID.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index a452e7565b4e..cc49c44e0e77 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -39,7 +39,15 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro */ const updateCurrentReportID = useCallback( (state: NavigationState) => { - setCurrentReportID(Navigation.getTopmostReportId(state) ?? ''); + const reportID = Navigation.getTopmostReportId(state) ?? ''; + /** + * This is to make sure we don't set the undefined as reportID when + * switching between chat list and settings->workspaces tab. + * and doing so avoid unnecessary re-render of `useOrderedReportIDs`. + */ + if (reportID) { + setCurrentReportID(reportID); + } }, [setCurrentReportID], ); From ae12b008886272f08668a089342bc9bacbcfb831 Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Wed, 28 Feb 2024 17:47:45 +0100 Subject: [PATCH 08/51] refactor: remove comment in SidebarLinksData --- src/pages/home/sidebar/SidebarLinksData.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 4966a2414e97..5cedc9bbb7fd 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -56,7 +56,6 @@ function SidebarLinksData({isFocused, currentReportID, insets, isLoadingApp, onL const orderedReportListItems = useOrderedReportIDs(); const optionListItems = useMemo(() => { - // this can be very heavy because we are no longer comapring the IDS but the whole objects for the list if (deepEqual(orderedReportListItemsRef.current, orderedReportListItems)) { return orderedReportListItemsRef.current; } From c87f579bda3c94452f6b093edfbc159996a7eb30 Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Wed, 28 Feb 2024 18:00:43 +0100 Subject: [PATCH 09/51] refactor: rename useOrderedReportIDs to useOrderedReportListItems --- src/App.tsx | 4 ++-- src/components/withCurrentReportID.tsx | 2 +- ...tIDs.tsx => useOrderedReportListItems.tsx} | 20 +++++++++---------- src/pages/home/sidebar/SidebarLinksData.js | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) rename src/hooks/{useOrderedReportIDs.tsx => useOrderedReportListItems.tsx} (92%) diff --git a/src/App.tsx b/src/App.tsx index eb1750a7fe5f..5670859e8908 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -29,7 +29,7 @@ import {KeyboardStateProvider} from './components/withKeyboardState'; import {WindowDimensionsProvider} from './components/withWindowDimensions'; import Expensify from './Expensify'; import useDefaultDragAndDrop from './hooks/useDefaultDragAndDrop'; -import {OrderedReportIDsContextProvider} from './hooks/useOrderedReportIDs'; +import {OrderedReportListItemsContextProvider} from './hooks/useOrderedReportListItems'; import OnyxUpdateManager from './libs/actions/OnyxUpdateManager'; import InitialUrlContext from './libs/InitialUrlContext'; import {ReportAttachmentsProvider} from './pages/home/report/ReportAttachmentsContext'; @@ -76,7 +76,7 @@ function App({url}: AppProps) { CustomStatusBarAndBackgroundContextProvider, ActiveElementRoleProvider, ActiveWorkspaceContextProvider, - OrderedReportIDsContextProvider, + OrderedReportListItemsContextProvider, PlaybackContextProvider, VolumeContextProvider, VideoPopoverMenuContextProvider, diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index cc49c44e0e77..d5c5a6896a9c 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -43,7 +43,7 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro /** * This is to make sure we don't set the undefined as reportID when * switching between chat list and settings->workspaces tab. - * and doing so avoid unnecessary re-render of `useOrderedReportIDs`. + * and doing so avoid unnecessary re-render of `useOrderedReportListItems`. */ if (reportID) { setCurrentReportID(reportID); diff --git a/src/hooks/useOrderedReportIDs.tsx b/src/hooks/useOrderedReportListItems.tsx similarity index 92% rename from src/hooks/useOrderedReportIDs.tsx rename to src/hooks/useOrderedReportListItems.tsx index 8f071f036fc7..643fcf053012 100644 --- a/src/hooks/useOrderedReportIDs.tsx +++ b/src/hooks/useOrderedReportListItems.tsx @@ -26,13 +26,13 @@ type OnyxProps = { draftComments: OnyxCollection; }; -type WithOrderedReportIDsContextProviderProps = OnyxProps & { +type WithOrderedReportListItemsContextProviderProps = OnyxProps & { children: React.ReactNode; }; -const OrderedReportIDsContext = createContext({}); +const OrderedReportListItemsContext = createContext({}); -function WithOrderedReportIDsContextProvider({ +function WithOrderedReportListItemsContextProvider({ children, chatReports, betas, @@ -43,7 +43,7 @@ function WithOrderedReportIDsContextProvider({ priorityMode, preferredLocale, draftComments, -}: WithOrderedReportIDsContextProviderProps) { +}: WithOrderedReportListItemsContextProviderProps) { const currentReportIDValue = useCurrentReportID(); const personalDetails = usePersonalDetails(); const {canUseViolations} = usePermissions(); @@ -108,7 +108,7 @@ function WithOrderedReportIDsContextProvider({ [orderedReportIDsWithCurrentReport, canUseViolations, personalDetails, draftComments, preferredLocale, chatReports, allReportActions, policies, transactionViolations], ); - return {children}; + return {children}; } /** @@ -174,7 +174,7 @@ const reportActionsSelector = (reportActions: OnyxEntry) => { }); }; -const OrderedReportIDsContextProvider = withOnyx({ +const OrderedReportListItemsContextProvider = withOnyx({ // @ts-expect-error Need some help in determining the correct type for this selector chatReports: { key: ONYXKEYS.COLLECTION.REPORT, @@ -213,10 +213,10 @@ const OrderedReportIDsContextProvider = withOnyx { if (deepEqual(orderedReportListItemsRef.current, orderedReportListItems)) { From df1069c1a1dc695f8faa0dcfd748541bbe1ea8bb Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Wed, 28 Feb 2024 18:05:35 +0100 Subject: [PATCH 10/51] perf: use memo for extraData in LHNOptionList --- src/components/LHNOptionsList/LHNOptionsList.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 8b18630a9646..8301bc034a74 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -1,6 +1,6 @@ import {FlashList} from '@shopify/flash-list'; import type {ReactElement} from 'react'; -import React, {memo, useCallback} from 'react'; +import React, {memo, useCallback, useMemo} from 'react'; import {StyleSheet, View} from 'react-native'; import useThemeStyles from '@hooks/useThemeStyles'; import {OrderedReports} from '@libs/SidebarUtils'; @@ -53,6 +53,8 @@ function LHNOptionsList({ [shouldDisableFocusOptions, currentReportID, onSelectRow, onLayoutItem, optionMode], ); + const extraData = useMemo(() => [currentReportID], [currentReportID]); + return ( From 1d294bcd188dd768d7c917c48ba19f54bf5a0275 Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Wed, 28 Feb 2024 19:14:59 +0100 Subject: [PATCH 11/51] fix: typescript fixes --- .../LHNOptionsList/LHNOptionsList.tsx | 5 +- .../LHNOptionsList/OptionRowLHNData.tsx | 14 +--- src/components/LHNOptionsList/types.ts | 74 ++++--------------- src/hooks/useOrderedReportListItems.tsx | 3 +- src/libs/SidebarUtils.ts | 3 +- src/pages/home/sidebar/SidebarLinks.js | 2 +- src/pages/home/sidebar/SidebarLinksData.js | 4 + src/types/onyx/index.ts | 2 + 8 files changed, 27 insertions(+), 80 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 8301bc034a74..1884997a58fe 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -3,13 +3,12 @@ import type {ReactElement} from 'react'; import React, {memo, useCallback, useMemo} from 'react'; import {StyleSheet, View} from 'react-native'; import useThemeStyles from '@hooks/useThemeStyles'; -import {OrderedReports} from '@libs/SidebarUtils'; import variables from '@styles/variables'; import CONST from '@src/CONST'; import OptionRowLHNData from './OptionRowLHNData'; -import type {LHNOptionsListProps, RenderItemProps} from './types'; +import type {LHNOptionsListProps, OptionListItem, RenderItemProps} from './types'; -const keyExtractor = (item: OrderedReports) => `report_${item?.reportID}`; +const keyExtractor = (item: OptionListItem) => `report_${item.reportID}`; function LHNOptionsList({ style, diff --git a/src/components/LHNOptionsList/OptionRowLHNData.tsx b/src/components/LHNOptionsList/OptionRowLHNData.tsx index 622d7b9d9123..bc1a06ce5f67 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.tsx +++ b/src/components/LHNOptionsList/OptionRowLHNData.tsx @@ -1,10 +1,5 @@ -import {deepEqual} from 'fast-equals'; -import React, {useEffect, useMemo, useRef} from 'react'; -import * as ReportUtils from '@libs/ReportUtils'; -import SidebarUtils from '@libs/SidebarUtils'; +import React, {useEffect} from 'react'; import * as Report from '@userActions/Report'; -import CONST from '@src/CONST'; -import type {OptionData} from '@src/libs/ReportUtils'; import OptionRowLHN from './OptionRowLHN'; import type {OptionRowLHNDataProps} from './types'; @@ -14,12 +9,7 @@ import type {OptionRowLHNDataProps} from './types'; * The OptionRowLHN component is memoized, so it will only * re-render if the data really changed. */ -function OptionRowLHNData({ - isFocused = false, - comment, - optionItem, - ...propsToForward -}: OptionRowLHNDataProps) { +function OptionRowLHNData({isFocused = false, comment, optionItem, ...propsToForward}: OptionRowLHNDataProps) { const reportID = propsToForward.reportID; useEffect(() => { diff --git a/src/components/LHNOptionsList/types.ts b/src/components/LHNOptionsList/types.ts index 4c79536571bf..3f72748ab8a8 100644 --- a/src/components/LHNOptionsList/types.ts +++ b/src/components/LHNOptionsList/types.ts @@ -1,38 +1,22 @@ import type {ContentStyle} from '@shopify/flash-list'; import type {RefObject} from 'react'; import type {LayoutChangeEvent, StyleProp, TextStyle, View, ViewStyle} from 'react-native'; -import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; import type {CurrentReportIDContextValue} from '@components/withCurrentReportID'; import type CONST from '@src/CONST'; import type {OptionData} from '@src/libs/ReportUtils'; -import type {Locale, PersonalDetailsList, Policy, Report, ReportAction, ReportActions, Transaction, TransactionViolation} from '@src/types/onyx'; -import type {EmptyObject} from '@src/types/utils/EmptyObject'; -import { OrderedReports } from '@libs/SidebarUtils'; type OptionMode = ValueOf; -type LHNOptionsListOnyxProps = { - /** The policy which the user has access to and which the report could be tied to */ - policy: OnyxCollection; - - /** Array of report actions for this report */ - reportActions: OnyxCollection; - - /** Indicates which locale the user currently has selected */ - preferredLocale: OnyxEntry; - - /** List of users' personal details */ - personalDetails: OnyxEntry; - - /** The transaction from the parent report action */ - transactions: OnyxCollection; +type OptionListItem = { + /** The reportID of the report */ + reportID: string; - /** List of draft comments */ - draftComments: OnyxCollection; + /** The item that should be rendered */ + optionItem: OptionData | undefined; - /** The list of transaction violations */ - transactionViolations: OnyxCollection; + /** Comment added to report */ + comment: string; }; type CustomLHNOptionsListProps = { @@ -43,7 +27,7 @@ type CustomLHNOptionsListProps = { contentContainerStyles?: StyleProp; /** Sections for the section list */ - data: string[]; + data: OptionListItem[]; /** Callback to fire when a row is selected */ onSelectRow?: (optionItem: OptionData, popoverAnchor: RefObject) => void; @@ -58,51 +42,21 @@ type CustomLHNOptionsListProps = { onFirstItemRendered: () => void; }; -type LHNOptionsListProps = CustomLHNOptionsListProps & CurrentReportIDContextValue & LHNOptionsListOnyxProps; +type LHNOptionsListProps = CustomLHNOptionsListProps & CurrentReportIDContextValue; type OptionRowLHNDataProps = { /** Whether row should be focused */ isFocused?: boolean; - /** List of users' personal details */ - personalDetails?: PersonalDetailsList; - - /** The preferred language for the app */ - preferredLocale?: OnyxEntry; - - /** The full data of the report */ - fullReport: OnyxEntry; - - /** The policy which the user has access to and which the report could be tied to */ - policy?: OnyxEntry; - - /** The action from the parent report */ - parentReportAction?: OnyxEntry; - - /** The transaction from the parent report action */ - transaction: OnyxEntry; - - /** The transaction linked to the report's last action */ - lastReportActionTransaction?: OnyxEntry; - /** Comment added to report */ comment: string; - /** The receipt transaction from the parent report action */ - receiptTransactions: OnyxCollection; + /** The item that should be rendered */ + optionItem: OptionData | undefined; /** The reportID of the report */ reportID: string; - /** Array of report actions for this report */ - reportActions: OnyxEntry; - - /** List of transaction violation */ - transactionViolations: OnyxCollection; - - /** Whether the user can use violations */ - canUseViolations: boolean | undefined; - /** Toggle between compact and default view */ viewMode?: OptionMode; @@ -130,11 +84,11 @@ type OptionRowLHNProps = { style?: StyleProp; /** The item that should be rendered */ - optionItem?: OptionData; + optionItem: OptionData | undefined; onLayout?: (event: LayoutChangeEvent) => void; }; -type RenderItemProps = {item: OrderedReports}; +type RenderItemProps = {item: OptionListItem}; -export type {LHNOptionsListProps, OptionRowLHNDataProps, OptionRowLHNProps, LHNOptionsListOnyxProps, RenderItemProps}; +export type {LHNOptionsListProps, OptionRowLHNDataProps, OptionRowLHNProps, OptionListItem, RenderItemProps}; diff --git a/src/hooks/useOrderedReportListItems.tsx b/src/hooks/useOrderedReportListItems.tsx index 643fcf053012..970f306c3a80 100644 --- a/src/hooks/useOrderedReportListItems.tsx +++ b/src/hooks/useOrderedReportListItems.tsx @@ -8,8 +8,7 @@ import * as ReportUtils from '@libs/ReportUtils'; import SidebarUtils from '@libs/SidebarUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Beta, Locale, Policy, PolicyMembers, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; -import type PriorityMode from '@src/types/onyx/PriorityMode'; +import type {Beta, Locale, Policy, PolicyMembers, PriorityMode, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; import useActiveWorkspace from './useActiveWorkspace'; import useCurrentReportID from './useCurrentReportID'; import usePermissions from './usePermissions'; diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 777a61c70733..2e929639270e 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -4,12 +4,11 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; 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'; diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 8cda70d2486a..4c9803e257bc 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -33,7 +33,7 @@ const basePropTypes = { const propTypes = { ...basePropTypes, - optionListItems: PropTypes.arrayOf(PropTypes.string).isRequired, + optionListItems: PropTypes.arrayOf(PropTypes.object).isRequired, isLoading: PropTypes.bool.isRequired, diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 91de05450aae..088899425a0b 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -31,12 +31,16 @@ const propTypes = { network: networkPropTypes.isRequired, + // eslint-disable-next-line react/forbid-prop-types + policyMembers: PropTypes.object, + ...withCurrentUserPersonalDetailsPropTypes, }; const defaultProps = { isLoadingApp: true, priorityMode: CONST.PRIORITY_MODE.DEFAULT, + policyMembers: {}, ...withCurrentUserPersonalDetailsDefaultProps, }; diff --git a/src/types/onyx/index.ts b/src/types/onyx/index.ts index 6846fc302639..f6147a27a49b 100644 --- a/src/types/onyx/index.ts +++ b/src/types/onyx/index.ts @@ -37,6 +37,7 @@ import type {PolicyMembers} from './PolicyMember'; import type PolicyMember from './PolicyMember'; import type {PolicyReportField, PolicyReportFields} from './PolicyReportField'; import type {PolicyTag, PolicyTagList, PolicyTags} from './PolicyTag'; +import type PriorityMode from './PriorityMode'; import type PrivatePersonalDetails from './PrivatePersonalDetails'; import type RecentlyUsedCategories from './RecentlyUsedCategories'; import type RecentlyUsedReportFields from './RecentlyUsedReportFields'; @@ -111,6 +112,7 @@ export type { PolicyTag, PolicyTags, PolicyTagList, + PriorityMode, PrivatePersonalDetails, RecentWaypoint, RecentlyUsedCategories, From 7dfba4379e6bc48ef256c6363743ea13ad65b614 Mon Sep 17 00:00:00 2001 From: Jakub Romanczyk Date: Wed, 28 Feb 2024 20:08:53 +0100 Subject: [PATCH 12/51] fix: update reportActionsSelector to match the one from SidebarLinksData --- src/hooks/useOrderedReportListItems.tsx | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/hooks/useOrderedReportListItems.tsx b/src/hooks/useOrderedReportListItems.tsx index 970f306c3a80..fa2d18684464 100644 --- a/src/hooks/useOrderedReportListItems.tsx +++ b/src/hooks/useOrderedReportListItems.tsx @@ -1,3 +1,5 @@ +import lodashGet from 'lodash/get'; +import lodashMap from 'lodash/map'; import React, {createContext, useCallback, useContext, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; @@ -152,29 +154,27 @@ const chatReportSelector = (report: OnyxEntry) => isUnreadWithMention: ReportUtils.isUnreadWithMention(report), }; -const reportActionsSelector = (reportActions: OnyxEntry) => { - if (!reportActions) { - return []; - } +const reportActionsSelector = (reportActions: OnyxEntry) => + reportActions && + lodashMap(reportActions, (reportAction) => { + const {reportActionID, parentReportActionID, actionName, errors = [], originalMessage} = reportAction; + const decision = lodashGet(reportAction, 'message[0].moderationDecision.decision'); - return Object.values(reportActions).map((reportAction) => { - const {reportActionID, actionName, originalMessage} = reportAction ?? {}; - const decision = reportAction?.message?.[0]?.moderationDecision?.decision; return { reportActionID, + parentReportActionID, actionName, - originalMessage, + errors, message: [ { moderationDecision: {decision}, }, ], + originalMessage, }; }); -}; const OrderedReportListItemsContextProvider = withOnyx({ - // @ts-expect-error Need some help in determining the correct type for this selector chatReports: { key: ONYXKEYS.COLLECTION.REPORT, selector: chatReportSelector, @@ -190,7 +190,6 @@ const OrderedReportListItemsContextProvider = withOnyx Date: Thu, 29 Feb 2024 18:01:07 +0500 Subject: [PATCH 13/51] fix: typescript issues --- src/hooks/useOrderedReportListItems.tsx | 70 +------------------------ src/libs/SidebarUtils.ts | 4 +- 2 files changed, 4 insertions(+), 70 deletions(-) diff --git a/src/hooks/useOrderedReportListItems.tsx b/src/hooks/useOrderedReportListItems.tsx index fa2d18684464..d368abd897af 100644 --- a/src/hooks/useOrderedReportListItems.tsx +++ b/src/hooks/useOrderedReportListItems.tsx @@ -1,9 +1,7 @@ -import lodashGet from 'lodash/get'; -import lodashMap from 'lodash/map'; import React, {createContext, useCallback, useContext, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; -import {usePersonalDetails} from '@components/OnyxProvider'; +import {usePersonalDetails, useReport} from '@components/OnyxProvider'; import {getCurrentUserAccountID} from '@libs/actions/Report'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; @@ -19,7 +17,7 @@ type OnyxProps = { chatReports: OnyxCollection; betas: OnyxEntry; policies: OnyxCollection; - allReportActions: OnyxCollection; + allReportActions: OnyxCollection; transactionViolations: OnyxCollection; policyMembers: OnyxCollection; priorityMode: OnyxEntry; @@ -112,72 +110,9 @@ function WithOrderedReportListItemsContextProvider({ return {children}; } -/** - * 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) => - report && { - reportID: report.reportID, - participantAccountIDs: report.participantAccountIDs, - hasDraft: report.hasDraft, - 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), - }; - -const reportActionsSelector = (reportActions: OnyxEntry) => - reportActions && - lodashMap(reportActions, (reportAction) => { - const {reportActionID, parentReportActionID, actionName, errors = [], originalMessage} = reportAction; - const decision = lodashGet(reportAction, 'message[0].moderationDecision.decision'); - - return { - reportActionID, - parentReportActionID, - actionName, - errors, - message: [ - { - moderationDecision: {decision}, - }, - ], - originalMessage, - }; - }); - const OrderedReportListItemsContextProvider = withOnyx({ chatReports: { key: ONYXKEYS.COLLECTION.REPORT, - selector: chatReportSelector, initialValue: {}, }, priorityMode: { @@ -190,7 +125,6 @@ const OrderedReportListItemsContextProvider = withOnyx, policies: OnyxCollection, priorityMode: OnyxEntry, - allReportActions: OnyxCollection, + allReportActions: OnyxCollection, transactionViolations: OnyxCollection, currentPolicyID = '', policyMemberAccountIDs: number[] = [], @@ -80,7 +80,7 @@ function getOrderedReportIDs( let reportsToDisplay = allReportsDictValues.filter((report) => { const parentReportActionsKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`; const parentReportActions = allReportActions?.[parentReportActionsKey]; - const parentReportAction = parentReportActions?.find((action) => action && report && action?.reportActionID === report?.parentReportActionID); + const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '']; const doesReportHaveViolations = !!betas && betas.includes(CONST.BETAS.VIOLATIONS) && !!parentReportAction && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction); return ReportUtils.shouldReportBeInOptionList({ From 6739b587e93144f2dfa7a2250ac15e3b11488289 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 29 Feb 2024 18:01:30 +0500 Subject: [PATCH 14/51] test: fix reassure failing test --- tests/perf-test/SidebarUtils.perf-test.ts | 26 ++++++--------------- tests/utils/LHNTestUtils.tsx | 3 ++- tests/utils/collections/createCollection.ts | 21 +++++++++++++++++ 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index 3aa65331b9c2..6fb63878a832 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -8,7 +8,7 @@ import type {PersonalDetails, TransactionViolation} from '@src/types/onyx'; import type Policy from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; import type ReportAction from '@src/types/onyx/ReportAction'; -import createCollection from '../utils/collections/createCollection'; +import createCollection, {createNestedCollection} from '../utils/collections/createCollection'; import createPersonalDetails from '../utils/collections/personalDetails'; import createRandomPolicy from '../utils/collections/policies'; import createRandomReportAction from '../utils/collections/reportActions'; @@ -27,6 +27,12 @@ const reportActions = createCollection( (index) => createRandomReportAction(index), ); +const allReportActions = createNestedCollection( + (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, + (item) => `${item.reportActionID}`, + (index) => createRandomReportAction(index), +); + const personalDetails = createCollection( (item) => item.accountID, (index) => createPersonalDetails(index), @@ -82,24 +88,6 @@ describe('SidebarUtils', () => { (index) => createRandomPolicy(index), ); - const allReportActions = Object.fromEntries( - Object.keys(reportActions).map((key) => [ - key, - [ - { - errors: reportActions[key].errors ?? [], - message: [ - { - moderationDecision: { - decision: reportActions[key].message?.[0]?.moderationDecision?.decision, - }, - }, - ], - }, - ], - ]), - ) as unknown as OnyxCollection; - await waitForBatchedUpdates(); await measureFunction(() => SidebarUtils.getOrderedReportIDs(currentReportId, allReports, betas, policies, CONST.PRIORITY_MODE.DEFAULT, allReportActions, transactionViolations)); }); diff --git a/tests/utils/LHNTestUtils.tsx b/tests/utils/LHNTestUtils.tsx index 80f28002f975..a10dbbdc17f8 100644 --- a/tests/utils/LHNTestUtils.tsx +++ b/tests/utils/LHNTestUtils.tsx @@ -10,6 +10,7 @@ import {LocaleContextProvider} from '@components/LocaleContextProvider'; import OnyxProvider from '@components/OnyxProvider'; import {CurrentReportIDContextProvider} from '@components/withCurrentReportID'; import {EnvironmentProvider} from '@components/withEnvironment'; +import {OrderedReportListItemsContextProvider} from '@hooks/useOrderedReportListItems'; import DateUtils from '@libs/DateUtils'; import ReportActionItemSingle from '@pages/home/report/ReportActionItemSingle'; import SidebarLinksData from '@pages/home/sidebar/SidebarLinksData'; @@ -280,7 +281,7 @@ function getFakeAdvancedReportAction(actionName: ActionName = 'IOU', actor = 'em function MockedSidebarLinks({currentReportID = ''}: MockedSidebarLinksProps) { return ( - + {}} diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index 848ef8f81f47..e62d9769bc53 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -9,3 +9,24 @@ export default function createCollection(createKey: (item: T, index: number) return map; } + +export function createNestedCollection( + createParentKey: (item: T, index: number) => string | number, + createKey: (item: T, index: number) => string | number, + createItem: (index: number) => T, + length = 500, +): Record> { + const map: Record> = {}; + + for (let i = 0; i < length; i++) { + const item = createItem(i); + const itemKey = createKey(item, i); + const itemParentKey = createParentKey(item, i); + map[itemParentKey] = { + ...map[itemParentKey], + [itemKey]: item, + }; + } + + return map; +} From 92fae0c08c3c3ca517fb57313f95e4fd2e77abf0 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 29 Feb 2024 18:02:17 +0500 Subject: [PATCH 15/51] revert: move option item data calculation to the renderItem component --- .../LHNOptionsList/LHNOptionsList.tsx | 117 +++++++++++++++--- .../LHNOptionsList/OptionRowLHNData.tsx | 63 +++++++++- src/components/LHNOptionsList/types.ts | 74 +++++++++-- src/hooks/useOrderedReportListItems.tsx | 47 +------ 4 files changed, 226 insertions(+), 75 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 1884997a58fe..5569e53942aa 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -2,13 +2,18 @@ import {FlashList} from '@shopify/flash-list'; import type {ReactElement} from 'react'; import React, {memo, useCallback, useMemo} from 'react'; import {StyleSheet, View} from 'react-native'; +import {withOnyx} from 'react-native-onyx'; +import withCurrentReportID from '@components/withCurrentReportID'; +import usePermissions from '@hooks/usePermissions'; import useThemeStyles from '@hooks/useThemeStyles'; +import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import variables from '@styles/variables'; import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; import OptionRowLHNData from './OptionRowLHNData'; -import type {LHNOptionsListProps, OptionListItem, RenderItemProps} from './types'; +import type {LHNOptionsListOnyxProps, LHNOptionsListProps, RenderItemProps} from './types'; -const keyExtractor = (item: OptionListItem) => `report_${item.reportID}`; +const keyExtractor = (item: string) => `report_${item}`; function LHNOptionsList({ style, @@ -18,9 +23,18 @@ function LHNOptionsList({ shouldDisableFocusOptions = false, currentReportID = '', optionMode, + reports = {}, + reportActions = {}, + policy = {}, + preferredLocale = CONST.LOCALES.DEFAULT, + personalDetails = {}, + transactions = {}, + draftComments = {}, + transactionViolations = {}, onFirstItemRendered = () => {}, }: LHNOptionsListProps) { const styles = useThemeStyles(); + const {canUseViolations} = usePermissions(); // When the first item renders we want to call the onFirstItemRendered callback. // At this point in time we know that the list is actually displaying items. @@ -38,18 +52,64 @@ function LHNOptionsList({ * Function which renders a row in the list */ const renderItem = useCallback( - ({item}: RenderItemProps): ReactElement => ( - - ), - [shouldDisableFocusOptions, currentReportID, onSelectRow, onLayoutItem, optionMode], + ({item: reportID}: RenderItemProps): ReactElement => { + const itemFullReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? null; + const itemReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? null; + const itemParentReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null; + const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null; + const itemPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null; + const transactionID = itemParentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? itemParentReportAction.originalMessage.IOUTransactionID ?? '' : ''; + const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? null; + const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? ''; + const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions); + const lastReportAction = sortedReportActions[0]; + + // Get the transaction for the last report action + let lastReportActionTransactionID = ''; + + if (lastReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU) { + lastReportActionTransactionID = lastReportAction.originalMessage?.IOUTransactionID ?? ''; + } + const lastReportActionTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${lastReportActionTransactionID}`] ?? {}; + + return ( + + ); + }, + [ + currentReportID, + draftComments, + onSelectRow, + optionMode, + personalDetails, + policy, + preferredLocale, + reportActions, + reports, + shouldDisableFocusOptions, + transactions, + transactionViolations, + canUseViolations, + onLayoutItem, + ], ); const extraData = useMemo(() => [currentReportID], [currentReportID]); @@ -74,6 +134,33 @@ function LHNOptionsList({ LHNOptionsList.displayName = 'LHNOptionsList'; -export default memo(LHNOptionsList); +export default withCurrentReportID( + withOnyx({ + reports: { + key: ONYXKEYS.COLLECTION.REPORT, + }, + reportActions: { + key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, + }, + policy: { + key: ONYXKEYS.COLLECTION.POLICY, + }, + preferredLocale: { + key: ONYXKEYS.NVP_PREFERRED_LOCALE, + }, + personalDetails: { + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + }, + transactions: { + key: ONYXKEYS.COLLECTION.TRANSACTION, + }, + draftComments: { + key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, + }, + transactionViolations: { + key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, + }, + })(memo(LHNOptionsList)), +); export type {LHNOptionsListProps}; diff --git a/src/components/LHNOptionsList/OptionRowLHNData.tsx b/src/components/LHNOptionsList/OptionRowLHNData.tsx index bc1a06ce5f67..a18d5a8ec1ec 100644 --- a/src/components/LHNOptionsList/OptionRowLHNData.tsx +++ b/src/components/LHNOptionsList/OptionRowLHNData.tsx @@ -1,5 +1,10 @@ -import React, {useEffect} from 'react'; +import {deepEqual} from 'fast-equals'; +import React, {useEffect, useMemo, useRef} from 'react'; +import * as ReportUtils from '@libs/ReportUtils'; +import SidebarUtils from '@libs/SidebarUtils'; import * as Report from '@userActions/Report'; +import CONST from '@src/CONST'; +import type {OptionData} from '@src/libs/ReportUtils'; import OptionRowLHN from './OptionRowLHN'; import type {OptionRowLHNDataProps} from './types'; @@ -9,9 +14,63 @@ import type {OptionRowLHNDataProps} from './types'; * The OptionRowLHN component is memoized, so it will only * re-render if the data really changed. */ -function OptionRowLHNData({isFocused = false, comment, optionItem, ...propsToForward}: OptionRowLHNDataProps) { +function OptionRowLHNData({ + isFocused = false, + fullReport, + reportActions, + personalDetails = {}, + preferredLocale = CONST.LOCALES.DEFAULT, + comment, + policy, + receiptTransactions, + parentReportAction, + transaction, + lastReportActionTransaction = {}, + transactionViolations, + canUseViolations, + ...propsToForward +}: OptionRowLHNDataProps) { const reportID = propsToForward.reportID; + const optionItemRef = useRef(); + + const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(fullReport, transactionViolations, parentReportAction ?? null); + + const optionItem = useMemo(() => { + // Note: ideally we'd have this as a dependent selector in onyx! + const item = SidebarUtils.getOptionData({ + report: fullReport, + reportActions, + personalDetails, + preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT, + policy, + parentReportAction, + hasViolations: !!hasViolations, + }); + if (deepEqual(item, optionItemRef.current)) { + return optionItemRef.current; + } + + optionItemRef.current = item; + + return item; + // Listen parentReportAction to update title of thread report when parentReportAction changed + // Listen to transaction to update title of transaction report when transaction changed + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + fullReport, + lastReportActionTransaction, + reportActions, + personalDetails, + preferredLocale, + policy, + parentReportAction, + transaction, + transactionViolations, + canUseViolations, + receiptTransactions, + ]); + useEffect(() => { if (!optionItem || !!optionItem.hasDraftComment || !comment || comment.length <= 0 || isFocused) { return; diff --git a/src/components/LHNOptionsList/types.ts b/src/components/LHNOptionsList/types.ts index 3f72748ab8a8..e09e5cb6c8b5 100644 --- a/src/components/LHNOptionsList/types.ts +++ b/src/components/LHNOptionsList/types.ts @@ -1,22 +1,40 @@ import type {ContentStyle} from '@shopify/flash-list'; import type {RefObject} from 'react'; import type {LayoutChangeEvent, StyleProp, TextStyle, View, ViewStyle} from 'react-native'; +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; import type {CurrentReportIDContextValue} from '@components/withCurrentReportID'; import type CONST from '@src/CONST'; import type {OptionData} from '@src/libs/ReportUtils'; +import type {Locale, PersonalDetailsList, Policy, Report, ReportAction, ReportActions, Transaction, TransactionViolation} from '@src/types/onyx'; +import type {EmptyObject} from '@src/types/utils/EmptyObject'; type OptionMode = ValueOf; -type OptionListItem = { - /** The reportID of the report */ - reportID: string; +type LHNOptionsListOnyxProps = { + /** The policy which the user has access to and which the report could be tied to */ + policy: OnyxCollection; - /** The item that should be rendered */ - optionItem: OptionData | undefined; + /** All reports shared with the user */ + reports: OnyxCollection; - /** Comment added to report */ - comment: string; + /** Array of report actions for this report */ + reportActions: OnyxCollection; + + /** Indicates which locale the user currently has selected */ + preferredLocale: OnyxEntry; + + /** List of users' personal details */ + personalDetails: OnyxEntry; + + /** The transaction from the parent report action */ + transactions: OnyxCollection; + + /** List of draft comments */ + draftComments: OnyxCollection; + + /** The list of transaction violations */ + transactionViolations: OnyxCollection; }; type CustomLHNOptionsListProps = { @@ -27,7 +45,7 @@ type CustomLHNOptionsListProps = { contentContainerStyles?: StyleProp; /** Sections for the section list */ - data: OptionListItem[]; + data: string[]; /** Callback to fire when a row is selected */ onSelectRow?: (optionItem: OptionData, popoverAnchor: RefObject) => void; @@ -42,21 +60,51 @@ type CustomLHNOptionsListProps = { onFirstItemRendered: () => void; }; -type LHNOptionsListProps = CustomLHNOptionsListProps & CurrentReportIDContextValue; +type LHNOptionsListProps = CustomLHNOptionsListProps & CurrentReportIDContextValue & LHNOptionsListOnyxProps; type OptionRowLHNDataProps = { /** Whether row should be focused */ isFocused?: boolean; + /** List of users' personal details */ + personalDetails?: PersonalDetailsList; + + /** The preferred language for the app */ + preferredLocale?: OnyxEntry; + + /** The full data of the report */ + fullReport: OnyxEntry; + + /** The policy which the user has access to and which the report could be tied to */ + policy?: OnyxEntry; + + /** The action from the parent report */ + parentReportAction?: OnyxEntry; + + /** The transaction from the parent report action */ + transaction: OnyxEntry; + + /** The transaction linked to the report's last action */ + lastReportActionTransaction?: OnyxEntry; + /** Comment added to report */ comment: string; - /** The item that should be rendered */ - optionItem: OptionData | undefined; + /** The receipt transaction from the parent report action */ + receiptTransactions: OnyxCollection; /** The reportID of the report */ reportID: string; + /** Array of report actions for this report */ + reportActions: OnyxEntry; + + /** List of transaction violation */ + transactionViolations: OnyxCollection; + + /** Whether the user can use violations */ + canUseViolations: boolean | undefined; + /** Toggle between compact and default view */ viewMode?: OptionMode; @@ -89,6 +137,6 @@ type OptionRowLHNProps = { onLayout?: (event: LayoutChangeEvent) => void; }; -type RenderItemProps = {item: OptionListItem}; +type RenderItemProps = {item: string}; -export type {LHNOptionsListProps, OptionRowLHNDataProps, OptionRowLHNProps, OptionListItem, RenderItemProps}; +export type {LHNOptionsListProps, OptionRowLHNDataProps, OptionRowLHNProps, LHNOptionsListOnyxProps, RenderItemProps}; diff --git a/src/hooks/useOrderedReportListItems.tsx b/src/hooks/useOrderedReportListItems.tsx index d368abd897af..f9aaeaf400c1 100644 --- a/src/hooks/useOrderedReportListItems.tsx +++ b/src/hooks/useOrderedReportListItems.tsx @@ -1,17 +1,14 @@ import React, {createContext, useCallback, useContext, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; -import {usePersonalDetails, useReport} from '@components/OnyxProvider'; import {getCurrentUserAccountID} from '@libs/actions/Report'; import {getPolicyMembersByIdWithoutCurrentUser} 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 {Beta, Locale, Policy, PolicyMembers, PriorityMode, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; +import type {Beta, Policy, PolicyMembers, PriorityMode, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; import useActiveWorkspace from './useActiveWorkspace'; import useCurrentReportID from './useCurrentReportID'; -import usePermissions from './usePermissions'; type OnyxProps = { chatReports: OnyxCollection; @@ -21,8 +18,6 @@ type OnyxProps = { transactionViolations: OnyxCollection; policyMembers: OnyxCollection; priorityMode: OnyxEntry; - preferredLocale: OnyxEntry; - draftComments: OnyxCollection; }; type WithOrderedReportListItemsContextProviderProps = OnyxProps & { @@ -40,12 +35,8 @@ function WithOrderedReportListItemsContextProvider({ transactionViolations, policyMembers, priorityMode, - preferredLocale, - draftComments, }: WithOrderedReportListItemsContextProviderProps) { const currentReportIDValue = useCurrentReportID(); - const personalDetails = usePersonalDetails(); - const {canUseViolations} = usePermissions(); const {activeWorkspaceID} = useActiveWorkspace(); const policyMemberAccountIDs = useMemo(() => getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, getCurrentUserAccountID()), [activeWorkspaceID, policyMembers]); @@ -80,34 +71,7 @@ function WithOrderedReportListItemsContextProvider({ return orderedReportIDs; }, [getOrderedReportIDs, currentReportIDValue?.currentReportID, orderedReportIDs]); - const orderedReportListItems = useMemo( - () => - orderedReportIDsWithCurrentReport.map((reportID) => { - const itemFullReport = chatReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? null; - const itemReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? null; - const itemParentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null; - const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null; - const itemPolicy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null; - const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? ''; - - const hasViolations = canUseViolations && ReportUtils.doesTransactionThreadHaveViolations(itemFullReport, transactionViolations, itemParentReportAction ?? null); - - const item = SidebarUtils.getOptionData({ - report: itemFullReport, - reportActions: itemReportActions, - personalDetails, - preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT, - policy: itemPolicy, - parentReportAction: itemParentReportAction, - hasViolations: !!hasViolations, - }); - - return {reportID, optionItem: item, comment: itemComment}; - }), - [orderedReportIDsWithCurrentReport, canUseViolations, personalDetails, draftComments, preferredLocale, chatReports, allReportActions, policies, transactionViolations], - ); - - return {children}; + return {children}; } const OrderedReportListItemsContextProvider = withOnyx({ @@ -138,13 +102,6 @@ const OrderedReportListItemsContextProvider = withOnyx Date: Thu, 29 Feb 2024 19:48:44 +0500 Subject: [PATCH 16/51] fix: linting --- src/hooks/useOrderedReportListItems.tsx | 2 +- src/pages/home/sidebar/SidebarLinks.js | 2 +- tests/utils/collections/createCollection.ts | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/hooks/useOrderedReportListItems.tsx b/src/hooks/useOrderedReportListItems.tsx index f9aaeaf400c1..bcd22320d1ca 100644 --- a/src/hooks/useOrderedReportListItems.tsx +++ b/src/hooks/useOrderedReportListItems.tsx @@ -6,7 +6,7 @@ import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import SidebarUtils from '@libs/SidebarUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Beta, Policy, PolicyMembers, PriorityMode, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; +import type {Beta, Policy, PolicyMembers, PriorityMode, Report, ReportActions, TransactionViolation} from '@src/types/onyx'; import useActiveWorkspace from './useActiveWorkspace'; import useCurrentReportID from './useCurrentReportID'; diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 4c9803e257bc..8cda70d2486a 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -33,7 +33,7 @@ const basePropTypes = { const propTypes = { ...basePropTypes, - optionListItems: PropTypes.arrayOf(PropTypes.object).isRequired, + optionListItems: PropTypes.arrayOf(PropTypes.string).isRequired, isLoading: PropTypes.bool.isRequired, diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index e62d9769bc53..bcc37c301279 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -10,7 +10,7 @@ export default function createCollection(createKey: (item: T, index: number) return map; } -export function createNestedCollection( +function createNestedCollection( createParentKey: (item: T, index: number) => string | number, createKey: (item: T, index: number) => string | number, createItem: (index: number) => T, @@ -30,3 +30,7 @@ export function createNestedCollection( return map; } + +export { + createNestedCollection, +} \ No newline at end of file From e735658f12826766791c2bfa1d3bb7b82b4559d6 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 1 Mar 2024 13:30:58 +0500 Subject: [PATCH 17/51] test: fix failing test --- .../LHNOptionsList/LHNOptionsList.tsx | 2 +- src/components/withCurrentReportID.tsx | 2 +- src/hooks/useOrderedReportListItems.tsx | 18 +++++++-- tests/unit/SidebarOrderTest.ts | 6 +-- tests/utils/LHNTestUtils.tsx | 37 ++++++++++++------- 5 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 5569e53942aa..d5b422122144 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -112,7 +112,7 @@ function LHNOptionsList({ ], ); - const extraData = useMemo(() => [currentReportID], [currentReportID]); + const extraData = useMemo(() => [reportActions, reports, policy, personalDetails], [reportActions, reports, policy, personalDetails]); return ( diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index d5c5a6896a9c..0d052f759f81 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -45,7 +45,7 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro * switching between chat list and settings->workspaces tab. * and doing so avoid unnecessary re-render of `useOrderedReportListItems`. */ - if (reportID) { + if (reportID !== undefined) { setCurrentReportID(reportID); } }, diff --git a/src/hooks/useOrderedReportListItems.tsx b/src/hooks/useOrderedReportListItems.tsx index bcd22320d1ca..b160c372679a 100644 --- a/src/hooks/useOrderedReportListItems.tsx +++ b/src/hooks/useOrderedReportListItems.tsx @@ -22,6 +22,7 @@ type OnyxProps = { type WithOrderedReportListItemsContextProviderProps = OnyxProps & { children: React.ReactNode; + currentReportIDForTests?: string; }; const OrderedReportListItemsContext = createContext({}); @@ -35,8 +36,19 @@ function WithOrderedReportListItemsContextProvider({ transactionViolations, policyMembers, priorityMode, + /** + * Only required to make unit tests work, since we + * explicitly pass the currentReportID in LHNTestUtils + * to SidebarLinksData, so this context doesn't have an + * access to currentReportID in that case. + * + * So this is a work around to have currentReportID available + * only in testing environment. + */ + currentReportIDForTests, }: WithOrderedReportListItemsContextProviderProps) { const currentReportIDValue = useCurrentReportID(); + const derivedCurrentReportID = currentReportIDForTests ?? currentReportIDValue?.currentReportID; const {activeWorkspaceID} = useActiveWorkspace(); const policyMemberAccountIDs = useMemo(() => getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, getCurrentUserAccountID()), [activeWorkspaceID, policyMembers]); @@ -65,11 +77,11 @@ function WithOrderedReportListItemsContextProvider({ // 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. const orderedReportIDsWithCurrentReport = useMemo(() => { - if (currentReportIDValue?.currentReportID && !orderedReportIDs.includes(currentReportIDValue.currentReportID)) { - return getOrderedReportIDs(currentReportIDValue.currentReportID); + if (derivedCurrentReportID && !orderedReportIDs.includes(derivedCurrentReportID)) { + return getOrderedReportIDs(derivedCurrentReportID); } return orderedReportIDs; - }, [getOrderedReportIDs, currentReportIDValue?.currentReportID, orderedReportIDs]); + }, [getOrderedReportIDs, derivedCurrentReportID, orderedReportIDs]); return {children}; } diff --git a/tests/unit/SidebarOrderTest.ts b/tests/unit/SidebarOrderTest.ts index 27da8348f43d..10d30e4c6dc8 100644 --- a/tests/unit/SidebarOrderTest.ts +++ b/tests/unit/SidebarOrderTest.ts @@ -298,7 +298,7 @@ describe('Sidebar', () => { Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, - [ONYXKEYS.IS_LOADING_REPORT_DATA]: false, + [ONYXKEYS.IS_LOADING_APP]: false, ...reportCollectionDataSet, }), ) @@ -362,7 +362,7 @@ describe('Sidebar', () => { Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, - [ONYXKEYS.IS_LOADING_REPORT_DATA]: false, + [ONYXKEYS.IS_LOADING_APP]: false, ...reportCollectionDataSet, }), ) @@ -429,7 +429,7 @@ describe('Sidebar', () => { Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails, - [ONYXKEYS.IS_LOADING_REPORT_DATA]: false, + [ONYXKEYS.IS_LOADING_APP]: false, [`${ONYXKEYS.COLLECTION.POLICY}${fakeReport.policyID}`]: fakePolicy, ...reportCollectionDataSet, }), diff --git a/tests/utils/LHNTestUtils.tsx b/tests/utils/LHNTestUtils.tsx index a10dbbdc17f8..c8c3f145d951 100644 --- a/tests/utils/LHNTestUtils.tsx +++ b/tests/utils/LHNTestUtils.tsx @@ -281,19 +281,30 @@ function getFakeAdvancedReportAction(actionName: ActionName = 'IOU', actor = 'em function MockedSidebarLinks({currentReportID = ''}: MockedSidebarLinksProps) { return ( - - {}} - insets={{ - top: 0, - left: 0, - right: 0, - bottom: 0, - }} - isSmallScreenWidth={false} - currentReportID={currentReportID} - /> + + {/* + * Only required to make unit tests work, since we + * explicitly pass the currentReportID in LHNTestUtils + * to SidebarLinksData, so this context doesn't have an + * access to currentReportID in that case. + * + * So this is a work around to have currentReportID available + * only in testing environment. + * */} + + {}} + insets={{ + top: 0, + left: 0, + right: 0, + bottom: 0, + }} + isSmallScreenWidth={false} + currentReportID={currentReportID} + /> + ); } From 94fcc48f89f95ba50fc22a20ac04183dbd25cb8a Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 1 Mar 2024 13:31:12 +0500 Subject: [PATCH 18/51] fix: prettier --- tests/utils/collections/createCollection.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index bcc37c301279..4a2a1fa4eb78 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -31,6 +31,4 @@ function createNestedCollection( return map; } -export { - createNestedCollection, -} \ No newline at end of file +export {createNestedCollection}; From e0673e0baf75e6610c39c234cd758e8958b6157c Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 5 Mar 2024 14:50:54 +0500 Subject: [PATCH 19/51] refactor: rename to useReportIDs --- src/App.tsx | 4 +- ...edReportListItems.tsx => useReportIDs.tsx} | 66 +++++++++++-------- src/pages/home/sidebar/SidebarLinksData.js | 4 +- tests/utils/LHNTestUtils.tsx | 6 +- 4 files changed, 46 insertions(+), 34 deletions(-) rename src/hooks/{useOrderedReportListItems.tsx => useReportIDs.tsx} (72%) diff --git a/src/App.tsx b/src/App.tsx index 3a294757e149..6cfc2587074b 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -29,7 +29,7 @@ import {KeyboardStateProvider} from './components/withKeyboardState'; import {WindowDimensionsProvider} from './components/withWindowDimensions'; import Expensify from './Expensify'; import useDefaultDragAndDrop from './hooks/useDefaultDragAndDrop'; -import {OrderedReportListItemsContextProvider} from './hooks/useOrderedReportListItems'; +import {ReportIDsContextProvider} from './hooks/useReportIDs'; import OnyxUpdateManager from './libs/actions/OnyxUpdateManager'; import InitialUrlContext from './libs/InitialUrlContext'; import {ReportAttachmentsProvider} from './pages/home/report/ReportAttachmentsContext'; @@ -76,7 +76,7 @@ function App({url}: AppProps) { CustomStatusBarAndBackgroundContextProvider, ActiveElementRoleProvider, ActiveWorkspaceContextProvider, - OrderedReportListItemsContextProvider, + ReportIDsContextProvider, PlaybackContextProvider, VolumeContextProvider, VideoPopoverMenuContextProvider, diff --git a/src/hooks/useOrderedReportListItems.tsx b/src/hooks/useReportIDs.tsx similarity index 72% rename from src/hooks/useOrderedReportListItems.tsx rename to src/hooks/useReportIDs.tsx index 4580df82ed34..651e21f08b24 100644 --- a/src/hooks/useOrderedReportListItems.tsx +++ b/src/hooks/useReportIDs.tsx @@ -1,7 +1,7 @@ +import _ from 'lodash'; import React, {createContext, useCallback, useContext, useMemo} from 'react'; import {withOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; -import _ from 'underscore'; import {getCurrentUserAccountID} from '@libs/actions/Report'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; @@ -9,6 +9,7 @@ import SidebarUtils from '@libs/SidebarUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Beta, Policy, PolicyMembers, PriorityMode, Report, ReportActions, Transaction, TransactionViolation} from '@src/types/onyx'; +import type * as OnyxCommon from '@src/types/onyx/OnyxCommon'; import useActiveWorkspace from './useActiveWorkspace'; import useCurrentReportID from './useCurrentReportID'; import usePermissions from './usePermissions'; @@ -24,14 +25,22 @@ type OnyxProps = { allTransactions: OnyxCollection; }; -type WithOrderedReportListItemsContextProviderProps = OnyxProps & { +type WithReportIDsContextProviderProps = OnyxProps & { children: React.ReactNode; currentReportIDForTests?: string; }; -const OrderedReportListItemsContext = createContext({}); +type ReportIDsContextValue = { + orderedReportIDs: string[]; + reportIDsWithErrors: Record; +}; + +const ReportIDsContext = createContext({ + orderedReportIDs: [], + reportIDsWithErrors: {}, +}); -function WithOrderedReportListItemsContextProvider({ +function WithReportIDsContextProvider({ children, chatReports, betas, @@ -51,7 +60,7 @@ function WithOrderedReportListItemsContextProvider({ * only in testing environment. */ currentReportIDForTests, -}: WithOrderedReportListItemsContextProviderProps) { +}: WithReportIDsContextProviderProps) { const currentReportIDValue = useCurrentReportID(); const derivedCurrentReportID = currentReportIDForTests ?? currentReportIDValue?.currentReportID; const {activeWorkspaceID} = useActiveWorkspace(); @@ -59,22 +68,25 @@ function WithOrderedReportListItemsContextProvider({ const policyMemberAccountIDs = useMemo(() => getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, getCurrentUserAccountID()), [activeWorkspaceID, policyMembers]); - const chatReportsKeys = useMemo(() => _.keys(chatReports), [chatReports]); - const reportIDsWithErrors = useMemo(() => { - return _.reduce( - chatReportsKeys, - (errorsMap, reportKey) => { - const report = chatReports && chatReports[reportKey]; - const allReportsActions = allReportActions && allReportActions[reportKey.replace(ONYXKEYS.COLLECTION.REPORT, ONYXKEYS.COLLECTION.REPORT_ACTIONS)]; - const errors = OptionsListUtils.getAllReportErrors(report, allReportsActions, allTransactions) || {}; - if (_.size(errors) === 0) { - return errorsMap; - } - return {...errorsMap, [reportKey.replace(ONYXKEYS.COLLECTION.REPORT, '')]: errors}; - }, - {}, - ); - }, [chatReportsKeys, allReportActions, allTransactions, chatReports]); + const chatReportsKeys = useMemo(() => Object.keys(chatReports ?? {}), [chatReports]); + // eslint-disable-next-line you-dont-need-lodash-underscore/reduce + const reportIDsWithErrors = useMemo( + () => + _.reduce( + chatReportsKeys, + (errorsMap, reportKey) => { + const report = chatReports?.[reportKey] ?? null; + const allReportsActions = allReportActions?.[reportKey.replace(ONYXKEYS.COLLECTION.REPORT, ONYXKEYS.COLLECTION.REPORT_ACTIONS)] ?? null; + const errors = OptionsListUtils.getAllReportErrors(report, allReportsActions, allTransactions) || {}; + if (Object.values(errors).length === 0) { + return errorsMap; + } + return {...errorsMap, [reportKey.replace(ONYXKEYS.COLLECTION.REPORT, '')]: errors}; + }, + {}, + ), + [chatReportsKeys, allReportActions, allTransactions, chatReports], + ); const getOrderedReportIDs = useCallback( (currentReportID?: string) => @@ -116,10 +128,10 @@ function WithOrderedReportListItemsContextProvider({ [orderedReportIDsWithCurrentReport, reportIDsWithErrors], ); - return {children}; + return {children}; } -const OrderedReportListItemsContextProvider = withOnyx({ +const ReportIDsContextProvider = withOnyx({ chatReports: { key: ONYXKEYS.COLLECTION.REPORT, initialValue: {}, @@ -151,10 +163,10 @@ const OrderedReportListItemsContextProvider = withOnyx { if (deepEqual(orderedReportIDsRef.current, orderedReportIDs)) { diff --git a/tests/utils/LHNTestUtils.tsx b/tests/utils/LHNTestUtils.tsx index c8c3f145d951..04344ba71184 100644 --- a/tests/utils/LHNTestUtils.tsx +++ b/tests/utils/LHNTestUtils.tsx @@ -10,7 +10,7 @@ import {LocaleContextProvider} from '@components/LocaleContextProvider'; import OnyxProvider from '@components/OnyxProvider'; import {CurrentReportIDContextProvider} from '@components/withCurrentReportID'; import {EnvironmentProvider} from '@components/withEnvironment'; -import {OrderedReportListItemsContextProvider} from '@hooks/useOrderedReportListItems'; +import {ReportIDsContextProvider} from '@hooks/useReportIDs'; import DateUtils from '@libs/DateUtils'; import ReportActionItemSingle from '@pages/home/report/ReportActionItemSingle'; import SidebarLinksData from '@pages/home/sidebar/SidebarLinksData'; @@ -291,7 +291,7 @@ function MockedSidebarLinks({currentReportID = ''}: MockedSidebarLinksProps) { * So this is a work around to have currentReportID available * only in testing environment. * */} - + {}} @@ -304,7 +304,7 @@ function MockedSidebarLinks({currentReportID = ''}: MockedSidebarLinksProps) { isSmallScreenWidth={false} currentReportID={currentReportID} /> - + ); } From 30ca3030ee04390772e1a03bc69305fbf4160600 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 5 Mar 2024 14:51:43 +0500 Subject: [PATCH 20/51] refactor: don't set currentReportID if it's on workspaces screen --- src/components/withCurrentReportID.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index 0d052f759f81..54cdc84f127a 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -4,6 +4,7 @@ import type {ComponentType, ForwardedRef, RefAttributes} from 'react'; import React, {createContext, forwardRef, useCallback, useMemo, useState} from 'react'; import getComponentDisplayName from '@libs/getComponentDisplayName'; import Navigation from '@libs/Navigation/Navigation'; +import SCREENS from '@src/SCREENS'; type CurrentReportIDContextValue = { updateCurrentReportID: (state: NavigationState) => void; @@ -40,14 +41,17 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro const updateCurrentReportID = useCallback( (state: NavigationState) => { const reportID = Navigation.getTopmostReportId(state) ?? ''; + /** * This is to make sure we don't set the undefined as reportID when * switching between chat list and settings->workspaces tab. - * and doing so avoid unnecessary re-render of `useOrderedReportListItems`. + * and doing so avoid unnecessary re-render of `useReportIDs`. */ - if (reportID !== undefined) { - setCurrentReportID(reportID); + const params = state.routes[state.index].params; + if (params && 'screen' in params && params.screen === SCREENS.SETTINGS.WORKSPACES) { + return; } + setCurrentReportID(reportID); }, [setCurrentReportID], ); From 121b5378606b353ecc014df082c140dfe5737d4f Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 5 Mar 2024 14:51:53 +0500 Subject: [PATCH 21/51] refactor: remove dead code --- tests/perf-test/SidebarUtils.perf-test.ts | 8 +------ tests/utils/collections/createCollection.ts | 23 --------------------- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index 8672d7c0cafe..2b2bdbc6b57a 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -10,7 +10,7 @@ import type Policy from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; import type ReportAction from '@src/types/onyx/ReportAction'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; -import createCollection, {createNestedCollection} from '../utils/collections/createCollection'; +import createCollection from '../utils/collections/createCollection'; import createPersonalDetails from '../utils/collections/personalDetails'; import createRandomPolicy from '../utils/collections/policies'; import createRandomReportAction from '../utils/collections/reportActions'; @@ -29,12 +29,6 @@ const reportActions = createCollection( (index) => createRandomReportAction(index), ); -// const allReportActions = createNestedCollection( -// (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, -// (item) => `${item.reportActionID}`, -// (index) => createRandomReportAction(index), -// ); - const personalDetails = createCollection( (item) => item.accountID, (index) => createPersonalDetails(index), diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index 4a2a1fa4eb78..848ef8f81f47 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -9,26 +9,3 @@ export default function createCollection(createKey: (item: T, index: number) return map; } - -function createNestedCollection( - createParentKey: (item: T, index: number) => string | number, - createKey: (item: T, index: number) => string | number, - createItem: (index: number) => T, - length = 500, -): Record> { - const map: Record> = {}; - - for (let i = 0; i < length; i++) { - const item = createItem(i); - const itemKey = createKey(item, i); - const itemParentKey = createParentKey(item, i); - map[itemParentKey] = { - ...map[itemParentKey], - [itemKey]: item, - }; - } - - return map; -} - -export {createNestedCollection}; From 9e41603f9da5a3eb38873a8ab24476c2eadacdaf Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 5 Mar 2024 15:13:57 +0500 Subject: [PATCH 22/51] fix: linting --- src/hooks/useReportIDs.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 651e21f08b24..2e7c63182646 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -67,11 +67,10 @@ function WithReportIDsContextProvider({ const {canUseViolations} = usePermissions(); const policyMemberAccountIDs = useMemo(() => getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, getCurrentUserAccountID()), [activeWorkspaceID, policyMembers]); - const chatReportsKeys = useMemo(() => Object.keys(chatReports ?? {}), [chatReports]); - // eslint-disable-next-line you-dont-need-lodash-underscore/reduce const reportIDsWithErrors = useMemo( () => + // eslint-disable-next-line you-dont-need-lodash-underscore/reduce _.reduce( chatReportsKeys, (errorsMap, reportKey) => { From 4a3284161b02ea616cc40a1bc741221672e6e777 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 5 Mar 2024 15:17:33 +0500 Subject: [PATCH 23/51] refactor: remove irrelevant changes --- src/components/LHNOptionsList/LHNOptionsList.tsx | 4 ++-- src/components/LHNOptionsList/types.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index 31af022a12aa..be8ce677b641 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -20,15 +20,15 @@ function LHNOptionsList({ contentContainerStyles, data, onSelectRow, - shouldDisableFocusOptions = false, - currentReportID = '', optionMode, + shouldDisableFocusOptions = false, reports = {}, reportActions = {}, policy = {}, preferredLocale = CONST.LOCALES.DEFAULT, personalDetails = {}, transactions = {}, + currentReportID = '', draftComments = {}, transactionViolations = {}, onFirstItemRendered = () => {}, diff --git a/src/components/LHNOptionsList/types.ts b/src/components/LHNOptionsList/types.ts index 0d1bda775255..c122ab018392 100644 --- a/src/components/LHNOptionsList/types.ts +++ b/src/components/LHNOptionsList/types.ts @@ -139,7 +139,7 @@ type OptionRowLHNProps = { style?: StyleProp; /** The item that should be rendered */ - optionItem: OptionData | undefined; + optionItem?: OptionData; onLayout?: (event: LayoutChangeEvent) => void; }; From 581d09d7a3dec1f1c301c7dccfba4877e60cb7c4 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Mon, 11 Mar 2024 11:33:46 +0500 Subject: [PATCH 24/51] fix: comments --- src/components/withCurrentReportID.tsx | 2 +- src/hooks/useReportIDs.tsx | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index 54cdc84f127a..bb3283a21d25 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -45,7 +45,7 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro /** * This is to make sure we don't set the undefined as reportID when * switching between chat list and settings->workspaces tab. - * and doing so avoid unnecessary re-render of `useReportIDs`. + * and doing so avoids an unnecessary re-render of `useReportIDs`. */ const params = state.routes[state.index].params; if (params && 'screen' in params && params.screen === SCREENS.SETTINGS.WORKSPACES) { diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 2e7c63182646..547975ae1cd0 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -56,8 +56,7 @@ function WithReportIDsContextProvider({ * to SidebarLinksData, so this context doesn't have an * access to currentReportID in that case. * - * So this is a work around to have currentReportID available - * only in testing environment. + * This is a workaround to have currentReportID available in testing environment. */ currentReportIDForTests, }: WithReportIDsContextProviderProps) { @@ -109,7 +108,7 @@ function WithReportIDsContextProvider({ // 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 here we check if + // 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. const orderedReportIDsWithCurrentReport = useMemo(() => { From fffe41ac7ed346c542da74cbe7952e581be0eb1c Mon Sep 17 00:00:00 2001 From: hurali97 Date: Mon, 11 Mar 2024 11:58:40 +0500 Subject: [PATCH 25/51] fix: safely check the nested properties --- src/components/withCurrentReportID.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index bb3283a21d25..22f68de9f57a 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -47,7 +47,7 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro * switching between chat list and settings->workspaces tab. * and doing so avoids an unnecessary re-render of `useReportIDs`. */ - const params = state.routes[state.index].params; + const params = state?.routes?.[state.index]?.params; if (params && 'screen' in params && params.screen === SCREENS.SETTINGS.WORKSPACES) { return; } From 6b41bf7819af297e0c3b7b32ced187b119c68193 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Mon, 11 Mar 2024 12:11:17 +0500 Subject: [PATCH 26/51] feat: add comments for extraData prop --- src/components/LHNOptionsList/LHNOptionsList.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index be8ce677b641..d141a5bbb3f4 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -129,6 +129,12 @@ function LHNOptionsList({ keyExtractor={keyExtractor} renderItem={renderItem} estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight} + // Previously, we were passing `extraData={[currentReportID]}`, which upon every render, was causing the + // re-render because of the new array reference. FlashList's children actually don't depend on the + // `currentReportID` prop but they depend on the `reportActions`, `reports`, `policy`, `personalDetails`. + // Previously it was working for us because of the new array reference. Even if you only pass an empty + // array, it will still work because of the new reference. But it's better to pass the actual dependencies + // to avoid unnecessary re-renders. extraData={extraData} showsVerticalScrollIndicator={false} /> From 64127110f19b7df5f74d3a6cb207303fd190cd4d Mon Sep 17 00:00:00 2001 From: hurali97 Date: Wed, 13 Mar 2024 16:44:48 +0500 Subject: [PATCH 27/51] fix: reassure tests --- tests/perf-test/SidebarUtils.perf-test.ts | 25 +++++---------------- tests/utils/collections/createCollection.ts | 25 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index 8566abb97c7f..a30c298f7471 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -9,7 +9,7 @@ import type {PersonalDetails, TransactionViolation} from '@src/types/onyx'; import type Policy from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; import type ReportAction from '@src/types/onyx/ReportAction'; -import createCollection from '../utils/collections/createCollection'; +import createCollection, {createNestedCollection} from '../utils/collections/createCollection'; import createPersonalDetails from '../utils/collections/personalDetails'; import createRandomPolicy from '../utils/collections/policies'; import createRandomReportAction, {getRandomDate} from '../utils/collections/reportActions'; @@ -51,24 +51,11 @@ const policies = createCollection( const mockedBetas = Object.values(CONST.BETAS); -const allReportActions = Object.fromEntries( - Object.keys(reportActions).map((key) => [ - `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${key}`, - [ - { - errors: reportActions[key].errors ?? [], - message: [ - { - moderationDecision: { - decision: reportActions[key].message?.[0]?.moderationDecision?.decision, - }, - }, - ], - reportActionID: reportActions[key].reportActionID, - }, - ], - ]), -) as unknown as OnyxCollection; +const allReportActions = createNestedCollection( + (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, + (item) => `${item.reportActionID}`, + (index) => createRandomReportAction(index), +); const currentReportId = '1'; const transactionViolations = {} as OnyxCollection; diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index 848ef8f81f47..ddb0b68742a4 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -9,3 +9,28 @@ export default function createCollection(createKey: (item: T, index: number) return map; } + +function createNestedCollection( + createParentKey: (item: T, index: number) => string | number, + createKey: (item: T, index: number) => string | number, + createItem: (index: number) => T, + length = 500, +): Record> { + const map: Record> = {}; + + for (let i = 0; i < length; i++) { + const item = createItem(i); + const itemKey = createKey(item, i); + const itemParentKey = createParentKey(item, i); + map[itemParentKey] = { + ...map[itemParentKey], + [itemKey]: item, + }; + } + + return map; +} + +export { + createNestedCollection, +}; \ No newline at end of file From e3477b2e751fe03702029de13aa8cee9f0ef68c6 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Wed, 13 Mar 2024 16:51:56 +0500 Subject: [PATCH 28/51] fix: apply prettier --- tests/utils/collections/createCollection.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index ddb0b68742a4..4a2a1fa4eb78 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -31,6 +31,4 @@ function createNestedCollection( return map; } -export { - createNestedCollection, -}; \ No newline at end of file +export {createNestedCollection}; From 30b319f5e5b6ec940e9101bf11fcb5bfc28eef77 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 14 Mar 2024 13:41:19 +0500 Subject: [PATCH 29/51] perf: check for the settings tab existence in screen params and early return --- src/components/withCurrentReportID.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index 22f68de9f57a..55b542ccacb7 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -4,7 +4,6 @@ import type {ComponentType, ForwardedRef, RefAttributes} from 'react'; import React, {createContext, forwardRef, useCallback, useMemo, useState} from 'react'; import getComponentDisplayName from '@libs/getComponentDisplayName'; import Navigation from '@libs/Navigation/Navigation'; -import SCREENS from '@src/SCREENS'; type CurrentReportIDContextValue = { updateCurrentReportID: (state: NavigationState) => void; @@ -44,11 +43,14 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro /** * This is to make sure we don't set the undefined as reportID when - * switching between chat list and settings->workspaces tab. - * and doing so avoids an unnecessary re-render of `useReportIDs`. + * switching between chat list and settings tab. The settings tab + * includes multiple screens and we don't want to set the reportID + * to falsy value when switching between them. + * + * Doing so avoids an unnecessary re-render of `useReportIDs`. */ const params = state?.routes?.[state.index]?.params; - if (params && 'screen' in params && params.screen === SCREENS.SETTINGS.WORKSPACES) { + if (params && 'screen' in params && typeof params.screen === 'string' && params.screen.indexOf('Settings_') !== -1) { return; } setCurrentReportID(reportID); From 8dba89bbcad90b6e13b9f55732376d8fe24fee4a Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 15 Mar 2024 13:29:59 +0500 Subject: [PATCH 30/51] feat: add selector from SidebarLinksData --- src/hooks/useReportIDs.tsx | 114 +++++++++++++++++++++++++++++++++---- 1 file changed, 103 insertions(+), 11 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 918a459e300a..f9b59be8e14c 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -3,21 +3,37 @@ import {withOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {getCurrentUserAccountID} from '@libs/actions/Report'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; +import * as ReportUtils from '@libs/ReportUtils'; import SidebarUtils from '@libs/SidebarUtils'; +import type {PolicySelector} from '@pages/home/sidebar/SidebarLinksData'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Beta, Policy, PolicyMembers, PriorityMode, Report, ReportActions, TransactionViolation} from '@src/types/onyx'; +import type * as OnyxTypes from '@src/types/onyx'; +import type {Message} from '@src/types/onyx/ReportAction'; import useActiveWorkspace from './useActiveWorkspace'; import useCurrentReportID from './useCurrentReportID'; type OnyxProps = { - chatReports: OnyxCollection; - betas: OnyxEntry; - policies: OnyxCollection; - allReportActions: OnyxCollection; - transactionViolations: OnyxCollection; - policyMembers: OnyxCollection; - priorityMode: PriorityMode; + /** List of reports */ + chatReports: OnyxCollection; + + /** Beta features list */ + betas: OnyxEntry; + + /** The policies which the user has access to */ + policies: OnyxCollection; + + /** All report actions for all reports */ + allReportActions: OnyxCollection; + + /** All of the transaction violations */ + transactionViolations: OnyxCollection; + + /** All policy members */ + policyMembers: OnyxCollection; + + /** The chat priority mode */ + priorityMode: OnyxTypes.PriorityMode; }; type WithReportIDsContextProviderProps = OnyxProps & { @@ -63,10 +79,10 @@ function WithReportIDsContextProvider({ SidebarUtils.getOrderedReportIDs( currentReportID ?? null, chatReports, - betas ?? [], - policies, + betas, + policies as OnyxCollection, priorityMode, - allReportActions, + allReportActions as OnyxCollection, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, @@ -98,10 +114,84 @@ function WithReportIDsContextProvider({ return {children}; } +type ChatReportSelector = OnyxTypes.Report & {isUnreadWithMention: boolean}; +type ReportActionsSelector = Array>; + +/** + * 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): ChatReportSelector => + (report && { + reportID: report.reportID, + participantAccountIDs: report.participantAccountIDs, + hasDraft: report.hasDraft, + 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): 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): PolicySelector => + (policy && { + type: policy.type, + name: policy.name, + avatar: policy.avatar, + }) as PolicySelector; + const ReportIDsContextProvider = withOnyx({ chatReports: { key: ONYXKEYS.COLLECTION.REPORT, initialValue: {}, + selector: chatReportSelector, }, priorityMode: { key: ONYXKEYS.NVP_PRIORITY_MODE, @@ -114,10 +204,12 @@ const ReportIDsContextProvider = withOnyx Date: Fri, 15 Mar 2024 18:40:10 +0500 Subject: [PATCH 31/51] fix: reassure tests --- tests/perf-test/SidebarUtils.perf-test.ts | 26 +++++---------------- tests/utils/collections/createCollection.ts | 23 ++++++++++++++++++ 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index 8566abb97c7f..4bd04e823f63 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -9,7 +9,7 @@ import type {PersonalDetails, TransactionViolation} from '@src/types/onyx'; import type Policy from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; import type ReportAction from '@src/types/onyx/ReportAction'; -import createCollection from '../utils/collections/createCollection'; +import createCollection, {createNestedCollection} from '../utils/collections/createCollection'; import createPersonalDetails from '../utils/collections/personalDetails'; import createRandomPolicy from '../utils/collections/policies'; import createRandomReportAction, {getRandomDate} from '../utils/collections/reportActions'; @@ -51,25 +51,11 @@ const policies = createCollection( const mockedBetas = Object.values(CONST.BETAS); -const allReportActions = Object.fromEntries( - Object.keys(reportActions).map((key) => [ - `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${key}`, - [ - { - errors: reportActions[key].errors ?? [], - message: [ - { - moderationDecision: { - decision: reportActions[key].message?.[0]?.moderationDecision?.decision, - }, - }, - ], - reportActionID: reportActions[key].reportActionID, - }, - ], - ]), -) as unknown as OnyxCollection; - +const allReportActions = createNestedCollection( + (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, + (item) => `${item.reportActionID}`, + (index) => createRandomReportAction(index), +); const currentReportId = '1'; const transactionViolations = {} as OnyxCollection; diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index 848ef8f81f47..4a2a1fa4eb78 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -9,3 +9,26 @@ export default function createCollection(createKey: (item: T, index: number) return map; } + +function createNestedCollection( + createParentKey: (item: T, index: number) => string | number, + createKey: (item: T, index: number) => string | number, + createItem: (index: number) => T, + length = 500, +): Record> { + const map: Record> = {}; + + for (let i = 0; i < length; i++) { + const item = createItem(i); + const itemKey = createKey(item, i); + const itemParentKey = createParentKey(item, i); + map[itemParentKey] = { + ...map[itemParentKey], + [itemKey]: item, + }; + } + + return map; +} + +export {createNestedCollection}; From 3557027d7ccadd1dfa89cb3d2042e1c5ae927523 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 15 Mar 2024 18:40:36 +0500 Subject: [PATCH 32/51] fix: resolve merge conflicts --- src/hooks/useReportIDs.tsx | 114 ++++--------------------------------- 1 file changed, 11 insertions(+), 103 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index f9b59be8e14c..422febfca140 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -3,37 +3,21 @@ import {withOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {getCurrentUserAccountID} from '@libs/actions/Report'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; -import * as ReportUtils from '@libs/ReportUtils'; import SidebarUtils from '@libs/SidebarUtils'; -import type {PolicySelector} from '@pages/home/sidebar/SidebarLinksData'; 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 type {Beta, Policy, PolicyMembers, PriorityMode, Report, ReportActions, TransactionViolation} from '@src/types/onyx'; import useActiveWorkspace from './useActiveWorkspace'; import useCurrentReportID from './useCurrentReportID'; type OnyxProps = { - /** List of reports */ - chatReports: OnyxCollection; - - /** Beta features list */ - betas: OnyxEntry; - - /** The policies which the user has access to */ - policies: OnyxCollection; - - /** All report actions for all reports */ - allReportActions: OnyxCollection; - - /** All of the transaction violations */ - transactionViolations: OnyxCollection; - - /** All policy members */ - policyMembers: OnyxCollection; - - /** The chat priority mode */ - priorityMode: OnyxTypes.PriorityMode; + chatReports: OnyxCollection; + betas: OnyxEntry; + policies: OnyxCollection; + allReportActions: OnyxCollection; + transactionViolations: OnyxCollection; + policyMembers: OnyxCollection; + priorityMode: OnyxEntry; }; type WithReportIDsContextProviderProps = OnyxProps & { @@ -79,10 +63,10 @@ function WithReportIDsContextProvider({ SidebarUtils.getOrderedReportIDs( currentReportID ?? null, chatReports, - betas, - policies as OnyxCollection, + betas ?? [], + policies, priorityMode, - allReportActions as OnyxCollection, + allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, @@ -114,84 +98,10 @@ function WithReportIDsContextProvider({ return {children}; } -type ChatReportSelector = OnyxTypes.Report & {isUnreadWithMention: boolean}; -type ReportActionsSelector = Array>; - -/** - * 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): ChatReportSelector => - (report && { - reportID: report.reportID, - participantAccountIDs: report.participantAccountIDs, - hasDraft: report.hasDraft, - 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): 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): PolicySelector => - (policy && { - type: policy.type, - name: policy.name, - avatar: policy.avatar, - }) as PolicySelector; - const ReportIDsContextProvider = withOnyx({ chatReports: { key: ONYXKEYS.COLLECTION.REPORT, initialValue: {}, - selector: chatReportSelector, }, priorityMode: { key: ONYXKEYS.NVP_PRIORITY_MODE, @@ -204,12 +114,10 @@ const ReportIDsContextProvider = withOnyx Date: Thu, 21 Mar 2024 15:13:38 +0500 Subject: [PATCH 33/51] refactor: use currentReportID from useReportIDs --- src/hooks/useReportIDs.tsx | 5 ++++- src/pages/home/sidebar/SidebarLinksData.js | 6 ++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 422febfca140..bdb9bbf9ada3 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -27,10 +27,12 @@ type WithReportIDsContextProviderProps = OnyxProps & { type ReportIDsContextValue = { orderedReportIDs: string[]; + currentReportID: string; }; const ReportIDsContext = createContext({ orderedReportIDs: [], + currentReportID: '', }); function WithReportIDsContextProvider({ @@ -91,8 +93,9 @@ function WithReportIDsContextProvider({ const contextValue = useMemo( () => ({ orderedReportIDs: orderedReportIDsWithCurrentReport, + currentReportID: derivedCurrentReportID ?? '', }), - [orderedReportIDsWithCurrentReport], + [orderedReportIDsWithCurrentReport, derivedCurrentReportID], ); return {children}; diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 35c4a2c59cd2..1f4a8cf343d4 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -5,7 +5,6 @@ import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import networkPropTypes from '@components/networkPropTypes'; import {withNetwork} from '@components/OnyxProvider'; -import withCurrentReportID from '@components/withCurrentReportID'; import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsDefaultProps, withCurrentUserPersonalDetailsPropTypes} from '@components/withCurrentUserPersonalDetails'; import withNavigationFocus from '@components/withNavigationFocus'; import useActiveWorkspace from '@hooks/useActiveWorkspace'; @@ -44,7 +43,7 @@ const defaultProps = { ...withCurrentUserPersonalDetailsDefaultProps, }; -function SidebarLinksData({isFocused, currentReportID, insets, isLoadingApp, onLinkClick, priorityMode, network, policyMembers, currentUserPersonalDetails}) { +function SidebarLinksData({isFocused, insets, isLoadingApp, onLinkClick, priorityMode, network, policyMembers, currentUserPersonalDetails}) { const styles = useThemeStyles(); const {activeWorkspaceID} = useActiveWorkspace(); const {translate} = useLocalize(); @@ -57,7 +56,7 @@ function SidebarLinksData({isFocused, currentReportID, insets, isLoadingApp, onL const orderedReportIDsRef = useRef(null); const isLoading = isLoadingApp; - const {orderedReportIDs} = useReportIDs(); + const {orderedReportIDs, currentReportID} = useReportIDs(); const optionListItems = useMemo(() => { if (deepEqual(orderedReportIDsRef.current, orderedReportIDs)) { @@ -103,7 +102,6 @@ SidebarLinksData.defaultProps = defaultProps; SidebarLinksData.displayName = 'SidebarLinksData'; export default compose( - withCurrentReportID, withCurrentUserPersonalDetails, withNavigationFocus, withNetwork(), From 84aa5ac37af2faf3eeb547b600627ef9c40fa920 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 21 Mar 2024 16:10:24 +0500 Subject: [PATCH 34/51] refactor: remove unnecessary ref --- src/pages/home/sidebar/SidebarLinksData.js | 29 +++------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 1f4a8cf343d4..207eed55586a 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -1,15 +1,11 @@ -import {deepEqual} from 'fast-equals'; import PropTypes from 'prop-types'; -import React, {useCallback, useEffect, useMemo, useRef} from 'react'; +import React, {useCallback, useEffect, useRef} from 'react'; import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; -import networkPropTypes from '@components/networkPropTypes'; -import {withNetwork} from '@components/OnyxProvider'; import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsDefaultProps, withCurrentUserPersonalDetailsPropTypes} from '@components/withCurrentUserPersonalDetails'; import withNavigationFocus from '@components/withNavigationFocus'; import useActiveWorkspace from '@hooks/useActiveWorkspace'; import useLocalize from '@hooks/useLocalize'; -import usePrevious from '@hooks/usePrevious'; import {useReportIDs} from '@hooks/useReportIDs'; import useThemeStyles from '@hooks/useThemeStyles'; import compose from '@libs/compose'; @@ -28,8 +24,6 @@ const propTypes = { /** The chat priority mode */ priorityMode: PropTypes.string, - network: networkPropTypes.isRequired, - // eslint-disable-next-line react/forbid-prop-types policyMembers: PropTypes.object, @@ -43,35 +37,19 @@ const defaultProps = { ...withCurrentUserPersonalDetailsDefaultProps, }; -function SidebarLinksData({isFocused, insets, isLoadingApp, onLinkClick, priorityMode, network, policyMembers, currentUserPersonalDetails}) { +function SidebarLinksData({isFocused, insets, isLoadingApp, onLinkClick, priorityMode, policyMembers, currentUserPersonalDetails}) { const styles = useThemeStyles(); const {activeWorkspaceID} = useActiveWorkspace(); const {translate} = useLocalize(); - const prevPriorityMode = usePrevious(priorityMode); const policyMemberAccountIDs = getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, currentUserPersonalDetails.accountID); // eslint-disable-next-line react-hooks/exhaustive-deps useEffect(() => Policy.openWorkspace(activeWorkspaceID, policyMemberAccountIDs), [activeWorkspaceID]); - const orderedReportIDsRef = useRef(null); const isLoading = isLoadingApp; const {orderedReportIDs, currentReportID} = useReportIDs(); - const optionListItems = useMemo(() => { - if (deepEqual(orderedReportIDsRef.current, orderedReportIDs)) { - return orderedReportIDsRef.current; - } - - // 1. We need to update existing reports only once while loading because they are updated several times during loading and causes this regression: https://github.com/Expensify/App/issues/24596#issuecomment-1681679531 - // 2. If the user is offline, we need to update the reports unconditionally, since the loading of report data might be stuck in this case. - // 3. Changing priority mode to Most Recent will call OpenApp. If there is an existing reports and the priority mode is updated, we want to immediately update the list instead of waiting the OpenApp request to complete - if (!isLoading || !orderedReportIDsRef.current || network.isOffline || (orderedReportIDsRef.current && prevPriorityMode !== priorityMode)) { - orderedReportIDsRef.current = orderedReportIDs; - } - return orderedReportIDsRef.current || []; - }, [orderedReportIDs, isLoading, network.isOffline, prevPriorityMode, priorityMode]); - const currentReportIDRef = useRef(currentReportID); currentReportIDRef.current = currentReportID; const isActiveReport = useCallback((reportID) => currentReportIDRef.current === reportID, []); @@ -91,7 +69,7 @@ function SidebarLinksData({isFocused, insets, isLoadingApp, onLinkClick, priorit isActiveReport={isActiveReport} isLoading={isLoading} activeWorkspaceID={activeWorkspaceID} - optionListItems={optionListItems} + optionListItems={orderedReportIDs} /> ); @@ -104,7 +82,6 @@ SidebarLinksData.displayName = 'SidebarLinksData'; export default compose( withCurrentUserPersonalDetails, withNavigationFocus, - withNetwork(), withOnyx({ isLoadingApp: { key: ONYXKEYS.IS_LOADING_APP, From 8532c0864d9b6a2ffb88ae3759801781d6bf23af Mon Sep 17 00:00:00 2001 From: Muhammad Hur Ali Date: Thu, 21 Mar 2024 16:16:02 +0500 Subject: [PATCH 35/51] update comment is updateCurrentReportID function Co-authored-by: Rory Abraham <47436092+roryabraham@users.noreply.github.com> --- src/components/withCurrentReportID.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index 55b542ccacb7..27a965a151de 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -42,7 +42,7 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro const reportID = Navigation.getTopmostReportId(state) ?? ''; /** - * This is to make sure we don't set the undefined as reportID when + * This is to make sure we don't set the reportID as undefined when * switching between chat list and settings tab. The settings tab * includes multiple screens and we don't want to set the reportID * to falsy value when switching between them. From ac465f3976aef5d891f7ba4e7eb4bc2043ca13a3 Mon Sep 17 00:00:00 2001 From: Muhammad Hur Ali Date: Thu, 21 Mar 2024 16:16:46 +0500 Subject: [PATCH 36/51] update comment in useReportIDs hook Co-authored-by: Rory Abraham <47436092+roryabraham@users.noreply.github.com> --- src/hooks/useReportIDs.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index bdb9bbf9ada3..4d7474905a2e 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -47,7 +47,7 @@ function WithReportIDsContextProvider({ /** * Only required to make unit tests work, since we * explicitly pass the currentReportID in LHNTestUtils - * to SidebarLinksData, so this context doesn't have an + * 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. From 886622e7940a7b28e11e299d0aa89a6e34030cc6 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 21 Mar 2024 16:20:12 +0500 Subject: [PATCH 37/51] refactor: make comment less verbose --- src/components/withCurrentReportID.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/withCurrentReportID.tsx b/src/components/withCurrentReportID.tsx index 27a965a151de..a72063913283 100644 --- a/src/components/withCurrentReportID.tsx +++ b/src/components/withCurrentReportID.tsx @@ -41,13 +41,9 @@ function CurrentReportIDContextProvider(props: CurrentReportIDContextProviderPro (state: NavigationState) => { const reportID = Navigation.getTopmostReportId(state) ?? ''; - /** - * This is to make sure we don't set the reportID as undefined when - * switching between chat list and settings tab. The settings tab - * includes multiple screens and we don't want to set the reportID - * to falsy value when switching between them. - * - * Doing so avoids an unnecessary re-render of `useReportIDs`. + /* + * 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) { From 6d17116c0e2af4fbadbe79b366fb16c7434b51be Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 21 Mar 2024 16:20:46 +0500 Subject: [PATCH 38/51] refactor: remove irrelevant comment --- src/components/LHNOptionsList/LHNOptionsList.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/components/LHNOptionsList/LHNOptionsList.tsx b/src/components/LHNOptionsList/LHNOptionsList.tsx index c9052567ac36..fa4c89216d08 100644 --- a/src/components/LHNOptionsList/LHNOptionsList.tsx +++ b/src/components/LHNOptionsList/LHNOptionsList.tsx @@ -122,12 +122,6 @@ function LHNOptionsList({ keyExtractor={keyExtractor} renderItem={renderItem} estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight} - // Previously, we were passing `extraData={[currentReportID]}`, which upon every render, was causing the - // re-render because of the new array reference. FlashList's children actually don't depend on the - // `currentReportID` prop but they depend on the `reportActions`, `reports`, `policy`, `personalDetails`. - // Previously it was working for us because of the new array reference. Even if you only pass an empty - // array, it will still work because of the new reference. But it's better to pass the actual dependencies - // to avoid unnecessary re-renders. extraData={extraData} showsVerticalScrollIndicator={false} /> From 2ef5c7f58a2400bbca01dd007b3bf19b4346aa53 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 22 Mar 2024 13:10:46 +0500 Subject: [PATCH 39/51] refactor: reduce memo usage --- src/hooks/useReportIDs.tsx | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 4d7474905a2e..4af75c3e4e5b 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -77,26 +77,21 @@ function WithReportIDsContextProvider({ ); const orderedReportIDs = useMemo(() => getOrderedReportIDs(), [getOrderedReportIDs]); - - // 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. - const orderedReportIDsWithCurrentReport = useMemo(() => { + 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 getOrderedReportIDs(derivedCurrentReportID); + return {orderedReportIDs: getOrderedReportIDs(derivedCurrentReportID), currentReportID: derivedCurrentReportID ?? ''}; } - return orderedReportIDs; - }, [getOrderedReportIDs, derivedCurrentReportID, orderedReportIDs]); - const contextValue = useMemo( - () => ({ - orderedReportIDs: orderedReportIDsWithCurrentReport, + return { + orderedReportIDs: getOrderedReportIDs(), currentReportID: derivedCurrentReportID ?? '', - }), - [orderedReportIDsWithCurrentReport, derivedCurrentReportID], - ); + }; + }, [getOrderedReportIDs, orderedReportIDs, derivedCurrentReportID]); return {children}; } From 9e4f4640198210dfc259da9901500df48e85852c Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 16 Apr 2024 13:47:47 +0500 Subject: [PATCH 40/51] fix: typecheck --- tests/perf-test/SidebarUtils.perf-test.ts | 26 ++++++++++++++++----- tests/utils/LHNTestUtils.tsx | 1 + tests/utils/collections/createCollection.ts | 25 +------------------- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index 4bd04e823f63..8566abb97c7f 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -9,7 +9,7 @@ import type {PersonalDetails, TransactionViolation} from '@src/types/onyx'; import type Policy from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; import type ReportAction from '@src/types/onyx/ReportAction'; -import createCollection, {createNestedCollection} from '../utils/collections/createCollection'; +import createCollection from '../utils/collections/createCollection'; import createPersonalDetails from '../utils/collections/personalDetails'; import createRandomPolicy from '../utils/collections/policies'; import createRandomReportAction, {getRandomDate} from '../utils/collections/reportActions'; @@ -51,11 +51,25 @@ const policies = createCollection( const mockedBetas = Object.values(CONST.BETAS); -const allReportActions = createNestedCollection( - (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, - (item) => `${item.reportActionID}`, - (index) => createRandomReportAction(index), -); +const allReportActions = Object.fromEntries( + Object.keys(reportActions).map((key) => [ + `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${key}`, + [ + { + errors: reportActions[key].errors ?? [], + message: [ + { + moderationDecision: { + decision: reportActions[key].message?.[0]?.moderationDecision?.decision, + }, + }, + ], + reportActionID: reportActions[key].reportActionID, + }, + ], + ]), +) as unknown as OnyxCollection; + const currentReportId = '1'; const transactionViolations = {} as OnyxCollection; diff --git a/tests/utils/LHNTestUtils.tsx b/tests/utils/LHNTestUtils.tsx index 331453adfeb8..ab710c5dc8e0 100644 --- a/tests/utils/LHNTestUtils.tsx +++ b/tests/utils/LHNTestUtils.tsx @@ -297,6 +297,7 @@ function MockedSidebarLinks({currentReportID = ''}: MockedSidebarLinksProps) { right: 0, bottom: 0, }} + // @ts-expect-error - we need this prop to be able to test the component but normally its provided by HOC currentReportID={currentReportID} /> diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index 4a2a1fa4eb78..927864191411 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -8,27 +8,4 @@ export default function createCollection(createKey: (item: T, index: number) } return map; -} - -function createNestedCollection( - createParentKey: (item: T, index: number) => string | number, - createKey: (item: T, index: number) => string | number, - createItem: (index: number) => T, - length = 500, -): Record> { - const map: Record> = {}; - - for (let i = 0; i < length; i++) { - const item = createItem(i); - const itemKey = createKey(item, i); - const itemParentKey = createParentKey(item, i); - map[itemParentKey] = { - ...map[itemParentKey], - [itemKey]: item, - }; - } - - return map; -} - -export {createNestedCollection}; +} \ No newline at end of file From 20d798e6eed9ded69373add54e11c0445ac9f9b4 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 16 Apr 2024 13:49:26 +0500 Subject: [PATCH 41/51] refactor: remove unnecessary diff --- tests/utils/collections/createCollection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils/collections/createCollection.ts b/tests/utils/collections/createCollection.ts index 927864191411..848ef8f81f47 100644 --- a/tests/utils/collections/createCollection.ts +++ b/tests/utils/collections/createCollection.ts @@ -8,4 +8,4 @@ export default function createCollection(createKey: (item: T, index: number) } return map; -} \ No newline at end of file +} From 6f20396264e128ff960c6da68d0e862d7942dc0a Mon Sep 17 00:00:00 2001 From: hurali97 Date: Tue, 16 Apr 2024 17:23:49 +0500 Subject: [PATCH 42/51] refactor: leverage useOnyx hook instead of withOnyx HOC --- src/hooks/useReportIDs.tsx | 195 +++++++++++++++---------------------- 1 file changed, 81 insertions(+), 114 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index c807e7dfcdcc..d3426866fe63 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -1,5 +1,5 @@ import React, {createContext, useCallback, useContext, useMemo} from 'react'; -import {withOnyx} from 'react-native-onyx'; +import {useOnyx} from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {getPolicyMembersByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; @@ -16,18 +16,7 @@ type ChatReportSelector = OnyxTypes.Report & {isUnreadWithMention: boolean}; type PolicySelector = Pick; type ReportActionsSelector = Array>; -type OnyxProps = { - chatReports: OnyxCollection; - betas: OnyxEntry; - policies: OnyxCollection; - allReportActions: OnyxCollection; - transactionViolations: OnyxCollection; - policyMembers: OnyxCollection; - priorityMode: OnyxEntry; - reportsDrafts: OnyxCollection; -}; - -type WithReportIDsContextProviderProps = OnyxProps & { +type ReportIDsContextProviderProps = { children: React.ReactNode; currentReportIDForTests?: string; }; @@ -42,71 +31,6 @@ const ReportIDsContext = createContext({ currentReportID: '', }); -function WithReportIDsContextProvider({ - children, - chatReports, - betas, - policies, - allReportActions, - transactionViolations, - policyMembers, - priorityMode, - reportsDrafts, - /** - * 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, -}: WithReportIDsContextProviderProps) { - const {accountID} = useCurrentUserPersonalDetails(); - const currentReportIDValue = useCurrentReportID(); - const derivedCurrentReportID = currentReportIDForTests ?? currentReportIDValue?.currentReportID; - const {activeWorkspaceID} = useActiveWorkspace(); - - const policyMemberAccountIDs = useMemo(() => getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, accountID), [activeWorkspaceID, policyMembers, accountID]); - - const getOrderedReportIDs = useCallback( - (currentReportID?: string) => - SidebarUtils.getOrderedReportIDs( - currentReportID ?? null, - chatReports, - betas, - policies as OnyxCollection, - priorityMode, - allReportActions as OnyxCollection, - 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: getOrderedReportIDs(), - currentReportID: derivedCurrentReportID ?? '', - }; - }, [getOrderedReportIDs, orderedReportIDs, derivedCurrentReportID]); - - return {children}; -} - /** * 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. @@ -176,42 +100,85 @@ const policySelector = (policy: OnyxEntry): PolicySelector => avatar: policy.avatar, }) as PolicySelector; -const ReportIDsContextProvider = withOnyx({ - chatReports: { - key: ONYXKEYS.COLLECTION.REPORT, - selector: chatReportSelector, - initialValue: {}, - }, - 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: {}, - }, - policyMembers: { - key: ONYXKEYS.COLLECTION.POLICY_MEMBERS, - }, - transactionViolations: { - key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, - initialValue: {}, - }, - reportsDrafts: { - key: ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, - initialValue: {}, - }, -})(WithReportIDsContextProvider); +const priorityModeOptions = { + initialValue: CONST.PRIORITY_MODE.DEFAULT, +}; + +const chatReportsOptions = { + selector: chatReportSelector, +}; + +const policiesOptions = {selector: policySelector}; + +const allReportActionsOptions = {selector: reportActionsSelector}; + +const betasOptions = {initialValue: []}; + +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, +}: ReportIDsContextProviderProps) { + const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, priorityModeOptions); + const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, chatReportsOptions); + const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, policiesOptions); + const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, allReportActionsOptions); + const [policyMembers] = useOnyx(ONYXKEYS.COLLECTION.POLICY_MEMBERS); + const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); + const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT); + const [betas] = useOnyx(ONYXKEYS.BETAS, betasOptions); + + const {accountID} = useCurrentUserPersonalDetails(); + const currentReportIDValue = useCurrentReportID(); + const derivedCurrentReportID = currentReportIDForTests ?? currentReportIDValue?.currentReportID; + const {activeWorkspaceID} = useActiveWorkspace(); + + const policyMemberAccountIDs = useMemo(() => getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, accountID), [activeWorkspaceID, policyMembers, accountID]); + + const getOrderedReportIDs = useCallback( + (currentReportID?: string) => + SidebarUtils.getOrderedReportIDs( + currentReportID ?? null, + chatReports, + betas, + policies as OnyxCollection, + priorityMode, + allReportActions as OnyxCollection, + 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: getOrderedReportIDs(), + currentReportID: derivedCurrentReportID ?? '', + }; + }, [getOrderedReportIDs, orderedReportIDs, derivedCurrentReportID]); + + return {children}; +} function useReportIDs() { return useContext(ReportIDsContext); From c791be001044b85cc54bd0bf7536b7c243e0144a Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 18 Apr 2024 12:31:53 +0500 Subject: [PATCH 43/51] fix: max depth exceeded error by not mutating allReports object --- src/libs/SidebarUtils.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index cbc226f512ab..bfee4c2cbd67 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -106,7 +106,7 @@ function getOrderedReportIDs( report, currentReportId: currentReportId ?? '', isInGSDMode, - betas: betas ?? [], + betas, policies, excludeEmptyChats: true, doesReportHaveViolations, @@ -135,15 +135,11 @@ function getOrderedReportIDs( } // There are a few properties that need to be calculated for the report which are used when sorting reports. reportsToDisplay.forEach((report) => { - if (!report) { - return; - } - // 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. 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; From 4aa6b99ea61f6072bfd5185bc6563ad719e7e514 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 18 Apr 2024 12:32:43 +0500 Subject: [PATCH 44/51] fix: use inline options and relevant initialValue --- src/hooks/useReportIDs.tsx | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index fa0bd4c75ff0..2862694219e8 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -101,20 +101,6 @@ const policySelector = (policy: OnyxEntry): PolicySelector => employeeList: policy.employeeList, }) as PolicySelector; -const priorityModeOptions = { - initialValue: CONST.PRIORITY_MODE.DEFAULT, -}; - -const chatReportsOptions = { - selector: chatReportSelector, -}; - -const policiesOptions = {selector: policySelector}; - -const allReportActionsOptions = {selector: reportActionsSelector}; - -const betasOptions = {initialValue: []}; - function ReportIDsContextProvider({ children, /** @@ -127,14 +113,14 @@ function ReportIDsContextProvider({ */ currentReportIDForTests, }: ReportIDsContextProviderProps) { - const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, priorityModeOptions); - const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, chatReportsOptions); - const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, policiesOptions); - const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, allReportActionsOptions); + const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, {initialValue: CONST.PRIORITY_MODE.DEFAULT}); + const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {selector: chatReportSelector, initialValue: {}}); + const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: policySelector, initialValue: {}}); + const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {selector: reportActionsSelector, initialValue: {}}); const [policyMembers] = useOnyx(ONYXKEYS.COLLECTION.POLICY_MEMBERS); - const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); - const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT); - const [betas] = useOnyx(ONYXKEYS.BETAS, betasOptions); + const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {initialValue: {}}); + const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {initialValue: {}}); + const [betas] = useOnyx(ONYXKEYS.BETAS, {initialValue: []}); const {accountID} = useCurrentUserPersonalDetails(); const currentReportIDValue = useCurrentReportID(); From 459e86c0933549f21af87b099711ba9c1b98825d Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 18 Apr 2024 12:32:59 +0500 Subject: [PATCH 45/51] fix: remove not needed prop --- tests/utils/LHNTestUtils.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/utils/LHNTestUtils.tsx b/tests/utils/LHNTestUtils.tsx index 3a7fceb18f2a..e3daa93a3179 100644 --- a/tests/utils/LHNTestUtils.tsx +++ b/tests/utils/LHNTestUtils.tsx @@ -297,8 +297,6 @@ function MockedSidebarLinks({currentReportID = ''}: MockedSidebarLinksProps) { right: 0, bottom: 0, }} - // @ts-expect-error - we need this prop to be able to test the component but normally its provided by HOC - currentReportID={currentReportID} /> From 61fc9ce56bdec845dab66538ec3827b122b25218 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 18 Apr 2024 12:33:25 +0500 Subject: [PATCH 46/51] fix: use correct prop in memo check --- src/pages/home/sidebar/SidebarLinksData.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.tsx b/src/pages/home/sidebar/SidebarLinksData.tsx index bd03dd9d4993..24e30c642bd3 100644 --- a/src/pages/home/sidebar/SidebarLinksData.tsx +++ b/src/pages/home/sidebar/SidebarLinksData.tsx @@ -106,6 +106,6 @@ More details - https://github.com/Expensify/App/issues/35234#issuecomment-192691 prevProps.priorityMode === nextProps.priorityMode && lodashIsEqual(prevProps.insets, nextProps.insets) && prevProps.onLinkClick === nextProps.onLinkClick && - lodashIsEqual(prevProps.policyMembers, nextProps.policyMembers), + lodashIsEqual(prevProps.policies, nextProps.policies), ), ); From 489c6520a86cb459f95d205cef272ecf64df0495 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 18 Apr 2024 12:35:13 +0500 Subject: [PATCH 47/51] fix: use policies instead of policyMembers --- src/hooks/useReportIDs.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 2862694219e8..1bad62e469a7 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -117,7 +117,6 @@ function ReportIDsContextProvider({ const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {selector: chatReportSelector, initialValue: {}}); const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: policySelector, initialValue: {}}); const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {selector: reportActionsSelector, initialValue: {}}); - const [policyMembers] = useOnyx(ONYXKEYS.COLLECTION.POLICY_MEMBERS); const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {initialValue: {}}); const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {initialValue: {}}); const [betas] = useOnyx(ONYXKEYS.BETAS, {initialValue: []}); @@ -127,7 +126,7 @@ function ReportIDsContextProvider({ const derivedCurrentReportID = currentReportIDForTests ?? currentReportIDValue?.currentReportID; const {activeWorkspaceID} = useActiveWorkspace(); - const policyMemberAccountIDs = useMemo(() => getPolicyEmployeeListByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, accountID), [activeWorkspaceID, policyMembers, accountID]); + const policyMemberAccountIDs = useMemo(() => getPolicyEmployeeListByIdWithoutCurrentUser(policies, activeWorkspaceID, accountID), [activeWorkspaceID, policies, accountID]); const getOrderedReportIDs = useCallback( (currentReportID?: string) => From 10c0c048cadd84f47e3c259cd3f46270b9f0aa0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 18 Apr 2024 12:24:19 +0100 Subject: [PATCH 48/51] Apply workaround for initialValue --- src/hooks/useReportIDs.tsx | 23 +++++++++++---------- src/libs/SidebarUtils.ts | 10 +++++---- src/pages/home/sidebar/SidebarLinksData.tsx | 4 ++-- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 1bad62e469a7..99abd627704e 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -1,6 +1,6 @@ import React, {createContext, useCallback, useContext, useMemo} from 'react'; -import {useOnyx} from 'react-native-onyx'; 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'; @@ -13,7 +13,7 @@ import useCurrentReportID from './useCurrentReportID'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; type ChatReportSelector = OnyxTypes.Report & {isUnreadWithMention: boolean}; -export type PolicySelector = Pick; +type PolicySelector = Pick; type ReportActionsSelector = Array>; type ReportIDsContextProviderProps = { @@ -114,12 +114,12 @@ function ReportIDsContextProvider({ currentReportIDForTests, }: ReportIDsContextProviderProps) { const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, {initialValue: CONST.PRIORITY_MODE.DEFAULT}); - const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {selector: chatReportSelector, initialValue: {}}); - const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: policySelector, initialValue: {}}); - const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {selector: reportActionsSelector, initialValue: {}}); - const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {initialValue: {}}); - const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {initialValue: {}}); - const [betas] = useOnyx(ONYXKEYS.BETAS, {initialValue: []}); + 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}); + const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {initialValue: {} as OnyxCollection}); + const [betas] = useOnyx(ONYXKEYS.BETAS, {initialValue: [] as OnyxEntry}); const {accountID} = useCurrentUserPersonalDetails(); const currentReportIDValue = useCurrentReportID(); @@ -134,9 +134,9 @@ function ReportIDsContextProvider({ currentReportID ?? null, chatReports, betas, - policies as OnyxCollection, + policies, priorityMode, - allReportActions as OnyxCollection, + allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, @@ -170,4 +170,5 @@ function useReportIDs() { return useContext(ReportIDsContext); } -export {ReportIDsContextProvider, ReportIDsContext, useReportIDs, policySelector}; +export {ReportIDsContext, ReportIDsContextProvider, policySelector, useReportIDs}; +export type {ChatReportSelector, PolicySelector, ReportActionsSelector}; diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index bfee4c2cbd67..d938fb7d18e6 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -2,6 +2,7 @@ import Str from 'expensify-common/lib/str'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; +import type {ChatReportSelector, PolicySelector, ReportActionsSelector} from '@hooks/useReportIDs'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {PersonalDetails, PersonalDetailsList, ReportActions, TransactionViolation} from '@src/types/onyx'; @@ -65,11 +66,11 @@ function compareStringDates(a: string, b: string): 0 | 1 | -1 { */ function getOrderedReportIDs( currentReportId: string | null, - allReports: OnyxCollection, + allReports: OnyxCollection, betas: OnyxEntry, - policies: OnyxCollection, + policies: OnyxCollection, priorityMode: OnyxEntry, - allReportActions: OnyxCollection, + allReportActions: OnyxCollection, transactionViolations: OnyxCollection, currentPolicyID = '', policyMemberAccountIDs: number[] = [], @@ -136,10 +137,11 @@ function getOrderedReportIDs( // There are a few properties that need to be calculated for the report which are used when sorting reports. reportsToDisplay.forEach((report) => { if (report) { + // eslint-disable-next-line no-param-reassign report = { ...report, displayName: ReportUtils.getReportName(report), - } + }; } const isPinned = report?.isPinned ?? false; diff --git a/src/pages/home/sidebar/SidebarLinksData.tsx b/src/pages/home/sidebar/SidebarLinksData.tsx index 24e30c642bd3..46f7d2410ffe 100644 --- a/src/pages/home/sidebar/SidebarLinksData.tsx +++ b/src/pages/home/sidebar/SidebarLinksData.tsx @@ -9,13 +9,13 @@ import type {ValueOf} from 'type-fest'; import useActiveWorkspace from '@hooks/useActiveWorkspace'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useLocalize from '@hooks/useLocalize'; -import {PolicySelector, policySelector, useReportIDs} from '@hooks/useReportIDs'; +import type {PolicySelector} from '@hooks/useReportIDs'; +import {policySelector, useReportIDs} from '@hooks/useReportIDs'; import useThemeStyles from '@hooks/useThemeStyles'; import {getPolicyEmployeeListByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import * as Policy from '@userActions/Policy'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type * as OnyxTypes from '@src/types/onyx'; import SidebarLinks from './SidebarLinks'; type SidebarLinksDataOnyxProps = { From e075aee5bd4a246ceee3c83271072c821aac79e2 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 19 Apr 2024 17:11:13 +0500 Subject: [PATCH 49/51] fix: failing SidebarOrderTest --- src/hooks/useReportIDs.tsx | 4 ++-- tests/unit/SidebarOrderTest.ts | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 99abd627704e..97e2b5a52830 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -126,7 +126,7 @@ function ReportIDsContextProvider({ const derivedCurrentReportID = currentReportIDForTests ?? currentReportIDValue?.currentReportID; const {activeWorkspaceID} = useActiveWorkspace(); - const policyMemberAccountIDs = useMemo(() => getPolicyEmployeeListByIdWithoutCurrentUser(policies, activeWorkspaceID, accountID), [activeWorkspaceID, policies, accountID]); + const policyMemberAccountIDs = getPolicyEmployeeListByIdWithoutCurrentUser(policies, activeWorkspaceID, accountID); const getOrderedReportIDs = useCallback( (currentReportID?: string) => @@ -158,7 +158,7 @@ function ReportIDsContextProvider({ } return { - orderedReportIDs: getOrderedReportIDs(), + orderedReportIDs, currentReportID: derivedCurrentReportID ?? '', }; }, [getOrderedReportIDs, orderedReportIDs, derivedCurrentReportID]); diff --git a/tests/unit/SidebarOrderTest.ts b/tests/unit/SidebarOrderTest.ts index 3b3de600fdd8..5999f915ea82 100644 --- a/tests/unit/SidebarOrderTest.ts +++ b/tests/unit/SidebarOrderTest.ts @@ -785,10 +785,12 @@ describe('Sidebar', () => { // When a new report is added .then(() => - Promise.all([ - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`, report4), - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report4.reportID}`, 'report4 draft'), - ]), + Onyx.multiSet({ + ...reportDraftCommentCollectionDataSet, + [`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${report4.reportID}`]: 'report4 draft', + ...reportCollectionDataSet, + [`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`]: report4, + }), ) // Then they are still in alphabetical order From 0d4637e941b365a6a3c6894d3dd89dde41b83df7 Mon Sep 17 00:00:00 2001 From: hurali97 Date: Fri, 19 Apr 2024 17:11:36 +0500 Subject: [PATCH 50/51] fix: typecheck --- src/libs/SidebarUtils.ts | 8 ++++---- tests/perf-test/SidebarUtils.perf-test.ts | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index d938fb7d18e6..107ccd2e789a 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -92,7 +92,7 @@ function getOrderedReportIDs( const doesReportHaveViolations = !!( betas?.includes(CONST.BETAS.VIOLATIONS) && !!parentReportAction && - ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction) + ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction as OnyxEntry) ); const isHidden = report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN; const isFocused = report.reportID === currentReportId; @@ -108,7 +108,7 @@ function getOrderedReportIDs( currentReportId: currentReportId ?? '', isInGSDMode, betas, - policies, + policies: policies as OnyxCollection, excludeEmptyChats: true, doesReportHaveViolations, includeSelfDM: true, @@ -135,9 +135,9 @@ function getOrderedReportIDs( ); } // There are a few properties that need to be calculated for the report which are used when sorting reports. - reportsToDisplay.forEach((report) => { + reportsToDisplay.forEach((reportToDisplay) => { + let report = reportToDisplay as OnyxEntry; if (report) { - // eslint-disable-next-line no-param-reassign report = { ...report, displayName: ReportUtils.getReportName(report), diff --git a/tests/perf-test/SidebarUtils.perf-test.ts b/tests/perf-test/SidebarUtils.perf-test.ts index 8566abb97c7f..cceb3ae437b9 100644 --- a/tests/perf-test/SidebarUtils.perf-test.ts +++ b/tests/perf-test/SidebarUtils.perf-test.ts @@ -2,12 +2,12 @@ import {rand} from '@ngneat/falso'; import type {OnyxCollection} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import {measureFunction} from 'reassure'; +import type {ChatReportSelector} from '@hooks/useReportIDs'; import SidebarUtils from '@libs/SidebarUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {PersonalDetails, TransactionViolation} from '@src/types/onyx'; import type Policy from '@src/types/onyx/Policy'; -import type Report from '@src/types/onyx/Report'; import type ReportAction from '@src/types/onyx/ReportAction'; import createCollection from '../utils/collections/createCollection'; import createPersonalDetails from '../utils/collections/personalDetails'; @@ -20,7 +20,7 @@ const REPORTS_COUNT = 15000; const REPORT_TRESHOLD = 5; const PERSONAL_DETAILS_LIST_COUNT = 1000; -const allReports = createCollection( +const allReports = createCollection( (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, (index) => ({ ...createRandomReport(index), @@ -29,6 +29,7 @@ const allReports = createCollection( // add status and state to every 5th report to mock nonarchived reports statusNum: index % REPORT_TRESHOLD ? 0 : CONST.REPORT.STATUS_NUM.CLOSED, stateNum: index % REPORT_TRESHOLD ? 0 : CONST.REPORT.STATE_NUM.APPROVED, + isUnreadWithMention: false, }), REPORTS_COUNT, ); From 821e4e0c30ae5b5c198d66114336bdcabe25650a Mon Sep 17 00:00:00 2001 From: hurali97 Date: Thu, 25 Apr 2024 11:41:12 +0500 Subject: [PATCH 51/51] refactor: remove initialValues --- src/hooks/useReportIDs.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/hooks/useReportIDs.tsx b/src/hooks/useReportIDs.tsx index 97e2b5a52830..58d4e42cd83b 100644 --- a/src/hooks/useReportIDs.tsx +++ b/src/hooks/useReportIDs.tsx @@ -1,5 +1,5 @@ import React, {createContext, useCallback, useContext, useMemo} from 'react'; -import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; +import type {OnyxEntry} from 'react-native-onyx'; import {useOnyx} from 'react-native-onyx'; import {getPolicyEmployeeListByIdWithoutCurrentUser} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; @@ -114,12 +114,12 @@ function ReportIDsContextProvider({ currentReportIDForTests, }: 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}); - const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {initialValue: {} as OnyxCollection}); - const [betas] = useOnyx(ONYXKEYS.BETAS, {initialValue: [] as OnyxEntry}); + const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {selector: chatReportSelector}); + const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: policySelector}); + const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {selector: reportActionsSelector}); + const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); + const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT); + const [betas] = useOnyx(ONYXKEYS.BETAS); const {accountID} = useCurrentUserPersonalDetails(); const currentReportIDValue = useCurrentReportID();