From 288bd8c8b99fab912122a863449cee41ef4a639a Mon Sep 17 00:00:00 2001 From: tienifr Date: Wed, 20 Mar 2024 00:14:50 +0700 Subject: [PATCH 1/3] fix: read api is not triggered consistently when switch between features of collect workspace --- src/pages/workspace/WorkspaceMembersPage.tsx | 11 ++++++----- .../workspace/WorkspaceMoreFeaturesPage.tsx | 16 +++++++++------- .../workspace/WorkspacePageWithSections.tsx | 12 +++++++----- .../categories/WorkspaceCategoriesPage.tsx | 16 +++++++++------- .../distanceRates/PolicyDistanceRatesPage.tsx | 16 +++++++++------- src/pages/workspace/tags/WorkspaceTagsPage.tsx | 16 +++++++++------- src/pages/workspace/taxes/WorkspaceTaxesPage.tsx | 16 +++++++++------- .../workflows/WorkspaceWorkflowsPage.tsx | 16 +++++++++------- 8 files changed, 67 insertions(+), 52 deletions(-) diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index cebcc38aeae4..d37d76d4216b 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -1,4 +1,4 @@ -import {useIsFocused} from '@react-navigation/native'; +import {useFocusEffect, useIsFocused} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; import lodashIsEqual from 'lodash/isEqual'; import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; @@ -144,10 +144,11 @@ function WorkspaceMembersPage({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedEmployees, policy?.owner, session?.accountID]); - useEffect(() => { - getWorkspaceMembers(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + useFocusEffect( + useCallback(() => { + getWorkspaceMembers(); + }, [getWorkspaceMembers]), + ); useEffect(() => { validateSelection(); diff --git a/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx b/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx index 2c8123670e0b..2df5adc78cbd 100644 --- a/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx +++ b/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx @@ -1,5 +1,6 @@ +import {useFocusEffect} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; -import React, {useCallback, useEffect} from 'react'; +import React, {useCallback} from 'react'; import {View} from 'react-native'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; import * as Illustrations from '@components/Icon/Illustrations'; @@ -152,16 +153,17 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro [isSmallScreenWidth, styles, renderItem, translate], ); - function fetchFeatures() { + const fetchFeatures = useCallback(() => { Policy.openPolicyMoreFeaturesPage(route.params.policyID); - } + }, [route.params.policyID]); useNetwork({onReconnect: fetchFeatures}); - useEffect(() => { - fetchFeatures(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + useFocusEffect( + useCallback(() => { + fetchFeatures(); + }, [fetchFeatures]), + ); return ( diff --git a/src/pages/workspace/WorkspacePageWithSections.tsx b/src/pages/workspace/WorkspacePageWithSections.tsx index 4904a4f35193..ad3891b9e4ed 100644 --- a/src/pages/workspace/WorkspacePageWithSections.tsx +++ b/src/pages/workspace/WorkspacePageWithSections.tsx @@ -1,6 +1,6 @@ -import {useIsFocused} from '@react-navigation/native'; +import {useFocusEffect, useIsFocused} from '@react-navigation/native'; import type {ReactNode} from 'react'; -import React, {useEffect, useMemo, useRef} from 'react'; +import React, {useCallback, useEffect, useMemo, useRef} from 'react'; import {View} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import {withOnyx} from 'react-native-onyx'; @@ -133,9 +133,11 @@ function WorkspacePageWithSections({ firstRender.current = false; }, []); - useEffect(() => { - fetchData(policyID, shouldSkipVBBACall); - }, [policyID, shouldSkipVBBACall]); + useFocusEffect( + useCallback(() => { + fetchData(policyID, shouldSkipVBBACall); + }, [policyID, shouldSkipVBBACall]), + ); const shouldShow = useMemo(() => { // If the policy object doesn't exist or contains only error data, we shouldn't display it. diff --git a/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx b/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx index 74b439004020..50d6756244ac 100644 --- a/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx +++ b/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx @@ -1,5 +1,6 @@ +import {useFocusEffect} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; -import React, {useEffect, useMemo, useRef, useState} from 'react'; +import React, {useCallback, useMemo, useRef, useState} from 'react'; import {ActivityIndicator, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import type {OnyxEntry} from 'react-native-onyx'; @@ -61,16 +62,17 @@ function WorkspaceCategoriesPage({policy, policyCategories, route}: WorkspaceCat const dropdownButtonRef = useRef(null); const [deleteCategoriesConfirmModalVisible, setDeleteCategoriesConfirmModalVisible] = useState(false); - function fetchCategories() { + const fetchCategories = useCallback(() => { Policy.openPolicyCategoriesPage(route.params.policyID); - } + }, [route.params.policyID]); const {isOffline} = useNetwork({onReconnect: fetchCategories}); - useEffect(() => { - fetchCategories(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + useFocusEffect( + useCallback(() => { + fetchCategories(); + }, [fetchCategories]), + ); const categoryList = useMemo( () => diff --git a/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx b/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx index b29a3cd9a9b5..b10b278678ec 100644 --- a/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx +++ b/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx @@ -1,5 +1,6 @@ +import {useFocusEffect} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {useCallback, useMemo, useRef, useState} from 'react'; import {ActivityIndicator, View} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import {withOnyx} from 'react-native-onyx'; @@ -59,9 +60,9 @@ function PolicyDistanceRatesPage({policy, route}: PolicyDistanceRatesPageProps) ); const customUnitRates: Record = useMemo(() => customUnit?.rates ?? {}, [customUnit]); - function fetchDistanceRates() { + const fetchDistanceRates = useCallback(() => { Policy.openPolicyDistanceRatesPage(policyID); - } + }, [policyID]); const dismissError = useCallback( (item: RateForList) => { @@ -72,10 +73,11 @@ function PolicyDistanceRatesPage({policy, route}: PolicyDistanceRatesPageProps) const {isOffline} = useNetwork({onReconnect: fetchDistanceRates}); - useEffect(() => { - fetchDistanceRates(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + useFocusEffect( + useCallback(() => { + fetchDistanceRates(); + }, [fetchDistanceRates]), + ); const distanceRatesList = useMemo( () => diff --git a/src/pages/workspace/tags/WorkspaceTagsPage.tsx b/src/pages/workspace/tags/WorkspaceTagsPage.tsx index 4fa84b5c4977..5e284468a569 100644 --- a/src/pages/workspace/tags/WorkspaceTagsPage.tsx +++ b/src/pages/workspace/tags/WorkspaceTagsPage.tsx @@ -1,5 +1,6 @@ +import {useFocusEffect} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; -import React, {useEffect, useMemo, useState} from 'react'; +import React, {useCallback, useMemo, useState} from 'react'; import {ActivityIndicator, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import type {OnyxEntry} from 'react-native-onyx'; @@ -58,16 +59,17 @@ function WorkspaceTagsPage({policyTags, route}: WorkspaceTagsPageProps) { const {translate} = useLocalize(); const [selectedTags, setSelectedTags] = useState>({}); - function fetchTags() { + const fetchTags = useCallback(() => { Policy.openPolicyTagsPage(route.params.policyID); - } + }, [route.params.policyID]); const {isOffline} = useNetwork({onReconnect: fetchTags}); - useEffect(() => { - fetchTags(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + useFocusEffect( + useCallback(() => { + fetchTags(); + }, [fetchTags]), + ); const policyTagLists = useMemo(() => PolicyUtils.getTagLists(policyTags), [policyTags]); const tagList = useMemo( diff --git a/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx b/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx index 8eb730c0134f..28b27ab3dbb7 100644 --- a/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx +++ b/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx @@ -1,5 +1,6 @@ +import {useFocusEffect} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; -import React, {useCallback, useEffect, useMemo, useState} from 'react'; +import React, {useCallback, useMemo, useState} from 'react'; import {ActivityIndicator, View} from 'react-native'; import Button from '@components/Button'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; @@ -39,16 +40,17 @@ function WorkspaceTaxesPage({policy, route}: WorkspaceTaxesPageProps) { const defaultExternalID = policy?.taxRates?.defaultExternalID; const foreignTaxDefault = policy?.taxRates?.foreignTaxDefault; - const fetchTaxes = () => { + const fetchTaxes = useCallback(() => { openPolicyTaxesPage(route.params.policyID); - }; + }, [route.params.policyID]); const {isOffline} = useNetwork({onReconnect: fetchTaxes}); - useEffect(() => { - fetchTaxes(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + useFocusEffect( + useCallback(() => { + fetchTaxes(); + }, [fetchTaxes]), + ); const textForDefault = useCallback( (taxID: string): string => { diff --git a/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx b/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx index c6ace2b0856e..90bbdcf773fd 100644 --- a/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx +++ b/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx @@ -1,5 +1,6 @@ +import {useFocusEffect} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; -import React, {useCallback, useEffect, useMemo} from 'react'; +import React, {useCallback, useMemo} from 'react'; import {FlatList, View} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import {withOnyx} from 'react-native-onyx'; @@ -62,16 +63,17 @@ function WorkspaceWorkflowsPage({policy, betas, route, reimbursementAccount, ses const onPressAutoReportingFrequency = useCallback(() => Navigation.navigate(ROUTES.WORKSPACE_WORKFLOWS_AUTOREPORTING_FREQUENCY.getRoute(policy?.id ?? '')), [policy?.id]); - const fetchData = () => { + const fetchData = useCallback(() => { Policy.openPolicyWorkflowsPage(policy?.id ?? route.params.policyID); - }; + }, [policy?.id, route.params.policyID]); useNetwork({onReconnect: fetchData}); - useEffect(() => { - fetchData(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + useFocusEffect( + useCallback(() => { + fetchData(); + }, [fetchData]), + ); const optionItems: ToggleSettingOptionRowProps[] = useMemo(() => { const {accountNumber, state, bankName} = reimbursementAccount?.achData ?? {}; From bbbb8e6e7aeed2365562c7f14dab6e6c6dd895d8 Mon Sep 17 00:00:00 2001 From: tienifr Date: Mon, 25 Mar 2024 15:48:22 +0700 Subject: [PATCH 2/3] fix: getWorkspaceMembers twice after logging in --- src/pages/workspace/WorkspaceMembersPage.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index d37d76d4216b..49400522d97d 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -1,4 +1,4 @@ -import {useFocusEffect, useIsFocused} from '@react-navigation/native'; +import {useIsFocused} from '@react-navigation/native'; import type {StackScreenProps} from '@react-navigation/stack'; import lodashIsEqual from 'lodash/isEqual'; import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; @@ -144,11 +144,13 @@ function WorkspaceMembersPage({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedEmployees, policy?.owner, session?.accountID]); - useFocusEffect( - useCallback(() => { - getWorkspaceMembers(); - }, [getWorkspaceMembers]), - ); + useEffect(() => { + if (!isFocused) { + return; + } + getWorkspaceMembers(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isFocused]); useEffect(() => { validateSelection(); From ac6bb01a42138143ba577464e0d2e3ec8427a081 Mon Sep 17 00:00:00 2001 From: tienifr Date: Wed, 3 Apr 2024 20:27:16 +0700 Subject: [PATCH 3/3] Add comment for not using useFocusEffect --- src/pages/workspace/WorkspaceMembersPage.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/workspace/WorkspaceMembersPage.tsx b/src/pages/workspace/WorkspaceMembersPage.tsx index 3a876cb84a0d..9f210cb6d2ac 100644 --- a/src/pages/workspace/WorkspaceMembersPage.tsx +++ b/src/pages/workspace/WorkspaceMembersPage.tsx @@ -146,6 +146,7 @@ function WorkspaceMembersPage({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedEmployees, policy?.owner, session?.accountID]); + // useFocusEffect would make getWorkspaceMembers get called twice on fresh login because policyMember is a dependency of getWorkspaceMembers. useEffect(() => { if (!isFocused) { return;