From 78efe2f1887f0b211fbbb22d23e2b22c09e68621 Mon Sep 17 00:00:00 2001 From: tienifr Date: Fri, 22 Mar 2024 00:40:02 +0700 Subject: [PATCH 1/5] Perf optimization by holding personal details loading state in metadata onyx key --- src/ONYXKEYS.ts | 4 ++++ src/libs/actions/PersonalDetails.ts | 6 +++--- src/pages/ProfilePage.tsx | 14 ++++++++++---- src/pages/settings/Profile/ProfileAvatar.tsx | 10 +++++++--- src/types/onyx/PersonalDetails.ts | 10 ++++++---- src/types/onyx/index.ts | 3 ++- 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index d3fab1b9fcde..252e81ce707a 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -60,6 +60,9 @@ const ONYXKEYS = { /** Contains all the private personal details of the user */ PRIVATE_PERSONAL_DETAILS: 'private_personalDetails', + /** REPORT_METADATA is a perf optimization used to hold loading states. See REPORT_METADATA. */ + PERSONAL_DETAILS_METADATA: 'personalDetailsMetadata', + /** Contains all the info for Tasks */ TASK: 'task', @@ -546,6 +549,7 @@ type OnyxValuesMapping = { [ONYXKEYS.INPUT_FOCUSED]: boolean; [ONYXKEYS.PERSONAL_DETAILS_LIST]: OnyxTypes.PersonalDetailsList; [ONYXKEYS.PRIVATE_PERSONAL_DETAILS]: OnyxTypes.PrivatePersonalDetails; + [ONYXKEYS.PERSONAL_DETAILS_METADATA]: Record; [ONYXKEYS.TASK]: OnyxTypes.Task; [ONYXKEYS.WORKSPACE_RATE_AND_UNIT]: OnyxTypes.WorkspaceRateAndUnit; [ONYXKEYS.CURRENCY_LIST]: OnyxTypes.CurrencyList; diff --git a/src/libs/actions/PersonalDetails.ts b/src/libs/actions/PersonalDetails.ts index 5becaee1593c..20323020aa0b 100644 --- a/src/libs/actions/PersonalDetails.ts +++ b/src/libs/actions/PersonalDetails.ts @@ -286,7 +286,7 @@ function openPublicProfilePage(accountID: number) { const optimisticData: OnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS_LIST, + key: ONYXKEYS.PERSONAL_DETAILS_METADATA, value: { [accountID]: { isLoading: true, @@ -298,7 +298,7 @@ function openPublicProfilePage(accountID: number) { const successData: OnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS_LIST, + key: ONYXKEYS.PERSONAL_DETAILS_METADATA, value: { [accountID]: { isLoading: false, @@ -310,7 +310,7 @@ function openPublicProfilePage(accountID: number) { const failureData: OnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS_LIST, + key: ONYXKEYS.PERSONAL_DETAILS_METADATA, value: { [accountID]: { isLoading: false, diff --git a/src/pages/ProfilePage.tsx b/src/pages/ProfilePage.tsx index a9c350102424..f3c2840d5586 100755 --- a/src/pages/ProfilePage.tsx +++ b/src/pages/ProfilePage.tsx @@ -36,7 +36,7 @@ import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; -import type {PersonalDetails, PersonalDetailsList, Report, Session} from '@src/types/onyx'; +import type {PersonalDetails, PersonalDetailsList, PersonalDetailsMetadata, Report, Session} from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {EmptyObject} from '@src/types/utils/EmptyObject'; @@ -44,6 +44,9 @@ type ProfilePageOnyxProps = { /** The personal details of the person who is logged in */ personalDetails: OnyxEntry; + /** Loading status of the personal details */ + personalDetailsMetadata: OnyxEntry>; + /** The report currently being looked at */ report: OnyxEntry; @@ -74,12 +77,12 @@ const getPhoneNumber = ({login = '', displayName = ''}: PersonalDetails | EmptyO return login ? Str.removeSMSDomain(login) : ''; }; -function ProfilePage({personalDetails, route, session, report}: ProfilePageProps) { +function ProfilePage({personalDetails, personalDetailsMetadata, route, session, report}: ProfilePageProps) { const styles = useThemeStyles(); const {translate, formatPhoneNumber} = useLocalize(); const accountID = Number(route.params?.accountID ?? 0); const isCurrentUser = session?.accountID === accountID; - const details: PersonalDetails | EmptyObject = personalDetails?.[accountID] ?? (ValidationUtils.isValidAccountRoute(accountID) ? {} : {isLoading: false, accountID: 0, avatar: ''}); + const details: PersonalDetails | EmptyObject = personalDetails?.[accountID] ?? (ValidationUtils.isValidAccountRoute(accountID) ? {} : {accountID: 0, avatar: ''}); const displayName = PersonalDetailsUtils.getDisplayNameOrDefault(details, undefined, undefined, isCurrentUser); // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing @@ -102,7 +105,7 @@ function ProfilePage({personalDetails, route, session, report}: ProfilePageProps const phoneOrEmail = isSMSLogin ? getPhoneNumber(details) : login; const hasMinimumDetails = !isEmptyObject(details.avatar); - const isLoading = Boolean(details?.isLoading) || isEmptyObject(details); + const isLoading = Boolean(personalDetailsMetadata?.[accountID]?.isLoading) || isEmptyObject(details); // If the API returns an error for some reason there won't be any details and isLoading will get set to false, so we want to show a blocking screen const shouldShowBlockingView = !hasMinimumDetails && !isLoading; @@ -265,6 +268,9 @@ export default withOnyx({ personalDetails: { key: ONYXKEYS.PERSONAL_DETAILS_LIST, }, + personalDetailsMetadata: { + key: ONYXKEYS.PERSONAL_DETAILS_METADATA, + }, session: { key: ONYXKEYS.SESSION, }, diff --git a/src/pages/settings/Profile/ProfileAvatar.tsx b/src/pages/settings/Profile/ProfileAvatar.tsx index 450740d09da4..4925a6948fef 100644 --- a/src/pages/settings/Profile/ProfileAvatar.tsx +++ b/src/pages/settings/Profile/ProfileAvatar.tsx @@ -11,20 +11,21 @@ import * as ValidationUtils from '@libs/ValidationUtils'; import * as PersonalDetails from '@userActions/PersonalDetails'; import ONYXKEYS from '@src/ONYXKEYS'; import type SCREENS from '@src/SCREENS'; -import type {PersonalDetailsList} from '@src/types/onyx'; +import type {PersonalDetailsList, PersonalDetailsMetadata} from '@src/types/onyx'; type ProfileAvatarOnyxProps = { personalDetails: OnyxEntry; + personalDetailsMetadata: OnyxEntry>; isLoadingApp: OnyxEntry; }; type ProfileAvatarProps = ProfileAvatarOnyxProps & StackScreenProps; -function ProfileAvatar({route, personalDetails, isLoadingApp = true}: ProfileAvatarProps) { +function ProfileAvatar({route, personalDetails, personalDetailsMetadata, isLoadingApp = true}: ProfileAvatarProps) { const personalDetail = personalDetails?.[route.params.accountID]; const avatarURL = personalDetail?.avatar ?? ''; const accountID = Number(route.params.accountID ?? ''); - const isLoading = personalDetail?.isLoading ?? (isLoadingApp && !Object.keys(personalDetail ?? {}).length); + const isLoading = personalDetailsMetadata?.[accountID]?.isLoading ?? (isLoadingApp && !Object.keys(personalDetail ?? {}).length); const displayName = PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail); useEffect(() => { @@ -55,6 +56,9 @@ export default withOnyx({ personalDetails: { key: ONYXKEYS.PERSONAL_DETAILS_LIST, }, + personalDetailsMetadata: { + key: ONYXKEYS.PERSONAL_DETAILS_METADATA, + }, isLoadingApp: { key: ONYXKEYS.IS_LOADING_APP, }, diff --git a/src/types/onyx/PersonalDetails.ts b/src/types/onyx/PersonalDetails.ts index 42482f9104dc..69a0296f0c7f 100644 --- a/src/types/onyx/PersonalDetails.ts +++ b/src/types/onyx/PersonalDetails.ts @@ -68,9 +68,6 @@ type PersonalDetails = OnyxCommon.OnyxValueWithOfflineFeedback<{ /** Flag for checking if data is from optimistic data */ isOptimisticPersonalDetail?: boolean; - /** Whether we are loading the data via the API */ - isLoading?: boolean; - /** Field-specific server side errors keyed by microtime */ errorFields?: OnyxCommon.ErrorFields<'avatar'>; @@ -81,8 +78,13 @@ type PersonalDetails = OnyxCommon.OnyxValueWithOfflineFeedback<{ status?: Status; }>; +type PersonalDetailsMetadata = { + /** Whether we are loading the data via the API */ + isLoading?: boolean; +}; + type PersonalDetailsList = Record; export default PersonalDetails; -export type {Timezone, Status, SelectedTimezone, PersonalDetailsList}; +export type {Timezone, Status, SelectedTimezone, PersonalDetailsList, PersonalDetailsMetadata}; diff --git a/src/types/onyx/index.ts b/src/types/onyx/index.ts index de40dd4cf02f..5f0ee694646b 100644 --- a/src/types/onyx/index.ts +++ b/src/types/onyx/index.ts @@ -30,7 +30,7 @@ import type Network from './Network'; import type {OnyxUpdateEvent, OnyxUpdatesFromServer} from './OnyxUpdatesFromServer'; import type {DecisionName, OriginalMessageIOU} from './OriginalMessage'; import type PersonalBankAccount from './PersonalBankAccount'; -import type {PersonalDetailsList} from './PersonalDetails'; +import type {PersonalDetailsList, PersonalDetailsMetadata} from './PersonalDetails'; import type PersonalDetails from './PersonalDetails'; import type PlaidData from './PlaidData'; import type Policy from './Policy'; @@ -109,6 +109,7 @@ export type { PersonalBankAccount, PersonalDetails, PersonalDetailsList, + PersonalDetailsMetadata, PlaidData, Policy, PolicyCategories, From 86782414b54aa308235b526f01abfb6dc98b0424 Mon Sep 17 00:00:00 2001 From: Tienifr <113963320+tienifr@users.noreply.github.com> Date: Mon, 25 Mar 2024 09:30:20 +0700 Subject: [PATCH 2/5] Modify metadata comment Co-authored-by: c3024 <102477862+c3024@users.noreply.github.com> --- src/ONYXKEYS.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index 252e81ce707a..62eac44998ba 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -60,7 +60,7 @@ const ONYXKEYS = { /** Contains all the private personal details of the user */ PRIVATE_PERSONAL_DETAILS: 'private_personalDetails', - /** REPORT_METADATA is a perf optimization used to hold loading states. See REPORT_METADATA. */ + /** PERSONAL_DETAILS_METADATA is a perf optimization used to hold loading states. For more explanation, see REPORT_METADATA. */ PERSONAL_DETAILS_METADATA: 'personalDetailsMetadata', /** Contains all the info for Tasks */ From f09167e904cccc5836ba50e14d3adbad3f4b1221 Mon Sep 17 00:00:00 2001 From: Tienifr <113963320+tienifr@users.noreply.github.com> Date: Mon, 25 Mar 2024 10:59:07 +0700 Subject: [PATCH 3/5] Modify comment Co-authored-by: Puneet Lath --- src/types/onyx/PersonalDetails.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/onyx/PersonalDetails.ts b/src/types/onyx/PersonalDetails.ts index 69a0296f0c7f..b7c96998080c 100644 --- a/src/types/onyx/PersonalDetails.ts +++ b/src/types/onyx/PersonalDetails.ts @@ -79,7 +79,7 @@ type PersonalDetails = OnyxCommon.OnyxValueWithOfflineFeedback<{ }>; type PersonalDetailsMetadata = { - /** Whether we are loading the data via the API */ + /** Whether we are waiting for the data to load via the API */ isLoading?: boolean; }; From 17f18e7c8dff7339eba94a284cafc7d11b2195de Mon Sep 17 00:00:00 2001 From: tienifr Date: Tue, 26 Mar 2024 18:13:50 +0700 Subject: [PATCH 4/5] Modify comment for personalDetailsMetadata --- src/ONYXKEYS.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index 62eac44998ba..d9d65c7f5b71 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -60,7 +60,11 @@ const ONYXKEYS = { /** Contains all the private personal details of the user */ PRIVATE_PERSONAL_DETAILS: 'private_personalDetails', - /** PERSONAL_DETAILS_METADATA is a perf optimization used to hold loading states. For more explanation, see REPORT_METADATA. */ + /** + * PERSONAL_DETAILS_METADATA is a perf optimization used to hold loading states of each entry in PERSONAL_DETAILS_LIST. + * A lot of components are connected to the PERSONAL_DETAILS_LIST entity and do not care about the actions. + * Setting the loading state directly on the personal details entry caused a lot of unnecessary re-renders. + */ PERSONAL_DETAILS_METADATA: 'personalDetailsMetadata', /** Contains all the info for Tasks */ From 61728497f4f2b595e8698ec8592e54d4bfee42a5 Mon Sep 17 00:00:00 2001 From: tienifr Date: Thu, 28 Mar 2024 11:29:31 +0700 Subject: [PATCH 5/5] modify comment --- src/ONYXKEYS.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index d9d65c7f5b71..9abd22ed5d14 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -62,7 +62,7 @@ const ONYXKEYS = { /** * PERSONAL_DETAILS_METADATA is a perf optimization used to hold loading states of each entry in PERSONAL_DETAILS_LIST. - * A lot of components are connected to the PERSONAL_DETAILS_LIST entity and do not care about the actions. + * A lot of components are connected to the PERSONAL_DETAILS_LIST entity and do not care about the loading state. * Setting the loading state directly on the personal details entry caused a lot of unnecessary re-renders. */ PERSONAL_DETAILS_METADATA: 'personalDetailsMetadata',