gorhom / react-native-bottom-sheet

A performant interactive bottom sheet with fully configurable options 🚀
https://ui.gorhom.dev/components/bottom-sheet
MIT License
6.66k stars 740 forks source link

[v4] Many bottom sheets = performance issues #696

Closed AlexBHarley closed 2 years ago

AlexBHarley commented 2 years ago

Bug

We (Steakwallet) make liberal use of bottom sheets throughout our application, specifically the bottom sheet modal this excellent library provides. We have 12 as of right now, with more planned.

Environment info

Library Version
@gorhom/bottom-sheet ^4
react-native ^0.64.1
react-native-reanimated ^2.1.0
react-native-gesture-handler ^1.10.3

Steps To Reproduce

Here's a shortened version of what our app looks like:

import { BottomSheetModal } from '@gorhom/bottom-sheet';
import { RefObject, useCallback, useRef, useState } from 'react';
import { createContainer } from 'unstated-next';

export enum MODAL_KEYS {
  receive = 'receive',
  ...
}

function UiContainer() {
  const receiveRef = useRef<BottomSheetModal>(null);
  ...

  const refs: { [key in MODAL_KEYS]: RefObject<BottomSheetModal> } = {
    [MODAL_KEYS.receive]: receiveRef,
    ...
  };

  const [props, setProps] = useState<{
    [MODAL_KEYS.receive]: any;
    ...
  }>({
    [MODAL_KEYS.receive]: null,
    ...
  });

  const open = useCallback((key: string, newProps: any) => {
    setProps((p: any) => ({ ...p, [key]: newProps }));
    refs[key as MODAL_KEYS].current?.present();
  }, []);

  const close = useCallback((key: string) => {
    setProps((p: any) => ({ ...p, [key]: null }));
    refs[key as MODAL_KEYS].current?.dismiss();
  }, []);

  const openReceiveModal = useCallback((token: any) => {
    open(MODAL_KEYS.receive, token);
  }, []);
  const closeReceiveModal = useCallback(() => {
    close(MODAL_KEYS.receive);
  }, []);

  ...

  return {
    refs,

    openReceiveModal,
    closeReceiveModal,

    ...

    receiveProps: props.receive,

  };
}

export const Ui = createContainer(UiContainer);
export const useUi = Ui.useContainer;

Using React Context, it means we can do:

const {openReceiveModal} = useUi()

from anywhere in our application and show the "receive" modal, which is super handy.

We're starting to notice noticeable delays in our application, with modals sometimes taking multiple seconds to pop up. So our issue is not a specific bug per se, but more a question;

github-actions[bot] commented 2 years ago

@AlexBHarley: hello! :wave:

This issue is being automatically closed because it does not follow the issue template.

AlexBHarley commented 2 years ago

@gorhom I can't get the snack working with a BottomSheetModal, tried to reproduce as best I could https://snack.expo.dev/@alex-violet-labs/bottom-sheet-v4-reproducible-issue-template

gorhom commented 2 years ago

@AlexBHarley in the snack you provided

  const open = useCallback(() => {
    const a = getRandomInt(1, 21).toString()
    console.log(a, refs[a])
    if (!refs[a] || !refs[a].current || !refs[a].current.open) {
      return
    }
    refs[a].current.open();
  }, []);

you are selecting a random ref that may not be assigned to component ? the only ref that would work is refs[1]

i am a bit confused with the snack, could you update it ?

AlexBHarley commented 2 years ago

@gorhom I was trying to get the bottom sheet to open at all, that's why it's a bit messy. I've updated to only use refs[1] and I can't get it to open. Do the modals work on snack.expo.dev?

hirbod commented 2 years ago

@gorhom I think this has to be re-opened, right? What I am seeing is that it looks like ModalBottomSheet is memory leaking. Every open and close (even though it should unmount) increases my memory consumption by 4-10MB

Helyios commented 5 months ago

@hirbod It should indeed be reopened. I'm working on an app where I need to have multiple BottomSheet and when I have more than 1, the performance dips a lot... It takes 1.5 to 2 seconds to navigate to a new screen that has 3 BottomSheet and I see a RAM increase of ~15MB whereas it's instantaneous to navigate to that same screen if I remove the BottomSheet. (It starts being a problem once I have more than one)

edwinvrgs commented 2 months ago

Same behavior as @hirbod and @Helyios. This should be reopened.