Support incremental migration to Navigation Compose [265480755]

Assigned

Feature Request

Status Update

No update yet.

Description

ch...@google.com

created issue #1

Jan 14, 2023 12:02AM

Component used: Navigation Version used: 2.5.3

Use Case

I would like to be able to incrementally migrate to Navigation Compose while migrating a Fragment View-based app to Compose.

Problem Statement

When migrating a Fragment View-based app that is using Navigation Component to Compose, it's not currently possible to incrementally migrate to Navigation Compose. Instead, all screens first need to be migrated to Compose and only then can apps migrate to Navigation Compose.

While this might work for smaller apps, this may not be feasible for apps with a lot of screens as the switch would be a pretty large change all at once. Additionally, new features or screens would still need to be wrapped inside a Fragment which is tech debt as the eventual goal is to remove fragments altogether. This experience can be improved by creating better interop between Fragment and Compose Navigation.

Possible Solution

Add a single Compose container fragment in the navigation graph. This container can then add new composable destinations to the existing XML navigation graph. This would also avoid new screens having to create new fragments altogether and migrated screens (composables) can then be incrementally added to the graph. Once all screens are in Compose, the switch to remove Fragment-based navigation should be a smaller change.

Comments

mv...@google.com <mv...@google.com> #2Apr 5, 2023 08:09AM

Yigit, do you have time to fix it?

reemission of the same liveData is racy

il...@google.com <il...@google.com> #3May 31, 2023 11:16PM

yea i'll take it.

ch...@google.com <ch...@google.com> #4Jun 1, 2023 06:44PM

Thanks for the detailed analysis. This may not be an issue anymore since we've started using Main.immediate there but I' not sure; I'll try to create a test case.

an...@google.com <an...@google.com> #5Jun 2, 2023 03:50PM

just emitting same live data reproduces the issue.

@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}

il...@google.com <il...@google.com> #6Jun 12, 2023 05:28PM

With 2.2.0-alpha04 (that use Main.immediate), the issue seems to be still there (I tested it by calling emitSource() twice, like your test case)

ni...@hinge.co <ni...@hinge.co> #7Jun 27, 2023 06:14PM

yea sorry immediate does not fix it.
I actually have a WIP fix for it:

https://android-review.googlesource.com/c/platform/frameworks/support/+/1112186

if your case is the one i found (emitting same LiveData multiple times, as shown in #5) you can work around it by adding a dummy transformation.

val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData.map {it })
emitSource(subLiveData.map {it} )
}

mr...@gmail.com <mr...@gmail.com> #8Jul 26, 2023 11:44PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit af12e75e6b4110f48e44ca121466943909de8f06
Author: Yigit Boyar <yboyar@google.com>
Date: Tue Sep 03 12:58:11 2019

Fix coroutine livedata race condition

This CL fixes a bug in liveData builder where emitting same
LiveData source twice would make it crash because the second
emission registry could possibly happen before first one is
removed as source.

We fix it by using a suspending dispose function. It does feel
a bit hacky but we cannot make DisposableHandle.dispose async
and we do not want to block there. This does not mean that there
is a problem if developer disposes it manually since our emit
functions take care of making sure it disposes (and there is
no other way to add source to the underlying MediatorLiveData)

Bug: 140249349
Test: BuildLiveDataTest#raceTest_*

Change-Id: I0b464c242a583da4669af195cf2504e2adc4de40

M lifecycle/lifecycle-livedata-ktx/api/2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/current.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_current.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_current.txt
M lifecycle/lifecycle-livedata-ktx/src/main/java/androidx/lifecycle/CoroutineLiveData.kt
M lifecycle/lifecycle-livedata-ktx/src/test/java/androidx/lifecycle/BuildLiveDataTest.kt

https://android-review.googlesource.com/1112186

https://goto.google.com/android-sha1/af12e75e6b4110f48e44ca121466943909de8f06

jb...@google.com <jb...@google.com> Jan 30, 2024 04:24PM

Assigned to jb...@google.com.

st...@gmail.com <st...@gmail.com> #9Feb 21, 2024 08:09PM

Any updates here?

jb...@google.com <jb...@google.com> Apr 1, 2024 06:17PM

Reassigned to il...@google.com.

ap...@google.com <ap...@google.com> #10Apr 10, 2024 10:59PM

