Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate sidebarLinkData to useOnyx #52550

Merged
merged 4 commits into from
Nov 25, 2024
Merged

migrate sidebarLinkData to useOnyx #52550

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2fa4765
migrate sidebarLinkData to useOnyx
Nodebrute Nov 14, 2024
51ab5ea
fix lint errors
Nodebrute Nov 14, 2024
74d1486
Merge branch 'Expensify:main' into migrateUseOnyx
Nodebrute Nov 19, 2024
4292b3a
remove unused vars
Nodebrute Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 7 additions & 34 deletions src/pages/home/sidebar/SidebarLinksData.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import {useIsFocused} from '@react-navigation/native';
import lodashIsEqual from 'lodash/isEqual';
import React, {memo, useCallback, useEffect, useRef} from 'react';
import React, {useCallback, useEffect, useRef} from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import type {EdgeInsets} from 'react-native-safe-area-context';
import type {ValueOf} from 'type-fest';
import useActiveWorkspaceFromNavigationState from '@hooks/useActiveWorkspaceFromNavigationState';
import useLocalize from '@hooks/useLocalize';
import {useReportIDs} from '@hooks/useReportIDs';
Expand All @@ -15,24 +12,18 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import SidebarLinks from './SidebarLinks';

type SidebarLinksDataOnyxProps = {
/** Whether the reports are loading. When false it means they are ready to be used. */
isLoadingApp: OnyxEntry<boolean>;

/** The chat priority mode */
priorityMode: OnyxEntry<ValueOf<typeof CONST.PRIORITY_MODE>>;
};

type SidebarLinksDataProps = SidebarLinksDataOnyxProps & {
type SidebarLinksDataProps = {
/** Safe area insets required for mobile devices margins */
insets: EdgeInsets;
};

function SidebarLinksData({insets, isLoadingApp = true, priorityMode = CONST.PRIORITY_MODE.DEFAULT}: SidebarLinksDataProps) {
function SidebarLinksData({insets}: SidebarLinksDataProps) {
const isFocused = useIsFocused();
const styles = useThemeStyles();
const activeWorkspaceID = useActiveWorkspaceFromNavigationState();
const {translate} = useLocalize();
const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {initialValue: true});
const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, {initialValue: CONST.PRIORITY_MODE.DEFAULT});

const {orderedReportIDs, currentReportID, policyMemberAccountIDs} = useReportIDs();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: I noticed this constant assignment in this component which is redundant as it was leftover from another PRs commit 1b0738a back when we used to have 2 checks, but now that we only have 1 (isLoadingApp) there's no point in re-assigning it to a constant.

- const isLoading = isLoadingApp;

- isLoading={isLoadingApp ?? false}
+ isLoading={isLoadingApp ?? false}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed this. I'll update it shortly.

Expand Down Expand Up @@ -74,22 +65,4 @@ function SidebarLinksData({insets, isLoadingApp = true, priorityMode = CONST.PRI

SidebarLinksData.displayName = 'SidebarLinksData';

export default withOnyx<SidebarLinksDataProps, SidebarLinksDataOnyxProps>({
isLoadingApp: {
key: ONYXKEYS.IS_LOADING_APP,
},
priorityMode: {
key: ONYXKEYS.NVP_PRIORITY_MODE,
initialValue: CONST.PRIORITY_MODE.DEFAULT,
},
})(
/*
While working on audit on the App Start App metric we noticed that by memoizing SidebarLinksData we can avoid 2 additional run of getOrderedReportIDs.
With that we can reduce app start up time by ~2s on heavy account.
More details - https://github.com/Expensify/App/issues/35234#issuecomment-1926914534
*/
memo(
SidebarLinksData,
(prevProps, nextProps) => prevProps.isLoadingApp === nextProps.isLoadingApp && prevProps.priorityMode === nextProps.priorityMode && lodashIsEqual(prevProps.insets, nextProps.insets),
),
);
export default SidebarLinksData;
Loading