[3.7.0] Android change broke with appcompat >= 1.3.x and detachPreviousScreen

[cristianoccazinsp]

Description

#1066 introduced a bug that causes inactive screen to end up stuck when detachPreviousScreen is used alongside androidx.appcompat:appcompat 1.3.1 or above. The issue does not happen when using the outdated 1.1.0 version.

Screenshots

Steps To Reproduce

Force a compat version greater than 1.1, e.g.,

implementation('androidx.appcompat:appcompat') {
        version {
            strictly '1.3.1'
        }
    }

Use stack navigator with detachPreviousScreen, e.g.,

/* eslint-disable react/display-name */
/* eslint-disable @typescript-eslint/no-unused-vars */
import { Text, View, StyleSheet,TouchableOpacity, StatusBar, Animated } from 'react-native';
import React from 'react';
import { NavigationContainer } from '@react-navigation/native';
import {createStackNavigator} from '@react-navigation/stack';


const animationConfig = {
  animation: 'spring',
  config: {
    stiffness: 1100,
    damping: 100,
    mass: 3,
    overshootClamping: true,
    restDisplacementThreshold: 10,
    restSpeedThreshold: 10,
    useNativeDriver: true,
  },
};

//let animationInterp = CardStyleInterpolators.forHorizontalIOS; // use iOS slide as well on Android, looks cool
// https://reactnavigation.org/docs/stack-navigator/#animations
// https://github.com/react-navigation/react-navigation/blob/6cba517b74f5fd092db21d5574b558ef2d80897b/packages/stack/src/TransitionConfigs/CardStyleInterpolators.tsx#L14
const {multiply} = Animated;
const animationInterp = ({current, next, inverted, layouts: {screen}}) => {
  const translateFocused = multiply(
    current.progress.interpolate({
      inputRange: [0, 1],
      outputRange: [screen.width, 0],
      extrapolate: 'clamp',
    }),
    inverted,
  );

  const translateUnfocused = next
    ? multiply(
        next.progress.interpolate({
          inputRange: [0, 1],
          outputRange: [0, screen.width * -0.8],
          extrapolate: 'clamp',
        }),
        inverted,
      )
    : 0;

  const overlayOpacity = current.progress.interpolate({
    inputRange: [0, 1],
    outputRange: [0, 0.1],
    extrapolate: 'clamp',
  });

  const shadowOpacity = current.progress.interpolate({
    inputRange: [0, 1],
    outputRange: [0, 0.3],
    extrapolate: 'clamp',
  });

  return {
    cardStyle: {
      transform: [
        // Translation for the animation of the current card
        {translateX: translateFocused},
        // Translation for the animation of the card on top of this
        {translateX: translateUnfocused},
      ],
    },
    overlayStyle: {opacity: overlayOpacity},
    shadowStyle: {shadowOpacity},
  };
};

function ScreenX({navigation, route}) {
  return (
    <View style={styles.screenContainer}>
      <Text style={styles.paragraph}>
        This screen is used for testing bugs!
      </Text>
      <TouchableOpacity onPress={() => navigation.goBack()}><Text>Go Back</Text></TouchableOpacity>
      <TouchableOpacity onPress={() => navigation.navigate(route.params?.next)}><Text>Go Next</Text></TouchableOpacity>
    </View>
  );
}

function ScreenEnd({navigation}) {
  return (
    <View style={styles.screenContainer}>
      <Text style={styles.paragraph}>
        Final screen.
      </Text>
      <TouchableOpacity onPress={() => navigation.goBack()}><Text>Go back</Text></TouchableOpacity>
    </View>
  );
}



const Stack = createStackNavigator();

