adrielcafe / voyager

🛸 A pragmatic navigation library for Jetpack Compose
https://voyager.adriel.cafe
MIT License
2.27k stars 109 forks source link

Screens are not cleared when calling navigator.replaceAll #396

Open shpasha opened 4 weeks ago

shpasha commented 4 weeks ago

Suppose our rootNavigator is configured with the following disposeBehavior field:

NavigatorDisposeBehavior(
   disposeNestedNavigators = false,
   disposeSteps = true,
)

And it contains the navigation stack: A -> B -> C

Screen B also contains a nested navigator (for example, this could be a TabScreen) with the stack B1 -> B2 -> B3. In turn, within screens B1, B2, B3, there are some view models.

From screen C, we perform a rootNavigator.replaceAll action to screen D.

Current Behavior:

Expected Behavior:

It seems logical to me if ViewModels were cleared as soon as the screen they were attached to disappeared from the navigation stack. This is a critical issue for building hierarchical navigation. Are there any plans to fix this?

shpasha commented 4 weeks ago

My current workaround is:

Sample code:

private class NavigatorDisposer : ScreenDisposable {

    var navigator: Navigator? = null

    override fun onDispose(screen: Screen) {
        navigator?.let { disposeNavigator(it) }
        navigator = null
    }
}

@Composable
private fun Screen.DisposeNavigatorOnScreenDisposeEffect(navigator: Navigator) {
    val screen = this
    LaunchedEffect(navigator) {
        val disposer = ScreenLifecycleStore.get(screen) {
            NavigatorDisposer()
        }
        disposer.navigator = navigator
    }
}

@Composable
public fun Screen.Navigator(
    screen: Screen,
    disposeBehavior: NavigatorDisposeBehavior = NavigatorDisposeBehavior(),
    onBackPressed: OnBackPressed = { true },
    key: String = compositionUniqueId(),
    content: NavigatorContent = { CurrentScreen() }
) {
    Navigator(
        screen = screen,
        disposeBehavior = disposeBehavior,
        onBackPressed = onBackPressed,
        key = key,
        content = { navigator ->
            DisposeNavigatorOnScreenDisposeEffect(navigator)
            content(navigator)
        }
    )
}

But I don't think this is the best way

hristogochev commented 2 weeks ago

What would the disposeNavigator function be in your workaround? @shpasha

hristogochev commented 2 weeks ago

I believe the solution I've come up with for https://github.com/adrielcafe/voyager/issues/402 may also help this issue.

shpasha commented 2 weeks ago

What would the disposeNavigator function be in your workaround? @shpasha

@hristogochev you can find disposeNavigator function implementation in voyager source codes