From 18482b0fd42da6285b1177a8d582d220c1f13f56 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 7 Jun 2024 11:43:24 -0400 Subject: [PATCH 01/12] prevent OpenReport while app is loading --- src/pages/home/ReportScreen.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 414745270b68..7d0f71d23a11 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -155,6 +155,7 @@ function ReportScreen({ const isLoadingReportOnyx = isLoadingOnyxValue(reportResult); const permissions = useDeepCompareRef(reportOnyx?.permissions); + const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {initialValue: true}); /** * Create a lightweight Report so as to keep the re-rendering as light as possible by @@ -423,12 +424,21 @@ function ReportScreen({ return; } + /** + * Since OpenReport is a write, the response from OpenReport will get dropped while the app is + * still loading. This usually happens when signing in and deeplinking to a report. Instead, + * we'll fetch the report after the app finishes loading in an effect below + */ + if (isLoadingApp) { + return; + } + if (!shouldFetchReport(report)) { return; } fetchReport(); - }, [report, fetchReport, reportIDFromRoute]); + }, [report, fetchReport, reportIDFromRoute, isLoadingApp]); const dismissBanner = useCallback(() => { setIsBannerVisible(false); From f29e5838b8e3b09ac977a9a3cec9dff5b7d6bcf2 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 7 Jun 2024 11:43:51 -0400 Subject: [PATCH 02/12] show skeleton while app is loading --- src/pages/home/ReportScreen.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 7d0f71d23a11..f32ad6011880 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -377,7 +377,7 @@ function ReportScreen({ return reportIDFromRoute !== '' && !!report.reportID && !isTransitioning; }, [report, reportIDFromRoute]); - const isLoading = !reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty(); + const isLoading = isLoadingApp || !reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty(); const shouldShowSkeleton = !isLinkedMessageAvailable && (isLinkingToMessage || @@ -390,10 +390,10 @@ function ReportScreen({ // eslint-disable-next-line rulesdir/no-negated-variables const shouldShowNotFoundPage = useMemo( (): boolean => - (!wasReportAccessibleRef.current && !firstRenderRef.current && !report.reportID && !isOptimisticDelete && !reportMetadata?.isLoadingInitialReportActions && !userLeavingStatus) || + (!isLoadingApp && !wasReportAccessibleRef.current && !firstRenderRef.current && !report.reportID && !isOptimisticDelete && !reportMetadata?.isLoadingInitialReportActions && !userLeavingStatus) || shouldHideReport || (!!currentReportIDFormRoute && !ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute)), - [report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute], + [report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute, isLoadingApp], ); const fetchReport = useCallback(() => { From 04102d9f7d4fb1111f97fdc9d0e1b8ad480cbb36 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 7 Jun 2024 11:44:30 -0400 Subject: [PATCH 03/12] fetch report after the app is done loading --- src/pages/home/ReportScreen.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index f32ad6011880..5f0024117c2d 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -156,6 +156,7 @@ function ReportScreen({ const isLoadingReportOnyx = isLoadingOnyxValue(reportResult); const permissions = useDeepCompareRef(reportOnyx?.permissions); const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {initialValue: true}); + const wasLoadingApp = usePrevious(isLoadingApp); /** * Create a lightweight Report so as to keep the re-rendering as light as possible by @@ -623,6 +624,14 @@ function ReportScreen({ }); }, [reportMetadata?.isLoadingInitialReportActions]); + // If we deeplinked to the report after signing in, we need to fetch the report after the app is done loading + useEffect(() => { + const finishedLoadingApp = wasLoadingApp && !isLoadingApp; + if (finishedLoadingApp) { + fetchReportIfNeeded(); + } + }, [wasLoadingApp, isLoadingApp, fetchReportIfNeeded]) + const navigateToEndOfReport = useCallback(() => { Navigation.setParams({reportActionID: ''}); fetchReport(); From 800a624991aa138504b4e7dc18fc7863adf44d27 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 7 Jun 2024 12:36:49 -0400 Subject: [PATCH 04/12] prevent double report loading --- src/pages/home/ReportScreen.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 5f0024117c2d..4026c116252e 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -157,6 +157,8 @@ function ReportScreen({ const permissions = useDeepCompareRef(reportOnyx?.permissions); const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {initialValue: true}); const wasLoadingApp = usePrevious(isLoadingApp); + const prevMetadata = usePrevious(reportMetadata); + const finishedLoadingReport = prevMetadata?.isLoadingInitialReportActions && !reportMetadata?.isLoadingInitialReportActions; /** * Create a lightweight Report so as to keep the re-rendering as light as possible by @@ -559,6 +561,11 @@ function ReportScreen({ return; } + // If we just finished loading the report, we don't need to load it again + if (finishedLoadingReport) { + return; + } + fetchReportIfNeeded(); ComposerActions.setShouldShowComposeInput(true); }, [ @@ -575,6 +582,7 @@ function ReportScreen({ prevReport, reportIDFromRoute, isFocused, + finishedLoadingReport ]); useEffect(() => { From 765eaa03368b09ff0319fdbf9f152c34ca4fabd6 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 7 Jun 2024 12:40:53 -0400 Subject: [PATCH 05/12] simplify variables --- src/pages/home/ReportScreen.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 4026c116252e..06676ff1b226 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -157,8 +157,9 @@ function ReportScreen({ const permissions = useDeepCompareRef(reportOnyx?.permissions); const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {initialValue: true}); const wasLoadingApp = usePrevious(isLoadingApp); - const prevMetadata = usePrevious(reportMetadata); - const finishedLoadingReport = prevMetadata?.isLoadingInitialReportActions && !reportMetadata?.isLoadingInitialReportActions; + const isLoadingReport = reportMetadata?.isLoadingInitialReportActions; + const wasLoadingReport = usePrevious(isLoadingReport); + const finishedLoadingReport = wasLoadingReport && !isLoadingReport; /** * Create a lightweight Report so as to keep the re-rendering as light as possible by From 0ef3d071d7975c1386c73f0930e1a1dd0154fb47 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 7 Jun 2024 12:47:36 -0400 Subject: [PATCH 06/12] further simplify variables --- src/pages/home/ReportScreen.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 06676ff1b226..6960a0c28ea8 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -155,8 +155,10 @@ function ReportScreen({ const isLoadingReportOnyx = isLoadingOnyxValue(reportResult); const permissions = useDeepCompareRef(reportOnyx?.permissions); + const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {initialValue: true}); const wasLoadingApp = usePrevious(isLoadingApp); + const finishedLoadingApp = wasLoadingApp && !isLoadingApp; const isLoadingReport = reportMetadata?.isLoadingInitialReportActions; const wasLoadingReport = usePrevious(isLoadingReport); const finishedLoadingReport = wasLoadingReport && !isLoadingReport; @@ -635,11 +637,12 @@ function ReportScreen({ // If we deeplinked to the report after signing in, we need to fetch the report after the app is done loading useEffect(() => { - const finishedLoadingApp = wasLoadingApp && !isLoadingApp; - if (finishedLoadingApp) { - fetchReportIfNeeded(); + if (!finishedLoadingApp) { + return; } - }, [wasLoadingApp, isLoadingApp, fetchReportIfNeeded]) + + fetchReportIfNeeded(); + }, [finishedLoadingApp, fetchReportIfNeeded]); const navigateToEndOfReport = useCallback(() => { Navigation.setParams({reportActionID: ''}); From be02eaa7c3d4ec972412d5d8c8b3fb652874eda8 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 7 Jun 2024 15:56:16 -0400 Subject: [PATCH 07/12] prettier --- src/pages/home/ReportScreen.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 6960a0c28ea8..05fb3f3a71e6 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -396,7 +396,13 @@ function ReportScreen({ // eslint-disable-next-line rulesdir/no-negated-variables const shouldShowNotFoundPage = useMemo( (): boolean => - (!isLoadingApp && !wasReportAccessibleRef.current && !firstRenderRef.current && !report.reportID && !isOptimisticDelete && !reportMetadata?.isLoadingInitialReportActions && !userLeavingStatus) || + (!isLoadingApp && + !wasReportAccessibleRef.current && + !firstRenderRef.current && + !report.reportID && + !isOptimisticDelete && + !reportMetadata?.isLoadingInitialReportActions && + !userLeavingStatus) || shouldHideReport || (!!currentReportIDFormRoute && !ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute)), [report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute, isLoadingApp], @@ -431,8 +437,8 @@ function ReportScreen({ } /** - * Since OpenReport is a write, the response from OpenReport will get dropped while the app is - * still loading. This usually happens when signing in and deeplinking to a report. Instead, + * Since OpenReport is a write, the response from OpenReport will get dropped while the app is + * still loading. This usually happens when signing in and deeplinking to a report. Instead, * we'll fetch the report after the app finishes loading in an effect below */ if (isLoadingApp) { @@ -585,7 +591,7 @@ function ReportScreen({ prevReport, reportIDFromRoute, isFocused, - finishedLoadingReport + finishedLoadingReport, ]); useEffect(() => { From a4386301daa547e9a22143cdbd0b635d3b3f1915 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Mon, 17 Jun 2024 16:10:06 -0400 Subject: [PATCH 08/12] simplify loading checks --- src/pages/home/ReportScreen.tsx | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index b82c8cc0e555..2775c868b5aa 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -154,17 +154,13 @@ function ReportScreen({ canEvict: false, selector: (parentReportActions) => getParentReportAction(parentReportActions, reportOnyx?.parentReportActionID ?? ''), }); + const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP); + const wasLoadingApp = usePrevious(isLoadingApp); + const finishedLoadingApp = wasLoadingApp && !isLoadingApp; const isLoadingReportOnyx = isLoadingOnyxValue(reportResult); const permissions = useDeepCompareRef(reportOnyx?.permissions); - const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {initialValue: true}); - const wasLoadingApp = usePrevious(isLoadingApp); - const finishedLoadingApp = wasLoadingApp && !isLoadingApp; - const isLoadingReport = reportMetadata?.isLoadingInitialReportActions; - const wasLoadingReport = usePrevious(isLoadingReport); - const finishedLoadingReport = wasLoadingReport && !isLoadingReport; - /** * Create a lightweight Report so as to keep the re-rendering as light as possible by * passing in only the required props. @@ -441,9 +437,12 @@ function ReportScreen({ /** * Since OpenReport is a write, the response from OpenReport will get dropped while the app is * still loading. This usually happens when signing in and deeplinking to a report. Instead, - * we'll fetch the report after the app finishes loading in an effect below + * we'll fetch the report after the app finishes loading. + * + * This needs to be a strict equality check since isLoadingApp is initially undefined until the + * value is loaded from Onyx */ - if (isLoadingApp) { + if (isLoadingApp !== false) { return; } @@ -572,11 +571,6 @@ function ReportScreen({ return; } - // If we just finished loading the report, we don't need to load it again - if (finishedLoadingReport) { - return; - } - fetchReportIfNeeded(); ComposerActions.setShouldShowComposeInput(true); }, [ @@ -593,7 +587,6 @@ function ReportScreen({ prevReport, reportIDFromRoute, isFocused, - finishedLoadingReport, ]); useEffect(() => { @@ -650,7 +643,10 @@ function ReportScreen({ } fetchReportIfNeeded(); - }, [finishedLoadingApp, fetchReportIfNeeded]); + + // This should only run once when the app is done loading + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [finishedLoadingApp]); const navigateToEndOfReport = useCallback(() => { Navigation.setParams({reportActionID: ''}); From aecc99371f57aec5026d030e7eb124a7788fbe55 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Mon, 17 Jun 2024 16:10:18 -0400 Subject: [PATCH 09/12] fix condition --- src/pages/home/ReportScreen.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 2775c868b5aa..61cd514177ac 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -381,7 +381,7 @@ function ReportScreen({ return reportIDFromRoute !== '' && !!report.reportID && !isTransitioning; }, [report, reportIDFromRoute]); - const isLoading = isLoadingApp || !reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty(); + const isLoading = isLoadingApp ?? (!reportIDFromRoute || (!isSidebarLoaded && !isReportOpenInRHP) || PersonalDetailsUtils.isPersonalDetailsEmpty()); const shouldShowSkeleton = !isLinkedMessageAvailable && (isLinkingToMessage || From 144bb3c38a0e473e9f90c0240f0e4ab71b0a2463 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Mon, 17 Jun 2024 16:26:43 -0400 Subject: [PATCH 10/12] prettier --- src/pages/home/ReportScreen.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 61cd514177ac..efbecb08a72d 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -437,8 +437,8 @@ function ReportScreen({ /** * Since OpenReport is a write, the response from OpenReport will get dropped while the app is * still loading. This usually happens when signing in and deeplinking to a report. Instead, - * we'll fetch the report after the app finishes loading. - * + * we'll fetch the report after the app finishes loading. + * * This needs to be a strict equality check since isLoadingApp is initially undefined until the * value is loaded from Onyx */ @@ -643,7 +643,7 @@ function ReportScreen({ } fetchReportIfNeeded(); - + // This should only run once when the app is done loading // eslint-disable-next-line react-hooks/exhaustive-deps }, [finishedLoadingApp]); From 63061d935ac5c75305ca6510ef4e78cdf691e693 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Wed, 19 Jun 2024 10:08:04 -0400 Subject: [PATCH 11/12] improve and simplify conditions --- src/pages/home/ReportScreen.tsx | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index efbecb08a72d..9c4d21b40ded 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -391,20 +391,24 @@ function ReportScreen({ (!!reportActionIDFromRoute && reportMetadata?.isLoadingInitialReportActions)); const shouldShowReportActionList = isCurrentReportLoadedFromOnyx && !isLoading; const currentReportIDFormRoute = route.params?.reportID; + // eslint-disable-next-line rulesdir/no-negated-variables - const shouldShowNotFoundPage = useMemo( - (): boolean => - (!isLoadingApp && - !wasReportAccessibleRef.current && - !firstRenderRef.current && - !report.reportID && - !isOptimisticDelete && - !reportMetadata?.isLoadingInitialReportActions && - !userLeavingStatus) || - shouldHideReport || - (!!currentReportIDFormRoute && !ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute)), - [report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute, isLoadingApp], - ); + const shouldShowNotFoundPage = useMemo((): boolean => { + // Wait until we're sure the app is done loading (needs to be a strict equality check since it's undefined initially) + if (isLoadingApp !== false) { + return false; + } + + if (!wasReportAccessibleRef.current && !firstRenderRef.current && !report.reportID && !isOptimisticDelete && !reportMetadata?.isLoadingInitialReportActions && !userLeavingStatus) { + return true; + } + + if (shouldHideReport) { + return true; + } + + return !!currentReportIDFormRoute && !ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute); + }, [isLoadingApp, report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute]); const fetchReport = useCallback(() => { Report.openReport(reportIDFromRoute, reportActionIDFromRoute); From f6258445cdd6ab82f2f31911fd28462896876c61 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Wed, 19 Jun 2024 10:10:52 -0400 Subject: [PATCH 12/12] fix not found flash after loading app --- src/pages/home/ReportScreen.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 9c4d21b40ded..916a499fe2ed 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -399,6 +399,12 @@ function ReportScreen({ return false; } + // If we just finished loading the app, we still need to try fetching the report. Wait until that's done before + // showing the Not Found page + if (finishedLoadingApp) { + return false; + } + if (!wasReportAccessibleRef.current && !firstRenderRef.current && !report.reportID && !isOptimisticDelete && !reportMetadata?.isLoadingInitialReportActions && !userLeavingStatus) { return true; } @@ -408,7 +414,7 @@ function ReportScreen({ } return !!currentReportIDFormRoute && !ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute); - }, [isLoadingApp, report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute]); + }, [isLoadingApp, finishedLoadingApp, report.reportID, isOptimisticDelete, reportMetadata?.isLoadingInitialReportActions, userLeavingStatus, shouldHideReport, currentReportIDFormRoute]); const fetchReport = useCallback(() => { Report.openReport(reportIDFromRoute, reportActionIDFromRoute);