react-native-bottom-sheet icon indicating copy to clipboard operation
react-native-bottom-sheet copied to clipboard

Fix Memory leak in BottomSheetBackdrop

Open tsukudabuddha opened this issue 2 years ago • 7 comments

Motivation

  • Fix Memory leak in BottomSheetBackdrop that occurs when the backdrop tries to set the pointerEvents to none when dismissing the modal. If the disappearsOnIndex is set to -1 and the animatedIndex is <= -1 , there is no need to call setEventPointers since the modal should be removed.

tsukudabuddha avatar Nov 04 '22 18:11 tsukudabuddha

here's the patch if anyone is interested:

diff --git a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetBackdrop/BottomSheetBackdrop.tsx b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetBackdrop/BottomSheetBackdrop.tsx
index 35597ce..964bc25 100644
--- a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetBackdrop/BottomSheetBackdrop.tsx
+++ b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetBackdrop/BottomSheetBackdrop.tsx
@@ -105,7 +105,7 @@ const BottomSheetBackdropComponent = ({
   useAnimatedReaction(
     () => animatedIndex.value <= disappearsOnIndex,
     (shouldDisableTouchability, previous) => {
-      if (shouldDisableTouchability === previous) {
+      if (shouldDisableTouchability === previous || disappearsOnIndex === -1) {
         return;
       }
       runOnJS(handleContainerTouchability)(shouldDisableTouchability);

tsukudabuddha avatar Nov 05 '22 00:11 tsukudabuddha

using local patch until we wait for this to get merged, @gorhom appreciate when you have some time to review it

rgomesrn avatar Nov 21 '22 21:11 rgomesrn

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions[bot] avatar Jan 14 '23 09:01 github-actions[bot]

@gorhom waiting for this PR to be merged. Could you please check this PR and merge

vijaychouhan-rails avatar Feb 08 '23 08:02 vijaychouhan-rails

In my experience this fix prevents the bottom-sheet from opening on the first try. This pull request needs to be developed. Be careful before use.

HyopeR avatar Feb 17 '23 13:02 HyopeR

@tsukudabuddha thanks for submitting this PR, do you have a detailed issue reported i could verify the fix with?

gorhom avatar Apr 30 '23 14:04 gorhom

@gorhom here's an issue with more details: #1376

donatoaguirre24 avatar Jun 20 '23 20:06 donatoaguirre24