const StackNavigator = () => {

  return (
    <Stack.Navigator
      screenOptions={({navigation, route}) => {
        return {
          headerMode: 'screen',
          header: (props) => (
            <View style={styles.headerStyle}>
              <Text>{props.options.title}</Text>
            </View>
          ),
          gestureEnabled: false,
          cardOverlayEnabled: true,
          transitionSpec: {
            open: animationConfig,
            close: animationConfig,
          },
          cardStyleInterpolator: animationInterp,
        };
      }}>
      <Stack.Screen
        name="Screen1"
        component={ScreenX}
        options={({navigation, route}) => {
          return {
            title: 'Home',
            header: (props) => (
              <View style={styles.headerStyle}>
                <Text>Home Test</Text>
              </View>
            ),
          };
        }}
        initialParams={{ next: 'Screen2'}}
      />
      <Stack.Screen
        name="Screen2"
        component={ScreenX}
        options={{title: 'Screen2'}}
        initialParams={{ next: 'Screen3'}}
      />
      <Stack.Screen
        name="Screen3"
        component={ScreenX}
        options={{
          title: 'Screen3',
          headerShown: false,
          animationEnabled: true,
          detachPreviousScreen: false,
        }}
        initialParams={{ next: 'Screen4' }}
      />
      <Stack.Screen
        name="Screen4"
        component={ScreenEnd}
        options={{title: 'Screen4'}}
      />
    </Stack.Navigator>
  );
};


export default function BuggedStack() {
  return (
    <View style={styles.container}>
      <NavigationContainer>
        <StackNavigator />
      </NavigationContainer>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    paddingTop: StatusBar.currentHeight,
  },
  screenContainer: {
    alignItems: 'center',
    justifyContent: 'center',
    backgroundColor: 'black',
    padding: 24,
  },
  paragraph: {
    margin: 24,
    marginTop: 0,
    fontSize: 14,
    fontWeight: 'bold',
    textAlign: 'center',
  },
  headerStyle: {
    height: 80,
    elevation: 3,
    top: 0,
    left: 0,
    right: 0,
    zIndex: 100,
    backgroundColor: 'black',
  }
});

Run the app and navigate next until the last screen.

Navigate back and observe

Expected behavior

Screen transitioning normally

Actual behavior

Screen stuck

Snack or minimal code example

https://github.com/cristianoccazinsp/react-native-screens/tree/bug-repro
https://github.com/software-mansion/react-native-screens/compare/master...cristianoccazinsp:bug-repro?expand=1

Package versions

• React: 17.0.2
• React Native: 0.65.1
• React Native Screens: 3.7.0

[WoLewicki]

I managed to reproduce the freeze. It seems like a change in the behavior of Fragment in the newer versions of androidx.fragment:fragment. The issue goes away if you strictly provide the Fragment library version used by us, which is 1.2.1. Can you check if adding below code resolves your issue and does not introduce new problems? I don't think there is another easy solution since providing changes to the code would probably destroy the current behavior. If it works, can we close this issue then?

    implementation('androidx.fragment:fragment') {
      version {
            strictly '1.2.1'
        }
    }

[cristianoccazinsp]

@WoLewicki I had actually solved the issue by forcing androidx compat to 1.1 (the default one provided by RN?), but didn't think about only requiring for the fragment one. I will give it a try later today and reply back.

However, as we migrate closer to Android 12 (will be released shortly, and most apps will be forced to upgrade to SDK 31 due to some extremely ugly splash screen changes), this issue will need to be fixed eventually. Are the fragment changes that big?

[cristianoccazinsp]

@WoLewicki forcing androidx fragment version does fix the issue too.

However, if we also require to define androidx.appcompat:appcompat due to another lib requiring it, which version should we use? The initial issue happened when using androidx.appcompat:appcompat 1.3.+, and I'm afraid that mixing it with fragment 1.2.1 may break something.

[cristianoccazinsp]

@WoLewicki after further testing, forcing fragment 1.2.1 (or any in the 1.2.x series) breaks screens that use surfaces (only with screens 3.7.x). For instance, react-native-camera ends up stopping and restarting the camera when the screen that has a camera component unmounts. I have traced this to some odd behaviour that causes the screen texture (TextureView).

You can see the behaviour in the following videos, basically, the camera becomes nearly unusable because it stops and starts while the screen pop animation happens, making it very slow and freezing the UI.

With fragment 1.2.1
https://user-images.githubusercontent.com/48869228/133343397-6f802a96-8827-4d72-b73e-6271300cff6b.mp4

With fragment 1.3.x
https://user-images.githubusercontent.com/48869228/133343412-5baac484-5db3-467a-974c-98d504cdd362.mp4

