search

dominicstop / react-native-ios-context-menu

A react-native component to use context menu's (UIMenu) on iOS 13/14+
MIT License
558 stars 26 forks source link

Unmounting of a direct child doesn't happen with `removeFromSuperview` cleanup #63

Open fsher opened 1 year ago

fsher commented 1 year ago

Hello there,

We've noticed that ContextMenuView doesn't unmount a child that uses removeFromSuperview (in our case react-native-video - here) to cleanup the components on the iOS level. I haven't verified if that happens to any direct child, in our case it's specific to removeFromSuperview.

While debugging the issue, I've tried different internalCleanupMode variants and in no case breakpoint in removeFromSuperview method in the ContextMenuView child component has been called.

The bug

Repro code: https://gist.github.com/fsher/54fa665bd8eee77a5636c14a89c595c0 (If you wrap that <Video ... /> component with a <View /> then the unmounting on press will happen.)

Steps to reproduce

  1. Wait for the video to load
  2. Press the button to trigger unmount
  3. The button changes the state, the video doesn't get unmounted

Main use-case

The more relevant use-case is related to react-navigation. When you replace a screen, it gets unmounted. On the React level unmounting of children does happen, however the removeFromSuperview method on the pod level doesn't get called (verified through breakpoints). Once the ContextMenuView is removed, unmounting happens properly on all levels.

With basic View and Text components this issue doesn't affect the performance (maybe after a VERY long time of usage and React rerenders), however with more heavy-weight things, like videos or images, it gets pretty clunky.

Seems like a relatively serious issue, please let me know if I'm wrong here. Would appreciate any help on this as well, thanks!

Versions:

"react": "18.2.0",
"react-native": "0.71.1", // also tested on 0.69
"react-native-ios-context-menu": "1.15.3",
"react-native-video": "6.0.0-alpha.4" // just in case

P.S. reanimated has a similar issue related to component deallocation during screen changes, adding a link here as well, if it might be helpful - https://github.com/software-mansion/react-native-reanimated/issues/3124

fsher commented 1 year ago

UPD: I've tried adding override func removeReactSubview method to the RNIContextMenuView class and it hasn't been called on unmount. But I'm no expert in iOS development, I might be missing something here. Please let me know if you have any ideas. Thanks in advance!

cc @dominicstop

fsher commented 1 year ago

UPD 2: I think I found a way to force unmount of all children of ContextMenuView component.

By adding this lifecycle method to RNIContextMenuView class, I've started to see the removeFromSuperview method getting called on the child level:

override func removeFromSuperview() {
    self.subviews.forEach { view in
        RNIUtilities.recursivelyRemoveFromViewRegistry(bridge: self.bridge, reactView: view);
    };

    super.removeFromSuperview();
}

Again, I'm not 100% sure that this is the right way to fix this, please let me know if this is in any way helpful.