react-native-screens icon indicating copy to clipboard operation
react-native-screens copied to clipboard

fix(Android): crash API 25 when going back from screen with flat list

Open maciekstosio opened this issue 9 months ago • 1 comments

Description

Fixes #2810

This PR should fix a crash when going back from the screen that has flat list.

Changes

There are two exceptions thrown:

First:

Error: Exception in HostFunction: android.view.ViewRootImpl$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.

Second:

java.lang.IndexOutOfBoundsException: getChildDrawingOrder() returned invalid index 1 (child count is 1)

And it fails around this place in Screen.kt:

(...)
                if (parent is SwipeRefreshLayout && child is ImageView) {
                    // SwipeRefreshLayout class which has CircleImageView as a child,
                    // does not handle `startViewTransition` properly.
                    // It has a custom `getChildDrawingOrder` method which returns
                    // wrong index if we called `startViewTransition` on the views on new arch.
                    // We add a simple View to bump the number of children to make it work.
                    // TODO: find a better way to handle this scenario
                        it.addView(View(context), i)
                } else {
                    child?.let { view -> it.startViewTransition(view) }
                }
(...)

Based on the comment, I assume it.addView(View(context), i) is to prevent the second crash. Because the first exception, we do not add "workaround" view, thus we run into second. We get the first exception, as the operation is performed from different thread, thus I ensure the adding view is performed from UIThread.

Test code and steps to reproduce

Source code
import * as React from 'react';
import { View, Text, FlatList } from 'react-native';
import { NavigationContainer, useNavigation } from '@react-navigation/native';
import { createNativeStackNavigator, NativeStackScreenProps } from '@react-navigation/native-stack';
import { Button } from '@react-navigation/elements';
import { SafeAreaProvider } from 'react-native-safe-area-context';

type StackParamList = {
  Home: undefined;
  Details: undefined;
};

function HomeScreen() {
  const navigation = useNavigation<NativeStackScreenProps<StackParamList, 'Home'>['navigation']>();

  return (
    <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
      <Text>Home Screen</Text>
      <Button onPress={() => navigation.navigate('Details')}>
        Go to Details
      </Button>
    </View>
  );
}

function DetailsScreen() {
  const [isRefreshing, setIsRefreshing] = React.useState(false);

  return (
    <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
      <FlatList
        data={[1, 2, 3, 4, 5]}
        renderItem={({ item }) => <Text>{item}</Text>}
        onRefresh={() => {
          return new Promise<void>((resolve) => {
            setIsRefreshing(true);
            setTimeout(() => {
              resolve();
            }, 3000);
          }).then(() => {
            setIsRefreshing(false);
          });
        }}
        refreshing={isRefreshing}
      />
    </View>
  );
}

const Stack = createNativeStackNavigator<StackParamList>();

function NestedStack() {
  return (
    <Stack.Navigator>
      <Stack.Screen name="Home" component={HomeScreen} />
      <Stack.Screen name="Details" component={DetailsScreen} />
    </Stack.Navigator>
  );
}

export default function App() {
  return (
    <SafeAreaProvider>
      <NavigationContainer>
        <NestedStack />
      </NavigationContainer>
    </SafeAreaProvider>
  );
};

Checklist

  • [x] Included code example that can be used to test this change
  • [ ] Ensured that CI passes

maciekstosio avatar Jun 05 '25 09:06 maciekstosio

This is the approach I've mentioned to you: https://github.com/software-mansion/react-native-screens/pull/2532/files, however I'm not happy with this PR neither. Let's have a talk today & I'll explain.

kkafar avatar Jun 06 '25 03:06 kkafar

@maciekstosio Do you remember, what was the conclusion for this PR? Were there any issues? I have something in the back of my head, that there was an issue, but can not recall exactly.

kkafar avatar Jul 17 '25 09:07 kkafar

This problem would not be solved with?

fun startRemovalTransition() {
        if (!isBeingRemoved) {
            isBeingRemoved = true
            post { startTransitionRecursive(this) }
        }
    }

mensonones avatar Jul 24 '25 19:07 mensonones