Any ideas on what could it be? This is game breaking for us.

Note that there are no issues on screen pushes (surface is destroyed after the animation happens, instead of being destroyed and recreated in order to do the pop animation)

[WoLewicki]

I have no knowledge of the internals of the Fragment implementation unfortunately. Since your setup differs from the one suggested by the library, and probably used by most of the users, I can see 2 quick options.
First is to use patch-package in order not to apply this commit: #1066.
Second one is to try and debug code of updateContainer in order to fix the unwanted behavior which happens there. One of the options why the previous screen is shown may be the changed ordering of drawing Fragments introduced in one of the versions. I'd look especially this code: https://github.com/software-mansion/react-native-screens/blob/master/android/src/main/java/com/swmansion/rnscreens/ScreenContainer.kt#L333 to see why going forward keeps the correct ordering of multiple visible Fragments and, at the same time, going back doesn't.
Can we help you more with this?

[cristianoccazinsp]

For the long term, supporting the updated fragment versions I guess would be ideal, but for now I have no problem with forcing fragments 1.2 in order for the library to work. However, with fragment 1.2 the issue of surfaces getting recreated when popping starts to happen.

Would it be easier to debug what’s wrong with the 1.2 implementation as opposed to to trying to support fragment 1.3? Keep in mind this means that the recommended setup will cause issues to anyone using react native camera or any library that uses texture surfaces to draw content.

I will try to do some debugging, but debugging android native is not really my strong.

[WoLewicki]

You can rather quickly check the changes here: https://developer.android.com/jetpack/androidx/releases/fragment#1.2.1. Does this issue appear if you use the appCompat version suggested by the library?

[cristianoccazinsp]

@WoLewicki I have tried all version combinations, and the surface being re-created when popping happens with any version below fragment/appcompat < 1.3, and it was not introduced in 3.7, as I found it also happens with 3.6.

So, in conclusion, looks like I've been using appcompat and fragment 1.3.x for a while (with screens ever since 3.x), and never had issues. Then, when upgrading to 3.7, the stuck screens issue appeared due to the use of fragment 1.3, so I ended up going back to appcompat and fragment 1.1 and 1.2, but then this brought back, a possibly old bug, that makes texture surfaces to be re-created when popping a screen and breaking anything that relies on it (e.g., react native camera).

I guess I will stick with appcompat 1.3 so texture surfaces continue to work as before, and just stay away from detachPreviousScreen for now on Android until I figure out how this can be fixed. I either need to fix the screens behaviour with fragment 1.3, or figure out why surfaces are destroyed and created when using fragment 1.2.

Just to add a bit more, the issue with texture views comes from using them as recommended by android here https://developer.android.com/reference/android/view/TextureView . Basically, popping a screen causes onSurfaceTextureDestroyed and onSurfaceTextureAvailable to be fired twice, once when the animation starts, and once when the component unmounts (the only one that should ever happen).

[WoLewicki]

Hmm, and what is your use-case for detachPreviousScreen ?

[cristianoccazinsp]

I'm using detachPreviousScreen for a totally non-related issue. Basically, I have one screen that renders stuff in a modal and exposes that feature through its context. However, that screen may sometimes take the user to a second screen that in turn may request the previous screen to show its modal (odd I know!).

So without detachPreviousScreen, the modal cannot be shown because the previous screen is taken out of the rendering tree. I ended up using detachPreviousScreen only on iOS, as on Android, it does show even if the previous screen is detached. This difference in behaviour is also a bit odd, but not a big deal.

[WoLewicki]

Ok, let me know if you find out anything concerning the behavior in newer versions. I will also try and debug it a little in the free time, but I guess it won't be easy to apply changes to the code such that it works correctly for all versions of appCompat.

[WoLewicki]

I think I found the reason for the issue. It is described in PR: #1133. Can you check if it fixes your issue and does not introduce any new ones?

[cristianoccazinsp]

@WoLewicki you're awesome! I will test shortly and let you know.

[Bardiamist]

I also use detachPreviousScreen: false because detachPreviousScreen works bad (white blinks) and app stuck after call navigationContainerRef.current?.reset({ routes, index }).
I'll revert react-native-screens to 3.6.0 until fix.