React-native-navigation: [V2] componentWillUnmount not running on screen pop.

[SudoPlz]

Issue Description

I'm pushing a screen like normal, with Navigation.push, and then pop it w/ Navigation.pop.
I noticed after the pop takes place, the animation runs, the screen is no longer visible, bot componentWillUnmount doesn't run.

As I investigated further I found out that Navigation.pop never returns anything, the promises that it's supposed to return NEVER resolves, or rejects.

I decided to dig further into the code and found out that this never runs and that's because this never runs.

Now I tried to reproduce that on the playground app but couldn't. Not sure why that's happening on my end.

GOOD NEWS though, I also found this SO answer and tried surrounding the animation block with a main thread block.

After that the issue was gone!!
The completion block runs again and so does componentWillUnmount on every pop.

I still have no idea what's causing it, but here's how the new code looks like:

[self performAnimationBlock:^{
        poppedVC = [viewController.navigationController popViewControllerAnimated:animated];
}

is now:

[self performAnimationBlock:^{
        dispatch_async(dispatch_get_main_queue(), ^{
            poppedVC = [viewController.navigationController popViewControllerAnimated:animated];
        });
}

---

Environment

• React Native Navigation version: 2.26.2
• React Native version: 0.59.9
• Platform(s) (iOS, Android, or both?): iOS
• Device info (Simulator/Device? OS version? Debug/Release?): iPad Pro 12.9 Inch 3rd Generation iOS 12.4 Simulator

[SudoPlz]

@guyca this is the issue I've been telling you about ^, it doesn't reproduce on the playground app, but I'm pretty sure one of your iOS devs will have a better idea as to why that's happening.

[SudoPlz]

should I open a PR? Would you accept my code or is it too early to make assumptions?

[guyca]

@SudoPlz
This is a real strange one. @yogevbd and I went over the code and we can't think of a reason why the completion block won't fire.

• I suggest you check if performAnimationBlock is called from the main thread. If somehow it isn't, that might explain what you're observing.
• Perhaps the completion block isn't fired because the ViewController is actually leaking. If you could dig into that and find a leak, that would help us find the "real" fix to this issue.

Please let me know if I can help more

[SudoPlz]

@guyca
performAnimationBlock is running from the main thread just checked. 😞
I have no idea how to test if the view controller is leaking :/ any hints?
Should I check for zombie objects w/ instruments?

[SudoPlz]

I'm out of ideas, spent the whole day looking for the issue, but couldn't track it down.

My guess is that [CATransaction setCompletionBlock:] enqueues some work on the main queue that takes care of setting the completion, so by wrapping popViewControllerAnimated: in a dispatch async we’re waiting for that work to complete before triggering the animation.
That should be why the issue is fixed now.

What I've learned though is that:

1) Things like any other animation taking place right after a pop happens i.e an ActivityIndicator running can mess things up <-- example.

2) The completion block will not call if there is nothing to animate or the transition part already done by some another part of your source code and the code snippet runs on different thread

I'm wondering why did you implement this via a UINavigationControllerDelegate like so?

[SudoPlz]

For now I've opened a PR to stop this from happening. I don't foresee any possible harm making sure animations run on main thread may bring.

[guyca]

@SudoPlz Sorry for not being responsive, was a bit busy with v3.
Seems like this change broke some unit tests, I don't mind merging this if ci passes 👍

[SudoPlz]

@guyca no worries, let me know if you need me to do anything

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest Detox and report back. Thank you for your contributions.

[SudoPlz]

@guyca looks like this is going to be closed by the stale bot:/

[Crysp]

i have same issue with navigation... after pop animation screen still exists. and sometimes i've got a error Error: popping component failed after dismissOverlay and trying to pop current screen

[tootsweet]

We have this problem as well.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest Detox and report back. Thank you for your contributions.

[tootsweet]

This is not stale.

We have been able to circumvent this problem by avoiding any animations in our screens under iOS (we replaced loaders with animated GIFs), but it's obviously a bad solution.

Someone has to take a look a this and fix it in v3.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest Detox and report back. Thank you for your contributions.

[phuongwd]

This is not stale.

[tiennv-0883]

i have same issue with navigation .after pop animation screen still exists. and sometimes i've got a error Error: popping component failed

[SudoPlz]

@guyca can you merge the above PR? It's hard to keep the issue from going stale and this issue has been sitting like that for the past 5-6 months.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest Detox and report back. Thank you for your contributions.

[guyca]

@SudoPlz Seems like this is a bigger issue we need to look into. I closed your PR and we'll look into this asap.

[phuongwd]

Hi @guyca @yogevbd

This issue still happening if the animation of Lottie component is runs.

[guyca]

@phuongwd Please create a separate issue and include a reproduction