Skip to content
Merged
1 change: 1 addition & 0 deletions src/hooks/useArchivedReportsIdSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ function useArchivedReportsIdSet(): ArchivedReportsIDSet {
// useDeepCompareRef is used here to prevent unnecessary re-renders by maintaining referential equality
// when the Set contents are the same, even if it's a new Set instance. This is important for performance
// optimization since Sets are reference types and would normally cause re-renders even with same values
// Reassure test confirmed additional rerender when not using useDeepCompareRef
Copy link
Contributor

@dominictb dominictb Feb 1, 2026

Choose a reason for hiding this comment

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

Thanks! I see that useDeepCompareRef hook is also being used in these places:

New usage (I think this is similar to useArchivedReportsIdSet, we must use useDeepCompareRef, but please help to review it yourself):

function usePrivateIsArchivedMap(): PrivateIsArchivedMap {

You already mentioned this above:

const deepComparedReportsToDisplayInLHN = useDeepCompareRef(reportsToDisplayInLHN);
const deepComparedReportsDrafts = useDeepCompareRef(reportsDrafts);

Please add documentation for these usages as well.

Copy link
Contributor Author

@szymonzalarski98 szymonzalarski98 Feb 6, 2026

Choose a reason for hiding this comment

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

I've added comments to all useDeepCompareRef usages explaining why they can't easily be removed:

  1. src/hooks/usePrivateIsArchivedMap.ts The selector always returns a new object instance, and useMemo cannot compare by value

  2. src/hooks/useSidebarOrderedReports.tsx Both values have new references despite same content; without this, getOrderedReportIDs would be recreated on every render, causing expensive orderedReportIDs recalculation (the sortReportsToDisplayInLHN operation is a known performance bottleneck)

return useDeepCompareRef(archivedReportsIdSet) ?? new Set<string>();
}

Expand Down
18 changes: 18 additions & 0 deletions src/hooks/useDeepCompareRef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import {deepEqual} from 'fast-equals';
import {useRef} from 'react';

/**
* ⚠️ **WARNING: ANTI-PATTERN - AVOID IN NEW CODE** ⚠️
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

you can mark it as @deprecated and then using it will result in lint warnings.

*
* This hook returns a reference to the provided value,
* but only updates that reference if a deep comparison indicates that the value has changed.
*
Expand All @@ -14,6 +16,22 @@ import {useRef} from 'react';
* useEffect(() => {
* // This will run not just when myArray is a new array, but also when its contents change.
* }, [deepComparedArray]);
*
* **Why this is problematic:**
* - Violates React's exhaustive deps rule (can cause stale closures)
* - Performance overhead from deep equality checks on every render
* - Incompatible with React Compiler optimizations
* - Usually indicates a data flow problem that should be fixed at the source
*
* **Use instead:**
* - `useMemo` with primitive dependencies
* - Fix selectors/data sources to return stable references
*
* **Only use when ALL of these apply:**
* - Legacy infrastructure forces new references (e.g., Onyx collections)
* - Documented why it's necessary and what the risks are
* - No feasible alternative without major refactoring
* - Performance impact measured with tests (e.g., Reassure)
*/
export default function useDeepCompareRef<T>(value: T): T | undefined {
const ref = useRef<T | undefined>(undefined);
Expand Down
2 changes: 2 additions & 0 deletions src/hooks/usePrivateIsArchivedMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ function usePrivateIsArchivedMap(): PrivateIsArchivedMap {
selector: privateIsArchivedMapSelector,
});

// useDeepCompareRef prevents unnecessary rerenders when the object has same content but different reference.
// The selector always returns a new object instance, and useMemo cannot compare by value.
return useDeepCompareRef(privateIsArchivedMap) ?? {};
}

Expand Down
2 changes: 2 additions & 0 deletions src/hooks/useSidebarOrderedReports.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ function SidebarOrderedReportsContextProvider({
clearCacheDummyCounter,
]);

// useDeepCompareRef prevents unnecessary useCallback recreations when these have same content but different reference.
// Without this, getOrderedReportIDs would be recreated on every render, causing expensive orderedReportIDs recalculation.
const deepComparedReportsToDisplayInLHN = useDeepCompareRef(reportsToDisplayInLHN);
const deepComparedReportsDrafts = useDeepCompareRef(reportsDrafts);

Expand Down
7 changes: 2 additions & 5 deletions src/pages/inbox/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import useShowWideRHPVersion from '@components/WideRHPContextProvider/useShowWid
import WideRHPOverlayWrapper from '@components/WideRHPOverlayWrapper';
import useAppFocusEvent from '@hooks/useAppFocusEvent';
import {useCurrentReportIDState} from '@hooks/useCurrentReportID';
import useDeepCompareRef from '@hooks/useDeepCompareRef';
import useIsAnonymousUser from '@hooks/useIsAnonymousUser';
import useIsReportReadyToDisplay from '@hooks/useIsReportReadyToDisplay';
import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
Expand Down Expand Up @@ -192,8 +191,6 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr
const deletedParentAction = isDeletedParentAction(parentReportAction);
const prevDeletedParentAction = usePrevious(deletedParentAction);

const permissions = useDeepCompareRef(reportOnyx?.permissions);

