Exception when using nested navigation graphs [369616172]

Fixed

Bug

Status Update

No update yet.

Description

ko...@jetbrains.com

created issue #1

Sep 26, 2024 09:06AM

Component used: Navigation
Version used: 2.8.1

@Composable
internal fun App() = AppTheme {
    Column(
        modifier = Modifier.fillMaxSize(),
        verticalArrangement = Arrangement.Center
    ) {
        val navController = rememberNavController()
        val innerNavController = rememberNavController()
        NavHost(navController, startDestination = "first") {
            composable("first") {
                //val innerNavController = rememberNavController()
                NavHost(innerNavController, "nested1") {
                    composable("nested1") { Text("nested1") }
                    composable("nested2") { Text("nested2") }
                }
            }
            composable("second") { Text("second") }
        }
        Button(onClick = {
            navController.navigate("second") {
                popUpTo("first") {
                    inclusive = true
                    saveState = true
                }
            }
        }) {
            Text("second")
        }
        Button(onClick = {
            navController.navigate("first") {
                restoreState = true
                popUpTo("second") { inclusive = true }
            }
        }) {
            Text("first")
        }
    }
}

Steps:

navigate to Second
navigate to First there will be an exception:

You cannot access the NavBackStackEntry's ViewModels after the NavBackStackEntry is destroyed.

I move the innerNavController inside composable("first") it fixes the problem.

The problem easy can be reproduced in androidx-main navigation tests:

open androidx.navigation.compose.NavHostTest#nestedNavHostRestore
add composeTestRule.waitForIdle() to the end of the test
run it on a device

Comments

il...@google.com <il...@google.com> Sep 26, 2024 10:44PM

Assigned to jb...@google.com.

ap...@google.com <ap...@google.com> #2Oct 1, 2024 05:56AM

Project: platform/frameworks/support
Branch: androidx-main
Author: Jeremy Woods <jbwoods@google.com>
Link: https://android-review.googlesource.com/3288177

Throw an error sooner for using destroyed NavController

Expand for full commit details

Throw an error sooner for using destroyed NavController

Moving the LifecycleOwner of the NavController is a terminal state and
that NavController should not continue to be used.

This change ensure that calling setGraph on a destroyed NavController
will immediately throw an error.

NavController's that never had a lifecycle will work as expected.

RelNote: "Attempting to use a `NavController` that has been previously
`DESTROYED` will now cause an `IllegalStateException`."
Test: fixed nestedNavHostRestore test
Bug: 369616172

Change-Id: I520da520adf9f99d887e63c0255afe97ecefdab5

Files:

M navigation/navigation-compose/src/androidTest/java/androidx/navigation/compose/NavHostTest.kt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt

Hash: 318f8b1964af1483459ee2dc3a9a7f0e3b1e11da
Date: Mon Sep 30 20:15:26 2024

jb...@google.com <jb...@google.com> #3Oct 1, 2024 08:22AM

After some investigation here, we decided that it was best to improve the error message around this situation instead of supporting this particular use case.

While the goal of moving the innerNavController away from the inner NavHost is to hoist the state, the Lifecycle of that innerNavController is still set to the LocalLifecycleOwner, which in this case is the first NavBackStackEntry. When you navigate and pop first off of the back stack, you move the state of the innerNavController down to DESTROYED which is a "terminal" state and there is no good way for us to recover from that.

The state has all been DESTROYED so we can't reuse any of the same entries that were previously on the back stack of the innerNavController. Maybe we could make new instances of all of the DESTROYED entries and put them back on the back stack, but then you didn't gain anything from the hoisting cause we would still lose all of it. We could forget about the state, clear the back stack and start over from scratch, but is that would users would expect when they return back to the nested Nav if that were deep in the stack?

For these reasons, we decided to go with the more defined behavior of throwing an error and just letting users know that their NavController is out living the NavHost.

If developers are okay with the last option of clearing the entire stack if the controller is destroyed, doing that manually is still supported and will not throw the new error.

jb...@google.com <jb...@google.com> #4Oct 1, 2024 08:22AM

Marked as fixed.

This has been fixed internally and will be available in the Navigation 2.8.3 release.

na...@google.com <na...@google.com> #5Oct 16, 2024 05:48PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.navigation:navigation-compose:2.9.0-alpha01

jb...@google.com <jb...@google.com> #6Oct 22, 2024 01:17PM

This missed the 2.8.3 release and this will actually be part of navigation 2.8.4.

na...@google.com <na...@google.com> #7Nov 13, 2024 06:19PM