Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
yamir-godil opened this issue Nov 23, 2023 · 2 comments
Labels
di: koin "Koin" dependency injection issue transitions viewmodel

Comments

@yamir-godil
Copy link

yamir-godil commented Nov 23, 2023

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
Copy link

Still reproduce

@DevSrSouza DevSrSouza added di: koin "Koin" dependency injection issue lifecycle viewmodel and removed lifecycle labels Apr 8, 2024
@DevSrSouza
Copy link
Collaborator

I assume you use Transitions API, there is a known bug that cause this. #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/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
di: koin "Koin" dependency injection issue transitions viewmodel
Projects
None yet
Development

No branches or pull requests

3 participants