adrielcafe / voyager

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

[Voyager + Koin] Screen back (pop) navigation causes screen composable to recreate view model #260

Open yamir-godil opened 10 months ago

yamir-godil commented 10 months ago

Background I'm seeing an issue with pressing back on all my voyager screens which have transitions. I have a SlideTransition for my navigator, so the composable Content function is called several times both on pushing and popping of screens.

Issue I've noticed that during the last stage of the transition, I get a new instance of the view model because Voyager clears out the lifecycle store in onDispose. This causes my view model to trigger a network load call to fetch data on a back press transition.

Here's how my code is set up:

    @Composable
    override fun Content() {
        val navigator = LocalNavigator.currentOrThrow
        val workplaceScope = KoinJavaComponent.getKoin().getScope(WORKPLACE_SCOPE)

        val viewModel = koinViewModel<AdAccountsScreenViewModel>(scope = workplaceScope, parameters = {parametersOf(preloadedAdAccounts)})
        Log.i("TestLog", "VM initialized: $viewModel")

        AdAccountsScreen(viewModel) {adAccount ->
...
        }
    }

I see this log statements when I press back from this child screen to the parent screen:

// called when the voyager screen is instantiated
11-22 12:51:58.184 13604 13604 I TestLog: AdAccountsScreen initialized

11-22 12:51:58.206 13604 13604 I TestLog: VM initialized: AdAccountsScreenViewModel@2fa44c8
11-22 12:51:58.235 13604 13604 I TestLog: VM initialized: AdAccountsScreenViewModel@2fa44c8
11-22 12:51:58.702 13604 13604 I TestLog: VM initialized: AdAccountsScreenViewModel@2fa44c8
11-22 12:51:58.717 13604 13604 I TestLog: VM initialized: AdAccountsScreenViewModel@2fa44c8

// Back press happens
11-22 12:52:03.125 13604 13604 I TestLog: VM initialized: AdAccountsScreenViewModel@2fa44c8
11-22 12:52:03.171 13604 13604 I TestLog: VM initialized: AdAccountsScreenViewModel@ed851ef

Upon investigation, I see that the local view model store owner actually has the view model reference that Koin returns but upon back press, Voyager navigator clears out the store in its dispose, which causes Koin to return a new view model instance. This causes the side effect of my view model to trigger a data load

How to handle this? I can put a short circuit by checking the navigator's lastItem and returning from the composable if the screen's don't match but that is not scalable as it would cause each screen to have this hack. It also doesn't animate properly when back is pressed with this workaround

To me this is a bug in Voyager, as it should call dispose only when transition ends.

impossible1770 commented 8 months ago

Still reproduce

DevSrSouza commented 3 months ago

I assume you use Transitions API, there is a known bug that cause this. https://github.com/adrielcafe/voyager/issues/106

In the latest 1.1.0-beta02 we have introduce a new experimental API that make the ScreenTransition handle disposing, this should be fixed by providing disposeScreenAfterTransitionEnd = true.

Checkout the docs for more details. https://voyager.adriel.cafe/transitions-api/