From bd70f71bd9768916b3a7b8baa45c8980dbf6bbd0 Mon Sep 17 00:00:00 2001 From: layacat Date: Mon, 7 Oct 2024 14:42:07 +0700 Subject: [PATCH 1/3] fix: Book travel - Book travel animation becomes blank while RHP is dismissed --- src/components/Lottie/index.tsx | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/components/Lottie/index.tsx b/src/components/Lottie/index.tsx index eee037e083f7..b3c7933bd0a7 100644 --- a/src/components/Lottie/index.tsx +++ b/src/components/Lottie/index.tsx @@ -2,7 +2,7 @@ import {NavigationContainerRefContext, NavigationContext} from '@react-navigatio import type {AnimationObject, LottieViewProps} from 'lottie-react-native'; import LottieView from 'lottie-react-native'; import type {ForwardedRef} from 'react'; -import React, {forwardRef, useContext, useEffect, useState} from 'react'; +import React, {forwardRef, useContext, useEffect, useRef, useState} from 'react'; import {InteractionManager, View} from 'react-native'; import type DotLottieAnimation from '@components/LottieAnimations/types'; import useAppState from '@hooks/useAppState'; @@ -18,7 +18,8 @@ type Props = { shouldLoadAfterInteractions?: boolean; } & Omit; -function Lottie({source, webStyle, shouldLoadAfterInteractions, ...props}: Props, ref: ForwardedRef) { +function Lottie({source, webStyle, shouldLoadAfterInteractions, ...props}: Props, forwardedRef: ForwardedRef) { + const animationRef = useRef(null); const appState = useAppState(); const {splashScreenState} = useSplashScreenStateContext(); const styles = useThemeStyles(); @@ -79,19 +80,20 @@ function Lottie({source, webStyle, shouldLoadAfterInteractions, ...props}: Props return unsubscribeNavigationBlur; }, [browser, navigationContainerRef, navigator]); + // If user is being navigated away, let pause the animation to prevent memory leak. + useEffect(() => { + if (!animationRef.current || !hasNavigatedAway) { + return; + } + animationRef?.current?.pause(); + }, [hasNavigatedAway]); + // If the page navigates to another screen, the image fails to load, app is in background state, animation file isn't ready, or the splash screen isn't hidden yet, // we'll just render an empty view as the fallback to prevent // 1. memory leak, see issue: https://github.com/Expensify/App/issues/36645 // 2. heavy rendering, see issues: https://github.com/Expensify/App/issues/34696 and https://github.com/Expensify/App/issues/47273 // 3. lag on react navigation transitions, see issue: https://github.com/Expensify/App/issues/44812 - if ( - hasNavigatedAway || - isError || - appState.isBackground || - !animationFile || - splashScreenState !== CONST.BOOT_SPLASH_STATE.HIDDEN || - (!isInteractionComplete && shouldLoadAfterInteractions) - ) { + if (isError || appState.isBackground || !animationFile || splashScreenState !== CONST.BOOT_SPLASH_STATE.HIDDEN || (!isInteractionComplete && shouldLoadAfterInteractions)) { return ; } @@ -100,7 +102,15 @@ function Lottie({source, webStyle, shouldLoadAfterInteractions, ...props}: Props // eslint-disable-next-line react/jsx-props-no-spreading {...props} source={animationFile} - ref={ref} + ref={(ref) => { + if (typeof forwardedRef === 'function') { + forwardedRef(ref); + } else if (forwardedRef && 'current' in forwardedRef) { + // eslint-disable-next-line no-param-reassign + forwardedRef.current = ref; + } + animationRef.current = ref; + }} style={[aspectRatioStyle, props.style]} webStyle={{...aspectRatioStyle, ...webStyle}} onAnimationFailure={() => setIsError(true)} From 622b892a3c313699594362f51280871b19465830 Mon Sep 17 00:00:00 2001 From: layacat Date: Tue, 8 Oct 2024 10:31:52 +0700 Subject: [PATCH 2/3] Remove memory leak comment --- src/components/Lottie/index.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/Lottie/index.tsx b/src/components/Lottie/index.tsx index b3c7933bd0a7..53db20b13aa0 100644 --- a/src/components/Lottie/index.tsx +++ b/src/components/Lottie/index.tsx @@ -90,9 +90,8 @@ function Lottie({source, webStyle, shouldLoadAfterInteractions, ...props}: Props // If the page navigates to another screen, the image fails to load, app is in background state, animation file isn't ready, or the splash screen isn't hidden yet, // we'll just render an empty view as the fallback to prevent - // 1. memory leak, see issue: https://github.com/Expensify/App/issues/36645 - // 2. heavy rendering, see issues: https://github.com/Expensify/App/issues/34696 and https://github.com/Expensify/App/issues/47273 - // 3. lag on react navigation transitions, see issue: https://github.com/Expensify/App/issues/44812 + // 1. heavy rendering, see issues: https://github.com/Expensify/App/issues/34696 and https://github.com/Expensify/App/issues/47273 + // 2. lag on react navigation transitions, see issue: https://github.com/Expensify/App/issues/44812 if (isError || appState.isBackground || !animationFile || splashScreenState !== CONST.BOOT_SPLASH_STATE.HIDDEN || (!isInteractionComplete && shouldLoadAfterInteractions)) { return ; } From c2ebaad9c1e42ce1e40e5b36fe50c533e0a3d0af Mon Sep 17 00:00:00 2001 From: layacat Date: Tue, 8 Oct 2024 13:06:25 +0700 Subject: [PATCH 3/3] Add issue link for memory leak issue --- src/components/Lottie/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Lottie/index.tsx b/src/components/Lottie/index.tsx index 53db20b13aa0..017d68aa4b56 100644 --- a/src/components/Lottie/index.tsx +++ b/src/components/Lottie/index.tsx @@ -81,6 +81,7 @@ function Lottie({source, webStyle, shouldLoadAfterInteractions, ...props}: Props }, [browser, navigationContainerRef, navigator]); // If user is being navigated away, let pause the animation to prevent memory leak. + // see issue: https://github.com/Expensify/App/issues/36645 useEffect(() => { if (!animationRef.current || !hasNavigatedAway) { return;