const isAnonymousUser = useIsAnonymousUser();
const [isLoadingReportData = true] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA, {canBeMissing: true});
const prevIsLoadingReportData = usePrevious(isLoadingReportData);
Expand Down Expand Up @@ -287,12 +284,12 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr
private_isArchived: reportNameValuePairsOnyx?.private_isArchived,
lastMentionedTime: reportOnyx.lastMentionedTime,
avatarUrl: reportOnyx.avatarUrl,
permissions,
permissions: reportOnyx?.permissions,
invoiceReceiver: reportOnyx.invoiceReceiver,
policyAvatar: reportOnyx.policyAvatar,
nextStep: reportOnyx.nextStep,
},
[reportOnyx, reportNameValuePairsOnyx?.private_isArchived, permissions],
[reportOnyx, reportNameValuePairsOnyx?.private_isArchived],
);
const reportID = report?.reportID;

Expand Down
9 changes: 1 addition & 8 deletions src/pages/iou/request/step/IOURequestStepConfirmation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import PrevNextButtons from '@components/PrevNextButtons';
import ScreenWrapper from '@components/ScreenWrapper';
import useConfirmModal from '@hooks/useConfirmModal';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useDeepCompareRef from '@hooks/useDeepCompareRef';
import useFetchRoute from '@hooks/useFetchRoute';
import useFilesValidation from '@hooks/useFilesValidation';
import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
Expand Down Expand Up @@ -146,12 +145,6 @@ function IOURequestStepConfirmation({
() => (!isLoadingCurrentTransaction ? (optimisticTransaction ?? existingTransaction) : undefined),
[existingTransaction, optimisticTransaction, isLoadingCurrentTransaction],
);
const transactionsCategories = useDeepCompareRef(
transactions.map(({transactionID, category}) => ({
transactionID,
category,
})),
);
const isUnreported = transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
const isCreatingTrackExpense = action === CONST.IOU.ACTION.CREATE && iouType === CONST.IOU.TYPE.TRACK;
const {policyForMovingExpenses, policyForMovingExpensesID} = usePolicyForMovingExpenses();
Expand Down Expand Up @@ -398,7 +391,7 @@ function IOURequestStepConfirmation({
}
// We don't want to clear out category every time the transactions change
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [policy?.id, policyCategories, transactionsCategories]);
}, [policy?.id, policyCategories, transactions.length]);

const policyDistance = Object.values(policy?.customUnits ?? {}).find((customUnit) => customUnit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE);
const defaultCategory = policyDistance?.defaultCategory ?? '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type {SelectionListApprover} from '@components/ApproverSelectionList';
import ApproverSelectionList from '@components/ApproverSelectionList';
import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton';
import Text from '@components/Text';
import useDeepCompareRef from '@hooks/useDeepCompareRef';
import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
Expand Down Expand Up @@ -33,7 +32,6 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat
const styles = useThemeStyles();
const {translate} = useLocalize();
const [approvalWorkflow, approvalWorkflowResults] = useOnyx(ONYXKEYS.APPROVAL_WORKFLOW, {canBeMissing: true});
const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {canBeMissing: false});
const icons = useMemoizedLazyExpensifyIcons(['FallbackAvatar']);

const isLoadingApprovalWorkflow = isLoadingOnyxValue(approvalWorkflowResults);
Expand All @@ -45,8 +43,6 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat
const shouldShowListEmptyContent = !isLoadingApprovalWorkflow && approvalWorkflow && approvalWorkflow.availableMembers.length === 0;
const firstApprover = approvalWorkflow?.originalApprovers?.[0]?.email ?? '';

const personalDetailLogins = useDeepCompareRef(Object.fromEntries(Object.entries(personalDetails ?? {}).map(([id, details]) => [id, details?.login])));

useEffect(() => {
if (!approvalWorkflow?.members) {
return;
Expand All @@ -56,7 +52,6 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat
approvalWorkflow.members.map((member) => {
const policyMemberEmailsToAccountIDs = getMemberAccountIDsForWorkspace(policy?.employeeList);
const accountID = Number(policyMemberEmailsToAccountIDs[member.email] ?? '');
const login = personalDetailLogins?.[accountID];

return {
text: Str.removeSMSDomain(member.displayName),
Expand All @@ -69,13 +64,13 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat
<MemberRightIcon
role={policy?.employeeList?.[member.email]?.role}
owner={policy?.owner}
login={login}
login={member.email}
/>
),
};
}),
);
}, [approvalWorkflow?.members, policy?.employeeList, policy?.owner, personalDetailLogins, translate, icons.FallbackAvatar]);
}, [approvalWorkflow?.members, policy?.employeeList, policy?.owner, translate, icons.FallbackAvatar]);

const approversEmail = approvalWorkflow?.approvers.map((member) => member?.email);
const allApprovers: SelectionListApprover[] = [...selectedMembers];
Expand All @@ -85,7 +80,6 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat
.map((member) => {
const policyMemberEmailsToAccountIDs = getMemberAccountIDsForWorkspace(policy?.employeeList);
const accountID = Number(policyMemberEmailsToAccountIDs[member.email] ?? '');
const login = personalDetailLogins?.[accountID];

return {
text: Str.removeSMSDomain(member.displayName),
Expand All @@ -98,7 +92,7 @@ function WorkspaceWorkflowsApprovalsExpensesFromPage({policy, isLoadingReportDat
<MemberRightIcon
role={policy?.employeeList?.[member.email]?.role}
owner={policy?.owner}
login={login}
login={member.email}
/>
),
};
Expand Down
Loading