software-mansion / react-native-screens

Native navigation primitives for your React Native app.
https://docs.swmansion.com/react-native-screens/
MIT License
3.07k stars 518 forks source link

Android RN 0.75.3, app crashes when returning to the previous screen #2341

Closed DinhHoangNgoc closed 3 weeks ago

DinhHoangNgoc commented 1 month ago

Description

When I return to the previous screen, the app crashes. I am using version RN 0.75.3. I have tested with other RN versions, and this issue occurs from version RN 0.75 onwards.

https://github.com/user-attachments/assets/97f32a60-3a97-4b37-af4e-388d4db08553

7a0f880c8e5c2802714d

Steps to reproduce

app crashes when returning to the previous screen

Snack or a link to a repository

https://github.com/Ngoca1k15PT/AppNew

Screens version

3.34.0

React Native version

0.75.3

Platforms

Android

JavaScript runtime

None

Workflow

React Native (without Expo)

Architecture

None

Build type

Debug mode

Device

Real device

Device model

Xiaomi

Acknowledgements

Yes

hosseinmd commented 1 month ago

I have the same issue.

tamacroft commented 1 month ago

same issue here

DorianMazur commented 1 month ago

@tamacroft @hosseinmd @DinhHoangNgoc

diff --git a/android/src/main/java/com/swmansion/rnscreens/Screen.kt b/android/src/main/java/com/swmansion/rnscreens/Screen.kt
index 1ab8594ef35bf43821a46c97ca77292c87190850..8e9d712718f385638d0d58a1f3ef144e3de6eb55 100644
--- a/android/src/main/java/com/swmansion/rnscreens/Screen.kt
+++ b/android/src/main/java/com/swmansion/rnscreens/Screen.kt
@@ -10,13 +10,16 @@ import android.view.View
 import android.view.ViewGroup
 import android.view.WindowManager
 import android.webkit.WebView
+import android.widget.ImageView
 import androidx.core.view.children
 import androidx.fragment.app.Fragment
+import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
 import com.facebook.react.bridge.GuardedRunnable
 import com.facebook.react.bridge.ReactContext
 import com.facebook.react.uimanager.PixelUtil
 import com.facebook.react.uimanager.UIManagerHelper
 import com.facebook.react.uimanager.UIManagerModule
+import com.facebook.react.views.scroll.ReactScrollView
 import com.swmansion.rnscreens.events.HeaderHeightChangeEvent

 @SuppressLint("ViewConstructor") // Only we construct this view, it is never inflated.
