From cf4a7e77af6f4d58e91f97b59c064920cd26d660 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 12 May 2026 00:02:38 -0700 Subject: [PATCH 1/7] Group useEffects more clearly --- .../Modal/ReanimatedModal/index.tsx | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index b6d73936ee9a..a28932cba1a7 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -103,41 +103,45 @@ function ReanimatedModal({ }; }, [handleEscape, onBackButtonPressHandler]); - useEffect( - () => () => { + useEffect(() => { + if (isVisible && !isContainerOpen) { + handleRef.current = InteractionManager.createInteractionHandle(); + transitionHandleRef.current = TransitionTracker.startTransition(); + } else if (!isVisible && isContainerOpen) { + handleRef.current = InteractionManager.createInteractionHandle(); + transitionHandleRef.current = TransitionTracker.startTransition(); + } + + return () => { if (handleRef.current) { InteractionManager.clearInteractionHandle(handleRef.current); + handleRef.current = undefined; } if (transitionHandleRef.current) { TransitionTracker.endTransition(transitionHandleRef.current); transitionHandleRef.current = null; } - - setIsVisibleState(false); - setIsContainerOpen(false); - }, - - [], - ); + }; + }, [isVisible, isContainerOpen]); useEffect(() => { if (isVisible && !isContainerOpen && !isTransitioning) { - handleRef.current = InteractionManager.createInteractionHandle(); - transitionHandleRef.current = TransitionTracker.startTransition(); onModalWillShow(); + } else if (!isVisible && isContainerOpen && !isTransitioning) { + onModalWillHide(); + blurActiveElement(); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isVisible, isContainerOpen, isTransitioning]); + useEffect(() => { + if (isVisible && !isContainerOpen && !isTransitioning) { setIsVisibleState(true); setIsTransitioning(true); } else if (!isVisible && isContainerOpen && !isTransitioning) { - handleRef.current = InteractionManager.createInteractionHandle(); - transitionHandleRef.current = TransitionTracker.startTransition(); - onModalWillHide(); - - blurActiveElement(); setIsVisibleState(false); setIsTransitioning(true); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [isVisible, isContainerOpen, isTransitioning]); const backdropStyle: ViewStyle = useMemo(() => { @@ -149,6 +153,7 @@ function ReanimatedModal({ setIsContainerOpen(true); if (handleRef.current) { InteractionManager.clearInteractionHandle(handleRef.current); + handleRef.current = undefined; } if (transitionHandleRef.current) { TransitionTracker.endTransition(transitionHandleRef.current); @@ -162,6 +167,7 @@ function ReanimatedModal({ setIsContainerOpen(false); if (handleRef.current) { InteractionManager.clearInteractionHandle(handleRef.current); + handleRef.current = undefined; } if (transitionHandleRef.current) { TransitionTracker.endTransition(transitionHandleRef.current); From ef8e8a4463cfb28e0eee19f00b30bd0757027e04 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 12 May 2026 00:10:03 -0700 Subject: [PATCH 2/7] Fix React Compiler by extracting transition callback props with useEffectEvent --- src/components/Modal/ReanimatedModal/index.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index a28932cba1a7..d04ac96d2567 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -1,5 +1,5 @@ import noop from 'lodash/noop'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {useCallback, useEffect, useEffectEvent, useMemo, useRef, useState} from 'react'; import type {NativeEventSubscription, ViewStyle} from 'react-native'; // eslint-disable-next-line no-restricted-imports import {BackHandler, InteractionManager, Modal, StyleSheet, View} from 'react-native'; @@ -124,14 +124,17 @@ function ReanimatedModal({ }; }, [isVisible, isContainerOpen]); - useEffect(() => { + const fireTransitionCallbacks = useEffectEvent(() => { if (isVisible && !isContainerOpen && !isTransitioning) { onModalWillShow(); } else if (!isVisible && isContainerOpen && !isTransitioning) { onModalWillHide(); blurActiveElement(); } - // eslint-disable-next-line react-hooks/exhaustive-deps + }); + + useEffect(() => { + fireTransitionCallbacks(); }, [isVisible, isContainerOpen, isTransitioning]); useEffect(() => { From 3ee0a21c3c6703f47271c5e99f20896c085504b2 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 12 May 2026 00:15:21 -0700 Subject: [PATCH 3/7] Derive isTransitioning and eliminate isVisibleState isTransitioning can be derived as isVisible !== isContainerOpen, removing the need for a separate state variable and the effect that set it. isVisibleState was redundant - it always mirrored the isVisible prop with a one-render lag. Both are replaced with direct references to the prop and the derived expression. Co-authored-by: Cursor --- .../Modal/ReanimatedModal/index.tsx | 42 ++++++------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index d04ac96d2567..ad73ca1b6a06 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -55,9 +55,8 @@ function ReanimatedModal({ shouldReturnFocus, ...props }: ReanimatedModalProps) { - const [isVisibleState, setIsVisibleState] = useState(isVisible); const [isContainerOpen, setIsContainerOpen] = useState(false); - const [isTransitioning, setIsTransitioning] = useState(false); + const isTransitioning = isVisible !== isContainerOpen; const {windowWidth, windowHeight} = useWindowDimensions(); const backHandlerListener = useRef(null); @@ -67,15 +66,15 @@ function ReanimatedModal({ const styles = useThemeStyles(); const onBackButtonPressHandler = useCallback(() => { - if (shouldIgnoreBackHandlerDuringTransition && isTransitioning) { + if (shouldIgnoreBackHandlerDuringTransition && isVisible !== isContainerOpen) { return false; } - if (isVisibleState) { + if (isVisible) { onBackButtonPress(); return true; } return false; - }, [isVisibleState, onBackButtonPress, isTransitioning, shouldIgnoreBackHandlerDuringTransition]); + }, [isVisible, isContainerOpen, onBackButtonPress, shouldIgnoreBackHandlerDuringTransition]); const handleEscape = useCallback( (e: KeyboardEvent) => { @@ -104,10 +103,7 @@ function ReanimatedModal({ }, [handleEscape, onBackButtonPressHandler]); useEffect(() => { - if (isVisible && !isContainerOpen) { - handleRef.current = InteractionManager.createInteractionHandle(); - transitionHandleRef.current = TransitionTracker.startTransition(); - } else if (!isVisible && isContainerOpen) { + if (isVisible !== isContainerOpen) { handleRef.current = InteractionManager.createInteractionHandle(); transitionHandleRef.current = TransitionTracker.startTransition(); } @@ -125,9 +121,9 @@ function ReanimatedModal({ }, [isVisible, isContainerOpen]); const fireTransitionCallbacks = useEffectEvent(() => { - if (isVisible && !isContainerOpen && !isTransitioning) { + if (isVisible && !isContainerOpen) { onModalWillShow(); - } else if (!isVisible && isContainerOpen && !isTransitioning) { + } else if (!isVisible && isContainerOpen) { onModalWillHide(); blurActiveElement(); } @@ -135,24 +131,13 @@ function ReanimatedModal({ useEffect(() => { fireTransitionCallbacks(); - }, [isVisible, isContainerOpen, isTransitioning]); - - useEffect(() => { - if (isVisible && !isContainerOpen && !isTransitioning) { - setIsVisibleState(true); - setIsTransitioning(true); - } else if (!isVisible && isContainerOpen && !isTransitioning) { - setIsVisibleState(false); - setIsTransitioning(true); - } - }, [isVisible, isContainerOpen, isTransitioning]); + }, [isVisible, isContainerOpen]); const backdropStyle: ViewStyle = useMemo(() => { return {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; }, [windowWidth, windowHeight, backdropColor]); const onOpenCallBack = useCallback(() => { - setIsTransitioning(false); setIsContainerOpen(true); if (handleRef.current) { InteractionManager.clearInteractionHandle(handleRef.current); @@ -166,7 +151,6 @@ function ReanimatedModal({ }, [onModalShow]); const onCloseCallBack = useCallback(() => { - setIsTransitioning(false); setIsContainerOpen(false); if (handleRef.current) { InteractionManager.clearInteractionHandle(handleRef.current); @@ -221,7 +205,7 @@ function ReanimatedModal({ /> ); - if (!coverScreen && isVisibleState) { + if (!coverScreen && isVisible) { return ( ); } - const isBackdropMounted = isVisibleState || ((isTransitioning || isContainerOpen !== isVisibleState) && getPlatform() === CONST.PLATFORM.WEB); - const modalVisibility = isVisibleState || isTransitioning || isContainerOpen !== isVisibleState; + const isBackdropMounted = isVisible || (isTransitioning && getPlatform() === CONST.PLATFORM.WEB); + const modalVisibility = isVisible || isTransitioning; return ( - {isVisibleState && containerView} + {isVisible && containerView} ) : ( - {isVisibleState && containerView} + {isVisible && containerView} )} From 3326f8723ae1c12f2781d30d4f8502c0d5413487 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 12 May 2026 00:17:28 -0700 Subject: [PATCH 4/7] Remove manual memoization now handled by React Compiler useCallback and useMemo wrappers are unnecessary since the component compiles with React Compiler, which handles memoization automatically. Co-authored-by: Cursor --- .../Modal/ReanimatedModal/index.tsx | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index ad73ca1b6a06..7a5bdd789bd2 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -1,5 +1,5 @@ import noop from 'lodash/noop'; -import React, {useCallback, useEffect, useEffectEvent, useMemo, useRef, useState} from 'react'; +import React, {useEffect, useEffectEvent, useRef, useState} from 'react'; import type {NativeEventSubscription, ViewStyle} from 'react-native'; // eslint-disable-next-line no-restricted-imports import {BackHandler, InteractionManager, Modal, StyleSheet, View} from 'react-native'; @@ -65,7 +65,7 @@ function ReanimatedModal({ const styles = useThemeStyles(); - const onBackButtonPressHandler = useCallback(() => { + const onBackButtonPressHandler = () => { if (shouldIgnoreBackHandlerDuringTransition && isVisible !== isContainerOpen) { return false; } @@ -74,17 +74,14 @@ function ReanimatedModal({ return true; } return false; - }, [isVisible, isContainerOpen, onBackButtonPress, shouldIgnoreBackHandlerDuringTransition]); + }; - const handleEscape = useCallback( - (e: KeyboardEvent) => { - if (e.key !== 'Escape' || onBackButtonPressHandler() !== true) { - return; - } - e.stopImmediatePropagation(); - }, - [onBackButtonPressHandler], - ); + const handleEscape = (e: KeyboardEvent) => { + if (e.key !== 'Escape' || onBackButtonPressHandler() !== true) { + return; + } + e.stopImmediatePropagation(); + }; useEffect(() => { if (getPlatform() === CONST.PLATFORM.WEB) { @@ -133,11 +130,9 @@ function ReanimatedModal({ fireTransitionCallbacks(); }, [isVisible, isContainerOpen]); - const backdropStyle: ViewStyle = useMemo(() => { - return {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; - }, [windowWidth, windowHeight, backdropColor]); + const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; - const onOpenCallBack = useCallback(() => { + const onOpenCallBack = () => { setIsContainerOpen(true); if (handleRef.current) { InteractionManager.clearInteractionHandle(handleRef.current); @@ -148,9 +143,9 @@ function ReanimatedModal({ transitionHandleRef.current = null; } onModalShow(); - }, [onModalShow]); + }; - const onCloseCallBack = useCallback(() => { + const onCloseCallBack = () => { setIsContainerOpen(false); if (handleRef.current) { InteractionManager.clearInteractionHandle(handleRef.current); @@ -167,11 +162,9 @@ function ReanimatedModal({ if (getPlatform() === CONST.PLATFORM.ANDROID) { onModalHide(); } - }, [onModalHide]); + }; - const modalStyle = useMemo(() => { - return {zIndex: StyleSheet.flatten(style)?.zIndex}; - }, [style]); + const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; const containerView = ( Date: Tue, 12 May 2026 00:19:37 -0700 Subject: [PATCH 5/7] Sort component internals: hooks, refs, derived, callbacks, effects Co-authored-by: Cursor --- .../Modal/ReanimatedModal/index.tsx | 74 +++++++++---------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index 7a5bdd789bd2..baf820e7c759 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -56,14 +56,16 @@ function ReanimatedModal({ ...props }: ReanimatedModalProps) { const [isContainerOpen, setIsContainerOpen] = useState(false); - const isTransitioning = isVisible !== isContainerOpen; const {windowWidth, windowHeight} = useWindowDimensions(); + const styles = useThemeStyles(); const backHandlerListener = useRef(null); const handleRef = useRef(undefined); const transitionHandleRef = useRef(null); - const styles = useThemeStyles(); + const isTransitioning = isVisible !== isContainerOpen; + const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; + const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; const onBackButtonPressHandler = () => { if (shouldIgnoreBackHandlerDuringTransition && isVisible !== isContainerOpen) { @@ -83,6 +85,38 @@ function ReanimatedModal({ e.stopImmediatePropagation(); }; + const onOpenCallBack = () => { + setIsContainerOpen(true); + if (handleRef.current) { + InteractionManager.clearInteractionHandle(handleRef.current); + handleRef.current = undefined; + } + if (transitionHandleRef.current) { + TransitionTracker.endTransition(transitionHandleRef.current); + transitionHandleRef.current = null; + } + onModalShow(); + }; + + const onCloseCallBack = () => { + setIsContainerOpen(false); + if (handleRef.current) { + InteractionManager.clearInteractionHandle(handleRef.current); + handleRef.current = undefined; + } + if (transitionHandleRef.current) { + TransitionTracker.endTransition(transitionHandleRef.current); + transitionHandleRef.current = null; + } + + // Because on Android, the Modal's onDismiss callback does not work reliably. There's a reported issue at: + // https://stackoverflow.com/questions/58937956/react-native-modal-ondismiss-not-invoked + // Therefore, we manually call onModalHide() here for Android. + if (getPlatform() === CONST.PLATFORM.ANDROID) { + onModalHide(); + } + }; + useEffect(() => { if (getPlatform() === CONST.PLATFORM.WEB) { document.body.addEventListener('keyup', handleEscape, {capture: true}); @@ -130,42 +164,6 @@ function ReanimatedModal({ fireTransitionCallbacks(); }, [isVisible, isContainerOpen]); - const backdropStyle: ViewStyle = {width: windowWidth, height: windowHeight, backgroundColor: backdropColor}; - - const onOpenCallBack = () => { - setIsContainerOpen(true); - if (handleRef.current) { - InteractionManager.clearInteractionHandle(handleRef.current); - handleRef.current = undefined; - } - if (transitionHandleRef.current) { - TransitionTracker.endTransition(transitionHandleRef.current); - transitionHandleRef.current = null; - } - onModalShow(); - }; - - const onCloseCallBack = () => { - setIsContainerOpen(false); - if (handleRef.current) { - InteractionManager.clearInteractionHandle(handleRef.current); - handleRef.current = undefined; - } - if (transitionHandleRef.current) { - TransitionTracker.endTransition(transitionHandleRef.current); - transitionHandleRef.current = null; - } - - // Because on Android, the Modal's onDismiss callback does not work reliably. There's a reported issue at: - // https://stackoverflow.com/questions/58937956/react-native-modal-ondismiss-not-invoked - // Therefore, we manually call onModalHide() here for Android. - if (getPlatform() === CONST.PLATFORM.ANDROID) { - onModalHide(); - } - }; - - const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; - const containerView = ( Date: Tue, 12 May 2026 15:36:27 -0700 Subject: [PATCH 6/7] DRY up isTransitioning --- src/components/Modal/ReanimatedModal/index.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index baf820e7c759..da50a0507567 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -68,7 +68,7 @@ function ReanimatedModal({ const modalStyle = {zIndex: StyleSheet.flatten(style)?.zIndex}; const onBackButtonPressHandler = () => { - if (shouldIgnoreBackHandlerDuringTransition && isVisible !== isContainerOpen) { + if (shouldIgnoreBackHandlerDuringTransition && isTransitioning) { return false; } if (isVisible) { @@ -134,7 +134,7 @@ function ReanimatedModal({ }, [handleEscape, onBackButtonPressHandler]); useEffect(() => { - if (isVisible !== isContainerOpen) { + if (isTransitioning) { handleRef.current = InteractionManager.createInteractionHandle(); transitionHandleRef.current = TransitionTracker.startTransition(); } @@ -149,7 +149,7 @@ function ReanimatedModal({ transitionHandleRef.current = null; } }; - }, [isVisible, isContainerOpen]); + }, [isTransitioning]); const fireTransitionCallbacks = useEffectEvent(() => { if (isVisible && !isContainerOpen) { From d2809b9ba13df2e72e34abf22d4f851491ba5d81 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 12 May 2026 15:50:41 -0700 Subject: [PATCH 7/7] DRY up clearTransitionHandles --- .../Modal/ReanimatedModal/index.tsx | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/components/Modal/ReanimatedModal/index.tsx b/src/components/Modal/ReanimatedModal/index.tsx index da50a0507567..820f2bd4611c 100644 --- a/src/components/Modal/ReanimatedModal/index.tsx +++ b/src/components/Modal/ReanimatedModal/index.tsx @@ -85,8 +85,7 @@ function ReanimatedModal({ e.stopImmediatePropagation(); }; - const onOpenCallBack = () => { - setIsContainerOpen(true); + const clearTransitionHandles = () => { if (handleRef.current) { InteractionManager.clearInteractionHandle(handleRef.current); handleRef.current = undefined; @@ -95,19 +94,17 @@ function ReanimatedModal({ TransitionTracker.endTransition(transitionHandleRef.current); transitionHandleRef.current = null; } + }; + + const onOpenCallBack = () => { + setIsContainerOpen(true); + clearTransitionHandles(); onModalShow(); }; const onCloseCallBack = () => { setIsContainerOpen(false); - if (handleRef.current) { - InteractionManager.clearInteractionHandle(handleRef.current); - handleRef.current = undefined; - } - if (transitionHandleRef.current) { - TransitionTracker.endTransition(transitionHandleRef.current); - transitionHandleRef.current = null; - } + clearTransitionHandles(); // Because on Android, the Modal's onDismiss callback does not work reliably. There's a reported issue at: // https://stackoverflow.com/questions/58937956/react-native-modal-ondismiss-not-invoked @@ -140,14 +137,7 @@ function ReanimatedModal({ } return () => { - if (handleRef.current) { - InteractionManager.clearInteractionHandle(handleRef.current); - handleRef.current = undefined; - } - if (transitionHandleRef.current) { - TransitionTracker.endTransition(transitionHandleRef.current); - transitionHandleRef.current = null; - } + clearTransitionHandles(); }; }, [isTransitioning]);