Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
DinhHoangNgoc opened this issue Sep 16, 2024 · 45 comments · Fixed by #2383 or #2403
Closed

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

DinhHoangNgoc opened this issue Sep 16, 2024 · 45 comments · Fixed by #2383 or #2403
Assignees
Labels
NewArch Issues related only to new architecture Platform: Android This issue is specific to Android Repro provided A reproduction with a snack or repo is provided

Comments

@DinhHoangNgoc
Copy link

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.

7006066363014550704.mp4

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

@github-actions github-actions bot added Missing repro This issue need minimum repro scenario Platform: Android This issue is specific to Android labels Sep 16, 2024
@hosseinmd
Copy link

I have the same issue.

@tamacroft
Copy link

same issue here

@DorianMazur
Copy link

@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
Copy link
Author

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

@DorianMazur
Copy link

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
Copy link

mrenann commented Sep 18, 2024

@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
Copy link

abrah4mr commented Sep 18, 2024

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

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

@hosseinmd
Copy link

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

@NiuGuohui
Copy link
Contributor

Does anyone have a complete solution?

@DorianMazur
Copy link

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
Copy link
Contributor

NiuGuohui commented Sep 25, 2024

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.

@kkafar kkafar self-assigned this Sep 25, 2024
@b0iq
Copy link

b0iq commented Sep 29, 2024

its still happening 3.35.0-rc.1

@hosseinmd
Copy link

i commented entire of startTransitionRecursive and it works for me.

@kkafar
Copy link
Member

kkafar commented Sep 29, 2024

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

@NiuGuohui
Copy link
Contributor

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

Fabric and disable bridgeless.

@alduzy
Copy link
Member

alduzy commented Sep 30, 2024

I believe it's a duplicate of #2282 and should be resolved by #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.

@alduzy alduzy closed this as completed Sep 30, 2024
@hosseinmd
Copy link

Is it possible to have an option to disable transition?

@hosseinmd
Copy link

#2307 this is not fixed for all situations. I had to remove transition.

@alduzy
Copy link
Member

alduzy commented Sep 30, 2024

@hosseinmd Could you attach a minimal reproduction?

@DorianMazur
Copy link

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

@hosseinmd
Copy link

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
Copy link
Member

alduzy commented Oct 1, 2024

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.

@alduzy alduzy reopened this Oct 1, 2024
@hosseinmd
Copy link

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.

@github-actions github-actions bot added the Repro provided A reproduction with a snack or repo is provided label Oct 3, 2024
@alduzy alduzy closed this as completed in b67af86 Oct 9, 2024
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
…#2383)

## Description

This PR intents to fix crash happening on Android (fabric) when
navigating back from a screen with nested list.

### Before 
The crash happens whenever the nested list is visible on the screen, as
shown below:

| ✅  | ❌ | ❌ | ❌  | ✅  |
|----------|----------|----------|----------|----------|
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 16"
src="https://github.com/user-attachments/assets/55f36653-9451-48a1-b4fc-9d419622fe2b">
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 25"
src="https://github.com/user-attachments/assets/5cc1af46-4418-40de-9391-6dc5168e08af">
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 34"
src="https://github.com/user-attachments/assets/d2ec614a-fbeb-4717-8aeb-328e3a890cd6">
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 40"
src="https://github.com/user-attachments/assets/5d634fbd-45c9-4612-9dcf-25cf8b9295a0">
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 47"
src="https://github.com/user-attachments/assets/0881dd80-ff98-4ba2-992e-933d549eed19">
|

### After


https://github.com/user-attachments/assets/fb36d030-ef53-493d-a8cc-38be670ee504