@@ -295,7 +298,7 @@ class Screen(
         parent?.let {
             for (i in 0 until it.childCount) {
                 val child = it.getChildAt(i)
-                if (child.javaClass.simpleName.equals("CircleImageView")) {
+                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
@@ -312,6 +315,11 @@ class Screen(
                     startTransitionRecursive(child.toolbar)
                 }
                 if (child is ViewGroup) {
+                    if (it is ReactScrollView && it.removeClippedSubviews) {
+                        for (j in 0 until child.childCount) {
+                            child.addView(View(context))
+                        }
+                    }
                     startTransitionRecursive(child)
                 }
             }
diff --git a/common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h b/common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h
index 37aac4e1aca18f5dd537b3599c408abac114d4be..53415e9a8cc663d62373e3e432b4b1ed88e976e5 100644
--- a/common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h
+++ b/common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h
@@ -5,9 +5,9 @@
 #endif
 #include <react/debug/react_native_assert.h>
 #include <react/renderer/components/rnscreens/Props.h>
+#include <react/renderer/components/rnscreens/utils/RectUtil.h>
 #include <react/renderer/core/ConcreteComponentDescriptor.h>
 #include "RNSScreenShadowNode.h"
-#include "utils/RectUtil.h"

 namespace facebook {
 namespace react {
DinhHoangNgoc commented 1 month ago

@DorianMazur It works well, thank you for the help.

DorianMazur commented 1 month ago

FYI: For some reason, in my project this patch is not always working 🤷 That's why I decided to disable removeClippedSubviews globally in the index.js file, like this:

FlatList.defaultProps = FlatList.defaultProps || {}
FlatList.defaultProps.removeClippedSubviews = false
ScrollView.defaultProps = ScrollView.defaultProps || {}
ScrollView.defaultProps.removeClippedSubviews = false
SectionList.defaultProps = SectionList.defaultProps || {}
SectionList.defaultProps.removeClippedSubviews = false
mrenann commented 1 month ago

@DorianMazur I also put it and it worked but with a bottomtabnavigation, when switching between tabs it gives the error No view found for id 0x6fe for fragment ScreentStackFragment

abrah4mr commented 1 month ago

@mrenann If you add detachInactiveScreens={false} fix the error for now.

I already opened an issue for this error with the BottomTab.

hosseinmd commented 1 month ago

@DorianMazur your solution work but doesn't work in all situation, in my app some page still crashing when going back.

NiuGuohui commented 1 month ago

Does anyone have a complete solution?

DorianMazur commented 1 month ago

It worked for me

FlatList.defaultProps = FlatList.defaultProps || {}
FlatList.defaultProps.removeClippedSubviews = false
ScrollView.defaultProps = ScrollView.defaultProps || {}
ScrollView.defaultProps.removeClippedSubviews = false
SectionList.defaultProps = SectionList.defaultProps || {}
SectionList.defaultProps.removeClippedSubviews = false
NiuGuohui commented 1 month ago

It worked for me

FlatList.defaultProps = FlatList.defaultProps || {}
FlatList.defaultProps.removeClippedSubviews = false
ScrollView.defaultProps = ScrollView.defaultProps || {}
ScrollView.defaultProps.removeClippedSubviews = false
SectionList.defaultProps = SectionList.defaultProps || {}
SectionList.defaultProps.removeClippedSubviews = false

I don't think this is a good solution because this problem can be encountered in many scenarios. For example, when using dynamic content in the Drawer of react-native-drawer-layout.

b0iq commented 1 month ago

its still happening 3.35.0-rc.1

hosseinmd commented 1 month ago

i commented entire of startTransitionRecursive and it works for me. https://github.com/software-mansion/react-native-screens/blob/bc95fa466b3a115682db6b74ef5e2b73d830e6cc/android/src/main/java/com/swmansion/rnscreens/Screen.kt#L365

kkafar commented 1 month ago

What architecture is that? Are you all using Fabric or is that Paper?

NiuGuohui commented 1 month ago

What architecture is that? Are you all using Fabric or is that Paper?

Fabric and disable bridgeless.

alduzy commented 1 month ago

I believe it's a duplicate of https://github.com/software-mansion/react-native-screens/issues/2282 and should be resolved by https://github.com/software-mansion/react-native-screens/pull/2307

As a workaround for now you can try setting the removeClippedSubviews option in the FlatList to false or applying the patch pasted by @DorianMazur.

If the problem persists please attach a minimal reproduction.

hosseinmd commented 1 month ago

Is it possible to have an option to disable transition?

hosseinmd commented 1 month ago

https://github.com/software-mansion/react-native-screens/pull/2307 this is not fixed for all situations. I had to remove transition.

alduzy commented 1 month ago

@hosseinmd Could you attach a minimal reproduction?

DorianMazur commented 1 month ago

@alduzy We should reopen this issue. It's not fixed by #2307

hosseinmd commented 1 month ago

Sorry, it is not happening in a regular app. I don't know what is the exact problem, but when I remove transition, it work.

alduzy commented 1 month ago

Ok let's reopen it. Some reproduction snack would be necessary to find out what's broken though. 🙏 I tried the one provided by @DinhHoangNgoc and it works just fine with the fix.

hosseinmd commented 1 month ago

I reproduced the issue with latest version 4.0.0-beta.3 https://github.com/hosseinmd/react-native-screens-new-arch-crash-example the root cause is related to render an image in Flatlist which is rendered in another Flatlist.

alduzy commented 1 month ago

@hosseinmd I can't open it (404). Make sure the example is public

hosseinmd commented 1 month ago

sorry, i make it public now.

alduzy commented 1 month ago

@hosseinmd Thanks! I was able to reproduce the issue and found out that it's caused by the nested flatlist.

As a workaround for now try setting the removeClippedSubviews option in the nested FlatList to false.

github-actions[bot] commented 1 month ago

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

sankar-gp commented 1 month ago

any update?

spyshower commented 3 weeks ago

@alduzy still not fixed.

WKampel commented 3 weeks ago

Happens not only when navigating back for me, but also TO screens. I'm on 4.0.0-beta.9.

alduzy commented 3 weeks ago

@spyshower @WKampel Under what circumstances does it fail, can you attach a reproduction?

spyshower commented 3 weeks ago

@alduzy 2 lists in the same screen, one of them being horizontal, it crashes.

import { View, Text, Pressable, FlatList } from 'react-native';
import s from './styles';

export const Settings = () => {
    const renderItem = ({}) => {
        return <Text style={s.settings_item_label}></Text>;
    };

    const settings = [{}, {}, {}];

    return (
        <>

            <FlatList data={settings} renderItem={renderItem} />

            <FlatList data={settings} renderItem={renderItem} horizontal />
        </>
    );
};
joarkosberg commented 3 weeks ago

@alduzy Will the fix be back ported to v3 also, or is upgrade to v4 required?

alduzy commented 3 weeks ago

@spyshower Thanks for the repro, as you pointed out, the horizontal lists aren't handled properly. https://github.com/software-mansion/react-native-screens/pull/2383 fixes it.

spyshower commented 3 weeks ago

@alduzy not really, I am on ^4.0.0-beta.9

alduzy commented 3 weeks ago

@joarkosberg We're going to backport it to v3 after we release a stable v4.

alduzy commented 3 weeks ago

@spyshower You're right, the current change only considers nested horizontal lists. Thanks for noticing it! I opened a PR that fixes the issue https://github.com/software-mansion/react-native-screens/pull/2403

spyshower commented 3 weeks ago

@alduzy Thank you kind sir!

spyshower commented 2 weeks ago

@alduzy I know this is closed but I have noticed an issue with animating a nested list using reanimated 3.16.0. I get something like this error https://stackoverflow.com/questions/58584676/react-native-filter-flatlist-error-index-10-count-0. Do you think it's related to this one or should I open an issue there?

alduzy commented 2 weeks ago

@spyshower It's hard for me to tell given the information provided. Please open a new issue with some in-depth description and a reproduction code.

talaikis commented 2 weeks ago

Latest beta works fine w. RN 0.75.5

r0b0t3d commented 1 week ago

I'm using react native 0.76 with fabric. Here is my observation

  1. Using 4.0.0-beta.16 --> Works, but I got issue blank screen when going back ( #1685).
  2. Using 3.35.0 --> Not working, but don't have issue blank screen when going back
alduzy commented 1 week ago

@r0b0t3d I am unable to replicate the issues you mentioned on either 4.0.0-beta.16 or 3.35.0. Could you provide some more details? Ideally, having a code snippet for reproduction would be helpful.

EDIT: I managed to reproduce the problem with 4.0.0-beta.16. See https://github.com/software-mansion/react-native-screens/issues/1685