Project: platform/frameworks/support
Branch: androidx-main

commit 3792a45bd2a72af6915853bce71f2241843c9b70
Author: Ian Lake <ilake@google.com>
Date: Fri Mar 15 22:33:28 2024

Add project template for navigation-fragment-compose

Use the create_project.py script to create the
navigation-fragment-compose module.

Strips out the multiplatform support as
Navigation with Fragments (which this library extends)
will remain a single platform library.

Relnote: N/A
Test: Compose Studio opens
BUG: 265480755
Change-Id: Ia3dd103c856c470142fd2af40851ccf9a4bc5aa3

M docs-tip-of-tree/build.gradle
A navigation/navigation-fragment-compose/api/current.txt
A navigation/navigation-fragment-compose/api/res-current.txt
A navigation/navigation-fragment-compose/api/restricted_current.txt
A navigation/navigation-fragment-compose/build.gradle
A navigation/navigation-fragment-compose/src/main/java/androidx/navigation/androidx-navigation-navigation-fragment-compose-documentation.md
M settings.gradle

https://android-review.googlesource.com/3037820

ap...@google.com <ap...@google.com> #11Apr 11, 2024 04:12PM

Project: platform/frameworks/support
Branch: androidx-main

commit fca90c019bd25cc72ee9069af5ddbf51d48af99c
Author: Ian Lake <ilake@google.com>
Date: Sat Mar 16 00:02:31 2024

Add ComposableFragment

Create a generic ComposableFragment class that can
load its entire contents from a Composable method
that is given to it via a fully qualified name.

This has some limitations:
- the Composable method must be a static, top-level method
- the Composable method must take no arguments

Relnote: N/A
Test: new ComposableFragmentTest
BUG: 265480755
Change-Id: Ibceff877ee5ef8d4097b1f3e04b51e5d6e6be3eb

M navigation/navigation-fragment-compose/api/current.txt
M navigation/navigation-fragment-compose/api/restricted_current.txt
M navigation/navigation-fragment-compose/build.gradle
A navigation/navigation-fragment-compose/src/androidTest/AndroidManifest.xml
A navigation/navigation-fragment-compose/src/androidTest/java/androidx/navigation/fragment/compose/ComposableFragmentTest.kt
A navigation/navigation-fragment-compose/src/androidTest/java/androidx/navigation/fragment/compose/test/TestActivity.kt
A navigation/navigation-fragment-compose/src/androidTest/res/layout/activity_main.xml
A navigation/navigation-fragment-compose/src/main/java/androidx/navigation/fragment/compose/ComposableFragment.kt

https://android-review.googlesource.com/3037821

ap...@google.com <ap...@google.com> #12Apr 11, 2024 04:42PM

Project: platform/frameworks/support
Branch: androidx-main

commit 15b93a31800c4d1e86fb93ac9a793b7e08b6d892
Author: Ian Lake <ilake@google.com>
Date: Tue Mar 26 20:21:40 2024

Add ComposableFragmentNavigator

Add a Navigator that intercepts the inflation
of composable destinations and rewrites them as
FragmentNavigator.Destination instances that use
ComposableFragment as their implementation, passing
along the class to load via reflection as an
argument.

Relnote: N/A
Test: new ComposableFragmentNavigatorTest
BUG: 265480755
Change-Id: Ic70781f6b2d72f0c2d3ced183b969a708249afc4

M navigation/navigation-fragment-compose/api/current.txt
M navigation/navigation-fragment-compose/api/restricted_current.txt
A navigation/navigation-fragment-compose/src/androidTest/java/androidx/navigation/fragment/compose/ComposableFragmentNavigatorTest.kt
A navigation/navigation-fragment-compose/src/androidTest/res/navigation/nav_simple.xml
M navigation/navigation-fragment-compose/src/main/java/androidx/navigation/fragment/compose/ComposableFragment.kt
A navigation/navigation-fragment-compose/src/main/java/androidx/navigation/fragment/compose/ComposableFragmentNavigator.kt

https://android-review.googlesource.com/3037822

ap...@google.com <ap...@google.com> #13Apr 11, 2024 04:48PM

Project: platform/frameworks/support
Branch: androidx-main

commit b40ba45605bd3af6b2ce01b94e60acb86952f000
Author: Ian Lake <ilake@google.com>
Date: Wed Apr 10 21:19:43 2024

