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

Multiple transitions between screens causes app to crash #2578

Open
marklawlor opened this issue Dec 17, 2024 · 14 comments
Open

Multiple transitions between screens causes app to crash #2578

marklawlor opened this issue Dec 17, 2024 · 14 comments
Assignees
Labels
Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided

Comments

@marklawlor
Copy link

Description

Repeated navigation between 2 screens using React Navigation's Native Stack with a getId() prop causes the app to "crash". I use the term crash loosely as the behaviour is different per platform but both result in the app becoming unusable.

iOS

react-native elements that use Pressibility become stuck in their "onPress" state, preventing them from receiving further onPress events. The issue does not occur with non-Pressibility elements such as <TouchableOpacity />.

You can observe this behaviour with the reproduction snack by repeatedly moving between the two screens using the Forwards/Backwards <Text /> items. After 3 transitions, the <Text /> items are no longer pressable and will be rendered with their onPress highlighting.

Android

After 3 transitions, the native stack header will disappear. After 4 transitions, the native stack will no longer render any visible context.

Expo Router issue: expo/expo#33658
React Navigation issue: react-navigation/react-navigation#12346

Steps to reproduce

  1. Open Snack
  2. Click "forwards" / "backwards" 3-5 times

Snack or a link to a repository

https://snack.expo.dev/@mwlawlor/react-navigation-native-stack-stuck

Screens version

3.3.0

React Native version

0.76.3

Platforms

Android, iOS

JavaScript runtime

None

Workflow

None

Architecture

None

Build type

None

Device

None

Device model

No response

Acknowledgements

Yes

@github-actions github-actions bot added Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided labels Dec 17, 2024
@kkafar kkafar self-assigned this Dec 18, 2024
@Mokhtar25
Copy link

Facing the same issue right now

@mlukasik-dev
Copy link

My app with expo-router is affected by this problem.

@kashmiry
Copy link

kashmiry commented Jan 1, 2025

I am also facing this issue on Android.
On iOS everything works fine.

When I navigate back to the same screen with the same getId, I observe the described behavior.
However, in my actual app, where I have a lot of integrations, this issue results in a crash with the following error:

The specified child already has a parent. You must call removeView() on the child's parent first

The temporary fix working for me now on Android is conditionally using navigateDeprecated if Platform is Android:
navigation.navigateDeprecated(name, params);

Package Version
react 18.2.0
react-native 0.74.6
@react-navigation/bottom-tabs 7.2.0
@react-navigation/native ^7.0.14
@react-navigation/native-stack 7.2.0
react-native-screens ^4.4.0
react-native-reanimated 3.16.6
react-native-gesture-handler 2.21.2
react-native-pager-view ^6.6.1

@pizzascott
Copy link

pizzascott commented Jan 10, 2025

This is causing Pizzahuts app to freeze on IOS 18 devices and simulators.

@pizzascott
Copy link

Does anyone have a work around?

@Mokhtar25
Copy link

Does anyone have a work around?

Use push, and dismiss. Instead of navigate

@kkafar
Copy link
Member

kkafar commented Jan 10, 2025

removing the getId prop from reproducer fixes the issue. react-native-screens does not have any getId related logic. Are there any premises that this is react-native-screens issue? @marklawlor

@kkafar
Copy link
Member

kkafar commented Jan 10, 2025

Okay, I have high level grasp of the issue mechanism now.

Let's have a following stack of screens: A B and let's set getId prop for both screens with constant values (not depending on route params).

Now, in react-navigation@v7, navigate(A) instead of popping the B, will reshuffle screens on the stack, to achieve the state B A.
When getId is not present this will work fine, because "new navigate" will just push a brand new screen A, instead of navigating the the one already on the stack, so the resulting stack state would be A B A.

In previous version of react-navigation the navigate(A) call would just pop B and "navigate to the screen already on stack as the method name suggests", which would result in stack state: A.

Now, the issue arises because reshuffling the screens on stack is not really supported & it seems that it has not been the case for a longer while, as it was not needed. Moreover, the crash that happens (and e.g. causes the header to disappear) suggests that this is due to asynchronous way we do the updates - which won't be easily changed.

Immediate solution

use navigateDeprecated instead of navigate in your apps

Proper solution

One of:

  • restore original behaviour of navigate from before react-navigation@v7,
  • change the behaviour of navigate for native-stack only,
  • rewrite container update logic of screens - dunno if that's feasible - we do it in asynchronous way for a reason.

Tagging @satya164 as I think we should handle this on react-navigation level. Current behaviour of navigate is against stack paradigm leading to above issue. Do you think restoring navigate behaviour in future react-navigation versions is feasible?

@satya164
Copy link
Collaborator

@kkafar this behavior now being default with navigate only makes it easier to see the problem, but it's not the only way to surface the issue (e.g. you can also have this issue with reset or other actions - in v6 it can happen when using push and getId) so changing it will only make the issue less common - but not fix it. it's fundamental to how react navigation works and declarative nature of react.

from native perspective, is it not possible to take the same view containing the screens and render it in a new screen if it was reordered?

@kkafar
Copy link
Member

kkafar commented Jan 10, 2025

I haven't debugged it to very bottom, because the interaction between react-native's native mounting-stage code & Android Fragment transactions we use is quite complex, but the issue arises due to following:

  1. Having stack view S with two children A B
  2. UI block 1: React removes B from S,
  3. UI block 1: we schedule removal of B from S on the UI queue (block 2),
  4. UI block 1: React inserts B at index 0 into S,
  5. UI block 1: we schedule removal of A, insertion of B and insertion of A on the UI queue (block 3)
  6. block 2 executes
  7. block 3 executes

The issue appears at point 4 - react tries to insert B, however it has been not yet effectively detached and still has a parent (still attached to S), because we didn't perform the update - we've just scheduled it for later execution.

Idk what would be technical solution here yet. I'll start discussion internally on how we could start supporting such "reshuffling".

I still wonder whether allowing for screen reshuffling in stack-based navigators is ok. From my current perspective this seems against the 'stack' logic. I wonder what are the use cases that people might consciously choose reshuffling the screens in stack. My guess is that most of the time this decision is not deliberate.

@pizzascott
Copy link

What package is navigateDeprecated in? I can't see it under CommonActions?

@kkafar
Copy link
Member

kkafar commented Jan 10, 2025

@pizzascott its on navigation object in react-navigation https://reactnavigation.org/docs/navigation-object/#navigatedeprecated

@pizzascott
Copy link

Ah I think we are on an older version so already using deprecated. Which is why android is fine for our app, but IOS is breaking.

Still haven’t found a work around and seems to happen when changing tab index also.

@pizzascott
Copy link

Temp fix for IOS is to use StackActions.replace

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

No branches or pull requests

7 participants