voyager icon indicating copy to clipboard operation
voyager copied to clipboard

Screens are not cleared when calling navigator.replaceAll

Open shpasha opened this issue 1 year ago • 4 comments

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:

  • View models on screens B1, B2, B3 are not cleared (because StepDisposableEffect does not perform recursive clearing of screens).

Expected Behavior:

  • View models on screens B1, B2, B3 are cleared.

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 avatar Apr 20 '24 21:04 shpasha

My current workaround is:

  • Pass navigator to NavigatorDisposer class, which inherits ScreenDisposable
  • When Screen is disposed NavigatorDisposer also disposes navigator

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

shpasha avatar Apr 20 '24 21:04 shpasha

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

hristogochev avatar Apr 28 '24 21:04 hristogochev

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

hristogochev avatar Apr 29 '24 01:04 hristogochev

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

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

shpasha avatar May 01 '24 13:05 shpasha