Add ComposableNavHostFragment

Add a convenience extension on NavHostFragment
that automatically adds the
ComposableFragmentNavigator. This ensures that
developers can directly substitute
NavHostFragment for ComposableNavHostFragment in
a layout XML file.

Relnote: "Adds a new `navigation-fragment-compose`
artifact that includes a `ComposableNavHostFragment`
alternative to `NavHostFragment` that allows you to
add `composable` destinations to your Navigation XML
files. Each `composable` destination must be
expressed as a top-level, no argument `@Composable`
method whose fully qualified name is used as the
`android:name` attribute on each destination. When
navigating to one of these destinations, a containing
fragment is created to display the composable content."
BUG: 265480755
Test: new ComposableNavHostFragmentTest

Change-Id: I0ef2e5897128d18d1b374f3617c7095022ca7c0e

M navigation/navigation-fragment-compose/api/current.txt
M navigation/navigation-fragment-compose/api/restricted_current.txt
A navigation/navigation-fragment-compose/src/androidTest/java/androidx/navigation/fragment/compose/ComposableNavHostFragmentTest.kt
A navigation/navigation-fragment-compose/src/main/java/androidx/navigation/fragment/compose/ComposableNavHostFragment.kt

https://android-review.googlesource.com/3037636

ap...@google.com <ap...@google.com> #14Apr 15, 2024 08:54PM

Project: platform/frameworks/support
Branch: androidx-main

commit 261c755a06f74528077df263d847cfa2657ff89e
Author: Ian Lake <ilake@google.com>
Date: Fri Apr 12 21:07:39 2024

Add LocalFragment to navigation-fragment-compose

Add a composition local that allows a composable
method inside a ComposableFragment to access the
Fragment instance.

This allows the method to extract arguments,
set transitions on the Fragment instance, and
otherwise avoid cases where you'd manually need
to construct a Fragment with a ComposeView as
its view just to access Fragment related properties.

Relnote: "The `navigation-fragment-compose` artifact
now provides a `LocalFragment` composition local to
composable methods within a `ComposableFragment`."
BUG: 265480755
Test: new ComposableFragmentTest added

Change-Id: If35e5405bbd94fd5f45c1cc0dfe3d74278d64a09

M navigation/navigation-fragment-compose/api/current.txt
M navigation/navigation-fragment-compose/api/restricted_current.txt
M navigation/navigation-fragment-compose/src/androidTest/java/androidx/navigation/fragment/compose/ComposableFragmentTest.kt
M navigation/navigation-fragment-compose/src/main/java/androidx/navigation/fragment/compose/ComposableFragment.kt
A navigation/navigation-fragment-compose/src/main/java/androidx/navigation/fragment/compose/LocalFragment.kt

https://android-review.googlesource.com/3040302

ca...@gmail.com <ca...@gmail.com> #15Jul 3, 2024 10:51AM

why it is not possible to pass de backStackEntry to the composable function in order to get the some arguments inside the composable function?. This feature is very nice but it solves only a part of the problem, the majority of destinations have arguments being passed down.

il...@google.com <il...@google.com> #16Jul 3, 2024 08:26PM

Re comment#15 - if you look at the test added in comment#14 where we added LocalFragment, you'll see retrieving arguments is exactly the use case for that API using LocalFragment.current.requireArguments()

je...@pinkfroot.com <je...@pinkfroot.com> #17Aug 28, 2024 10:13AM

Is there a way to make this work with the Safe Args plugin? As soon as I add an action to a composable destination like so:

<composable
  android:id="@+id/home_screen"
  android:name="com.example.HomeScreenKt\$HomeScreen" >
  <action
    android:id="@+id/action_home_screen_to_other_fragment"
    app:destination="@id/other_fragment" />
</composable>

My project build fails on the :app:generateSafeArgs task with the following error:

Can't escape identifier `HomeScreenKt\$HomeScreenDirections` because it contains illegal characters: \

il...@google.com <il...@google.com> #18Aug 28, 2024 08:28PM

Re comment#17 - please file a new issue to track that issue.

cl...@couchbits.com <cl...@couchbits.com> #19Sep 24, 2024 11:08AM

Issue from

comment#17

https://issuetracker.google.com/issues/362791955