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

fix: handle React Native loaded on Fragment #1553

Merged
merged 13 commits into from
Aug 16, 2022
Merged

fix: handle React Native loaded on Fragment #1553

merged 13 commits into from
Aug 16, 2022

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Aug 11, 2022

Description

Fixes #1506

ScreenContainer#setupFragmentManager method resolves FragmentManager instance for ScreenContainer to use for conducting transactions. Until now, when first (not nested) ScreenContainer was initialised we took FragmentManager from top level activity, but it leads to conflicts with native Android navigation when React Native is not loaded directly in main activity (there are cases (see #1506) when both react-native-screens & Android native navigation use simultaneously the same FragmentManager leading to crashes).

Changes

Added a method resolving fragment manager whose fragment's view is ReactRootView.

Test code and steps to reproduce

See #1506 for crash description and how to reproduce.

Checklist

  • Included code example that can be used to test this change
  • Ensured that CI passes

@kkafar kkafar marked this pull request as ready for review August 11, 2022 12:47
Copy link
Member Author

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment below

Comment on lines 160 to 165
// We are in some custom setup & we want to use the closest fragment manager in hierarchy.
// `findFragment` method throws IllegalStateException when it fails to resolve appropriate
// fragment. It might happen when e.g. React Native is loaded directly in Activity
// but some custom fragments are still used. However such use case seems highly unlikely
// so, as for now, we let application crash.
FragmentManager.findFragment<Fragment>(rootView).childFragmentManager
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially we could catch the exception here and fallback to context.supportFragmentManager:

Suggested change
// We are in some custom setup & we want to use the closest fragment manager in hierarchy.
// `findFragment` method throws IllegalStateException when it fails to resolve appropriate
// fragment. It might happen when e.g. React Native is loaded directly in Activity
// but some custom fragments are still used. However such use case seems highly unlikely
// so, as for now, we let application crash.
FragmentManager.findFragment<Fragment>(rootView).childFragmentManager
// We are in some custom setup & we want to use the closest fragment manager in hierarchy.
// `findFragment` method throws IllegalStateException when it fails to resolve appropriate
// fragment. It might happen when e.g. React Native is loaded directly in Activity
// but some custom fragments are still used. However such use case seems highly unlikely
// so, as for now, we let application crash.
try {
FragmentManager.findFragment<Fragment>(rootView).childFragmentManager
} catch (IllegalStateException e) {
context.supportFragmentManager
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change added with 714ddea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Android - Crash): FragmentManager is already executing transactions
1 participant