facebook / flipper

A desktop debugging platform for mobile developers.
https://fbflipper.com/
MIT License
13.32k stars 953 forks source link

Flipper Layout plugin freezes apps in some cases when using Modals #1399

Closed djMax closed 4 years ago

djMax commented 4 years ago

🐛 Bug Report

Clear and concise is going to be a doozy. BUT, I have a react-native app using react-navigation. I added an overlay spinner using various plugins and then finally just using a raw react-native Modal (not a react-navigation modal). When the Modal is dismissed, I cannot click on the app anymore. Anywhere. I noticed in XCode that the UITransitionView that hosted the modal was "still there" after the Modal went away. A coworker couldn't reproduce, but then COULD reproduce when enabling the layout plugin in Flipper. Sure enough, we can all now reproduce IF the layout plugin is enabled. If it is not enabled, the UITransitionView goes away, and the app works as intended.

Far be it from me to guess, but perhaps something is holding on to a reference that causes the transition view not to clean up.

To Reproduce

Use react-navigation stack navigator or similar in RN 0.63.1 with Flipper, then add this to the app:

https://www.npmjs.com/package/react-native-loading-spinner-overlay

Bring up a modal, take it down, and then you will not be able to click anything.

Environment

iOS 13.6/XCode 13.6 (also happens in 13.3) RN 0.63.1 Latest of all related packages

nikoant commented 4 years ago

Hi @djMax, could you make a small repro project? It might be specific to a certain setup.

djMax commented 4 years ago

I have tried to make a repro, but it doesn't repro. I tried variations of this:

import * as React from 'react';
import { View, Text, Modal, Button } from 'react-native';
import { NavigationContainer } from '@react-navigation/native';
import { createStackNavigator } from '@react-navigation/stack';

const ModalContext = React.createContext();

const Stack = createStackNavigator();
const ModalStack = createStackNavigator();

function HomeScreen() {
  const modalContext = React.useContext(ModalContext);
  return (
    <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
      <Text>This is the home screen</Text>
      <Button onPress={modalContext.show} title="Show Modal" />
    </View>
  );
}

function Nested() {
  return (
    <ModalStack.Navigator
      mode="modal"
      screenOptions={({ route, navigation }) => ({
        headerShown: false,
        gestureEnabled: true,
        cardOverlayEnabled: true,
        headerStatusBarHeight:
          navigation.dangerouslyGetState().routes.indexOf(route) > 0 ? 0 : undefined,
        ...TransitionPresets.ModalPresentationIOS,
      })}>
      <ModalStack.Screen name="app" component={HomeScreen} />
      <ModalStack.Screen name="rnmodal" component={HomeScreen} />
    </ ModalStack.Navigator>
  );
}

function App() {
  const [visible, setVisible] = React.useState(false);
  const provider = {
    visible, show() {
      setVisible(true);
      setTimeout(() => setVisible(false), 5000);
    }
  };

  return (
    <ModalContext.Provider value={provider}>
      <NavigationContainer>
        <Stack.Navigator>
          <Stack.Screen name="Home" component={HomeScreen} />
        </Stack.Navigator>
        <Modal visible={visible}>
        <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
          <Text>This is the modal</Text>
        </View>
      </Modal>
      </NavigationContainer>
    </ModalContext.Provider>
  );
}

export default App;

It may very well be some sort of race condition, but I'm not sure how to repeat it. I don't notice a SECOND UITransitionView in my case, so maybe if someone can think of a way to get that second view up...

adrianso commented 4 years ago

Hi @djMax, I am having exactly the same issues in my project.

mweststrate commented 4 years ago

@adrianso in that case please try create a reproduction, as we still don't have one, to make your comment actionable :)

dmitryusikriseapps commented 4 years ago

I faced this issue also. Let me provide the working example: 1) Modal: https://pastebin.com/xvqGc9P9

2) Versions: "react-native": "0.63.2", "react-native-modal": "11.5.6",

dmitryusikriseapps commented 4 years ago

If you would like me to provide any additional info just ping me

dmitryusikriseapps commented 4 years ago

Modal.defaultProps = Modal.defaultProps || {} Modal.defaultProps.useNativeDriver = true Modal.defaultProps.hideModalContentWhileAnimating = true

lukewlms commented 4 years ago

This repros for us when pulling up:

Spent a couple hours thinking the problem was in my code before someone on Stack Overflow told me this happens. This repros when I reduce the app to just a view and a modal (but I haven't removed node_modules packages and tried that way).

mweststrate commented 4 years ago

@priteshrnandgaonkar do you have any ideas on this one? Seems to be iOS/ComponentKit specific if I understand the issue correctly, and IIRC there were some changes related to registering root views with Flipper a while back?

priteshrnandgaonkar commented 4 years ago

Can someone give me a reproducible example in their git repo ? Which I can pull in and easily reproduce.

mweststrate commented 4 years ago

Ok, created a small repo:

  1. clone https://github.com/mweststrate/flipper-issue-1399-repo
  2. yarn install
  3. yarn ios (make sure Flipper is NOT running)
  4. you can now click 'trigger alert' > 'OK' > 'trigger alert' > 'OK' many times
  5. start flipper and enable layout plugin (app name MRLTESTNATIVE)
  6. you can now click trigger alert > OK only once, but not a second time
anfriis commented 4 years ago

@priteshrnandgaonkar I have the same issue and was able to easily reproduce in a very basic project with only react-native and react-native-flipper as dependencies: https://github.com/anfriis/FlipperModalBug

How to reproduce:

If you debug with Flipper's Layout plugin, you can also see that the UITransitionView is not removed from the view hierarchy. Open the RN Debug Menu again and it just stack's another one on top.

With Flipper disabled, the RN Debug menu is removed from the view hierarchy, causing no problems:

//  #ifdef FB_SONARKIT_ENABLED
//    InitializeFlipper(application);
//  #endif

Hoping for a fix soon as this kinda makes Flipper unusable right now for iOS at this version, if you use modal transitions which is rather common

priteshrnandgaonkar commented 4 years ago

@mweststrate I will look at the repro and try to fix it.

priteshrnandgaonkar commented 4 years ago

I figured out the fix For RN 0.63.2, The flipper pod used is too old, so for temporary fix, you can do the following:

Go to file UIView+SKInvalidation.mm Locate swizzle_removeFromSuperview

Replace that function with the following:

- (void)swizzle_removeFromSuperview { 
  id<SKInvalidationDelegate> delegate =
      [SKInvalidation sharedInstance].delegate;
  if (delegate != nil && self.superview != nil) {
    if (shouldInvalidateRootNode(self.superview)) {
      [delegate invalidateRootNode]; 
      [self swizzle_removeFromSuperview]; // Just added this line,rest is same.
      return;
    }
    [delegate invalidateNode:self.superview];
  }

  [self swizzle_removeFromSuperview];
}

@mweststrate can you coordinate with RN team to bump up our Pod.

anfriis commented 4 years ago

@priteshrnandgaonkar I can confirm that the fix works for my repro too. Thanks 🥇

mweststrate commented 4 years ago

Simpler solution, in ios/Podfile change use_flipper! to use_flipper!('Flipper' => '0.54.0') and run pod install --repo-update afterwards in ios dir.

Working on an update in the react-native repo. But since it is basically solved closing this one for now.

3runoDesign commented 2 years ago

I have this problem here when using reanimated v2