Nested lists are rendered as ViewGroups, thus the check for
ReactScrollView fails. This PR fixes the issue by checking wether the
view is an indirect child of a ScrollView with
[removeClippedSubviews](https://reactnative.dev/docs/optimizing-flatlist-configuration#removeclippedsubviews)
enabled.

Fixes: software-mansion#2341

## Changes

- added `isInsideScrollViewWithRemoveClippedSubviews` check to
`startTransitionRecursive`
- modified `Test2282.tsx` repro

<!--

## Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

### Before

### After

-->

## Test code and steps to reproduce

- use `Test2282.tsx` repro

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
@spyshower
Copy link

@alduzy still not fixed.

@WKampel
Copy link

WKampel commented Oct 12, 2024

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

@alduzy
Copy link
Member

alduzy commented Oct 14, 2024

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

@spyshower
Copy link

spyshower commented Oct 14, 2024

@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
Copy link

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

@alduzy
Copy link
Member

alduzy commented Oct 14, 2024

@spyshower Thanks for the repro, as you pointed out, the horizontal lists aren't handled properly. #2383 fixes it.

@spyshower
Copy link

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

@alduzy
Copy link
Member

alduzy commented Oct 14, 2024

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

@alduzy
Copy link
Member

alduzy commented Oct 14, 2024

@spyshower You're right, the current change only considers nested horizontal lists. Thanks for noticing it!
I opened a PR that fixes the issue #2403

@alduzy alduzy reopened this Oct 14, 2024
@spyshower
Copy link

@alduzy Thank you kind sir!

@spyshower
Copy link

@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
Copy link
Member

alduzy commented Oct 21, 2024

@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
Copy link

Latest beta works fine w. RN 0.75.5

kkafar pushed a commit that referenced this issue Oct 25, 2024
This PR intents to fix crash happening on Android (fabric) when
navigating back from a screen with nested list.

The crash happens whenever the nested list is visible on the screen, as
shown below:

| ✅  | ❌ | ❌ | ❌  | ✅  |
|----------|----------|----------|----------|----------|
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 16"
src="https://github.com/user-attachments/assets/55f36653-9451-48a1-b4fc-9d419622fe2b">
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 25"
src="https://github.com/user-attachments/assets/5cc1af46-4418-40de-9391-6dc5168e08af">
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 34"
src="https://github.com/user-attachments/assets/d2ec614a-fbeb-4717-8aeb-328e3a890cd6">
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 40"
src="https://github.com/user-attachments/assets/5d634fbd-45c9-4612-9dcf-25cf8b9295a0">
| <img width="247" alt="Screenshot 2024-10-03 at 16 12 47"
src="https://github.com/user-attachments/assets/0881dd80-ff98-4ba2-992e-933d549eed19">
|

https://github.com/user-attachments/assets/fb36d030-ef53-493d-a8cc-38be670ee504

Nested lists are rendered as ViewGroups, thus the check for
ReactScrollView fails. This PR fixes the issue by checking wether the
view is an indirect child of a ScrollView with
[removeClippedSubviews](https://reactnative.dev/docs/optimizing-flatlist-configuration#removeclippedsubviews)
enabled.

Fixes: #2341

- added `isInsideScrollViewWithRemoveClippedSubviews` check to
`startTransitionRecursive`
- modified `Test2282.tsx` repro

<!--

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

-->

- use `Test2282.tsx` repro

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

(cherry picked from commit b67af86)
kkafar pushed a commit that referenced this issue Oct 25, 2024
## Description

This PR handles yet another case for going back on fabric. In the
previous one
(#2383) I
omitted a case where a horizontal list is simply rendered w/o being
nested in any other list!

Fixes #2341

## Changes

- updated `Test2292.tsx` repro
- handle `parentView is ReactHorizontalScrollView`

<!--

## Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

### Before

### After

-->

## Test code and steps to reproduce

- use `Test2282.tsx` repro

## Checklist

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

(cherry picked from commit d40e108)
@r0b0t3d
Copy link
Contributor

r0b0t3d commented Oct 29, 2024

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 ( When I want to go to the previous page, everything suddenly disappears #1685).
  2. Using 3.35.0
    --> Not working, but don't have issue blank screen when going back

@alduzy
Copy link
Member

alduzy commented Oct 29, 2024

@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 #1685

@dppo
Copy link

dppo commented Nov 16, 2024

This error still exists when using react-native-drawer-layout, similar to 12248. Are there any other fixes?

@dppo
Copy link

dppo commented Nov 16, 2024

I just found this PR which has been solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NewArch Issues related only to new architecture Platform: Android This issue is specific to Android Repro provided A reproduction with a snack or repo is provided
Projects
None yet