fix(android): correctly end view transitions

[hannojg]

Description

Fixes https://github.com/software-mansion/react-native-screens/issues/3249

[!NOTE] This PR still contains the reproduction code necessary + it contains changes related to RN core. This is meant so we can have a conversation about this issue and see how to apply the fixes best.

There are two problems which this PR tries to face to fix usage with recycling views:

1. Screen#endRemovalTransition is broken:

https://github.com/software-mansion/react-native-screens/blob/4982d4c44bcc788448337ce55e3329c57b87e37a/android/src/main/java/com/swmansion/rnscreens/Screen.kt#L472-L477

Here you can see that we try to iterate over the children of the screen to end the view transition. The problem is that the react-native mounting layer already has all the children removed from the parent. The flow of events is:

• The user requests to remove a screen, we call through NativeProxy startRemovalTransition. All children will be marked as "in-transition"
• In JS react our screen component gets removed (i believe thats how its working, I might be wrong) and the react-native mounting layer will call removeView for all the children of our screen
• The views will remain visible as we marked them as "in transition". HOWEVER, the children will already get removed from the parents mChildren list. Only the children will keep their mParent field set
• We call endRemovalTransition once the transition is done, trying to iterate over the children, but they are already unset. You can verify this by logging the children count when this method gets called
• 🐛 Bug: Our views, that may get recycled, are never called with endViewTransition(child) and their mParent field never gets reset. Thus the react-native mounting layer will always crash when trying to reuse this view.

2. Interrupting a screen transition can cause a wrong order of events

We rely on Screen#endRemovalTransition to be called before ScreenStack#endRemovalTransition. ScreenStack is in some way the parent of Screen. So the parent of the screen first calls endViewTransition. This is problematic because:

1. When a parent gets called with endViewTransition it will loop over all children and call dispatchDetachedFromWindow():
   • ViewGroup#endViewTransiton
   • ViewGroup#dispatchDetachedFromWindow
2. The problem with that is that the children will be simply detached, and cleared from the mDisappearingChildren
3. Later, when we call Screen#endRemovalTransition and loop over the views calling endViewTransition will have no effect, since the views are no longer marked as disappearing:
   • https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/view/ViewGroup.java;l=7229-7233
4. 🐛 Bug: Due to this the children's mParent field never gets reset. Thus the react-native mounting layer will always crash when trying to reuse this view.

Changes

1. Fixing Screen#endRemovalTransition is broken:

I simply added a list of parent-child pairs that we add to in startRemovalTransition. Then when endRemovalTransition gets called we loop over that list.

2. Fixing interrupting a screen transition can cause a wrong order of events:

Two fixes are necessary for this:

1. In ScreenStack we call Screen#endViewTransition first, before calling into super.endViewTransition. I think this is safe since Screen#endRemovalTransition has a flag checking if we are currently in a removal transition or not
2. We need to call endViewTransition from the most inner children up to the top parent so we avoid the problem explained above, where calling endViewTransition on a parent will remove all children from mDisappearingChildren list

Additional changes in react-native core

As you can see I patched RN + override the default ReactViewManager. I think we also need to address this in react-native somehow. There are two things that need to be addressed I believe:

• When recycling views (onDropViewInstance) we could check if a view still has a parent. If so we could assume its in transition (or maybe have it marked as such earlier), and only add the view to the recyling pool on detach (ie. when the transition has ended)
• This crash seems not just reproducible with recycling views. It was just one way for me to get it to reproduce, but it seems that there might be other code paths where the same native view is trying to be reused (at least thats what the crash report's telemetry suggest me) [also in the app where this is happening we don't have view recycling enabled]. So additionally we might want to add something to addView in the ReactViewManager / BaseViewManager that will check if a view is "in-transition" / has a parent, and only attach the view once the transition is done (ie. on detach)

➤ I think that we can still apply the changes in RNS without needing to wait for react-native core though.

Screenshots / GIFs

Before	After

Test code and steps to reproduce

You can simply run this PR and go to the TestRecylingViews tab in the example app to play with it.

Checklist

• [x] Included code example that can be used to test this change
• [x] Updated TS types
• [x] Updated documentation:
  • [x] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
  • [x] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
  • [x] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
  • [x] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
• [x] Ensured that CI passes

[hannojg]

As for the view recycling part, I'll admit that we have not considered up to this moment much, as, correct me if I'm wrong, it is disabled by default on Android.

yes thats correct, also not sure if this is ever gonna land in stable 😅 It was just one way for me to reproduce this crash.

ReactViewGroup tracks which children have been removed during transition and which were not -> this is information we could inspect from child view to make sure, we've been removed from parent while being in transition

Ah yeah, thats nice, i will experiment with that!

---

Okay cool, so then i'd remove everything else which i added for reproduction from this PR and we can move forward with the main change? 😊

[kkafar]

Yes, we can. Let's get this PR up to date & I think we can proceed.

[hannojg]

Cool, there is just one thing that might needs to be changed. I had this change in production and saw today the following reports in sentry:

java.util.ConcurrentModificationException: null
    at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1111)
    at java.util.ArrayList$ListItr.previous(ArrayList.java:1138)
    at kotlin.collections.ReversedList$listIterator$1.next(ReversedViews.kt:48)
    at com.discord.view.ScreenOverride.endRemovalTransition(ScreenViewManagerOverride.kt:96)
    at com.swmansion.rnscreens.ScreenStackFragment.onViewAnimationEnd(ScreenStackFragment.kt:141)
    at com.swmansion.rnscreens.stack.views.ScreensCoordinatorLayout$animationListener$1.onAnimationEnd(ScreensCoordinatorLayout.kt:38)
    at android.view.animation.Animation.dispatchAnimationEnd(Animation.java:1171)
    at android.view.animation.AnimationSet.getTransformation(AnimationSet.java:418)
    at android.view.animation.Animation.getTransformation(Animation.java:1190)
    at android.view.View.applyLegacyAnimation(View.java:25871)
    at android.view.View.draw(View.java:26010)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4810)
    at com.swmansion.rnscreens.ScreenStack.performDraw(ScreenStack.kt:335)
    at com.swmansion.rnscreens.ScreenStack.access$performDraw(ScreenStack.kt:20)
    at com.swmansion.rnscreens.ScreenStack$DrawingOp.draw(ScreenStack.kt:348)
    at com.swmansion.rnscreens.ScreenStack.drawAndRelease(ScreenStack.kt:304)
    at com.swmansion.rnscreens.ScreenStack.dispatchDraw(ScreenStack.kt:314)
    at android.view.View.updateDisplayListIfDirty(View.java:25161)
    at android.view.ViewGroup.recreateChildDisplayList(ViewGroup.java:4794)
    ...

I can see from the breadcrumbs, that this crash happens, while we are still executing startRemovalTransition (there is only a 9ms difference between the two calls). I think the problem here is that startRemovalTransition can be called from the JS thread, setting isBeingRemoved = true. Then on the UI thread the animation is very quickly done , and already calling endRemovalTransition, concurrently modifying the intransitionViews list.

My fix for this right now would be to always run startRemovalTransition from the UI thread. What do you think about that change? Would look something like:

    // Note: this can be called from the JS thread 👀
   fun startRemovalTransition() {
        // Dispatch on the main thread if not already on it
        val mainLooper = Looper.getMainLooper()
        if (Looper.myLooper() != mainLooper) {
            Handler(mainLooper).post {
                startRemovalTransition()
            }
            return
        }

        if (isBeingRemoved) return
        isBeingRemoved = true

        startTransitionRecursive(this)
    }

[kkafar]

I see. The problem here is that we had at least two attempts to schedule the call to startRemovalTransition on UI & we backed out. I'll try to find the old PRs

[kkafar]

We have this one here: https://github.com/software-mansion/react-native-screens/pull/2964 & it seems I remembered things wrong. We just didn't finalise the PR.

I'll see to that one first & see whether we can land it before we land this one.

[kkafar]

I've landed https://github.com/software-mansion/react-native-screens/pull/2964, we can update this PR now

[michael-pingo-ai]

I believe this PR might fix the issue I’ve been seeing with @react-navigation/stack: GSTJ/react-native-magic-modal#155. We’re encountering it for roughly 10% of Android users, particularly on devices with stricter view teardown behavior — it seems to be timing-related.