adrielcafe / voyager

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

How to multiple instance viewmodel for same compose fun #7

Closed zakrodionov closed 2 years ago

zakrodionov commented 2 years ago

I have a list of items and a detail screen. Detail Screen is a compose function that takes a (post) parameter that I pass as an argument to the ViewModel. val viewModel = getStateViewModel<PostDetailViewModel>(parameters = { parametersOf(args) }) The problem is that it looks like the viewModel is cached, and all the time it opens the first post (argument) that was followed.

In jetpack navigation works correctly.

https://user-images.githubusercontent.com/27068529/129693071-ade8b0bc-1c0e-4bc0-a367-cceac22c4e53.mp4

davidbilik commented 2 years ago

I've just encountered exactly the same issue. Maybe it's a problem with Koin?

adrielcafe commented 2 years ago

Are you guys using SavedStateHandle in the ViewModel? If not, it works with by viewModel()?

davidbilik commented 2 years ago

Nope, this is my code that is called from the Content function of my Screen

@Composable
fun SessionDetailScreen(
    archiveItem: UiArchiveItem,
    subsessionItem: UiSubSession?,
    vm: SessionDetailViewModel = getViewModel(parameters = {
        parametersOf(archiveItem, subsessionItem)
    })
) {
...

and I am using Koin for retrieving VM https://insert-koin.io/docs/reference/koin-android/compose/#viewmodel-for-composable

adrielcafe commented 2 years ago

Since we are in the same activity the ViewModelStoreOwner is the same between screens (https://github.com/InsertKoinIO/koin/issues/1142). Internally it uses a Map<String, ViewModel> and Koin uses the qualifier as the key. Because of that we get the same ViewModel even if the params are different.

The solution for that is to implement ViewModelStoreOwner in our screens:

class DetailsScreen(
    val id: Int
) : Screen, ViewModelStoreOwner {

    private val viewModelStore = ViewModelStore()

    override fun getViewModelStore() = viewModelStore

    @Composable
    override fun Content() {
        CompositionLocalProvider(
            LocalViewModelStoreOwner provides this
        ) {
            val viewModel = getViewModel<DetailsViewModel> { parametersOf(id) }

        }
    }
}

I'm looking for a way to integrate that on Voyager, in the meamtime you can use the above snippet as workaround.

davidbilik commented 2 years ago

But when I used navigation-compose from Jetpack this problem was not there. I've refactored my app to Voyager because of arguments passing and then it started. Koin should have this solved IMO, it wouldn't make sense if there would be such limitation

adrielcafe commented 2 years ago

It works on navigation-compose because NavBackStackEntry implements ViewModelStoreOwner, so I have to do the same.

Working on that, shouldn't take long.

adrielcafe commented 2 years ago

Just released 1.0.0-beta06 with a new module: voyager-androidx. Just replace Screen with AndroidScreen and ViewModels should now work as expected (docs).

Closing the issue, but feel free to re-open if needed.

davidbilik commented 2 years ago

Works like a charm! Thanks :)

zakrodionov commented 2 years ago

@adrielcafe Thanks for your quick help! It worked for me only without SavedStateHandle, is there a way to use it with SavedStateHandle?

adrielcafe commented 2 years ago

@zakrodionov the ViewModel still is the same when you use the SavedStateHandle, or the SavedStateHandle content is not being saved? Can you provide a code snippet please?

zakrodionov commented 2 years ago

@adrielcafe If I use getViewModel everything is ok. But with getStateViewModel an in-memory reference to the same viewModel. It looks like the problem is in the SavedStateRegistryOwner. pic1

adrielcafe commented 2 years ago

@zakrodionov can you try with 1.0.0-beta08? Should work now.

zakrodionov commented 2 years ago

@adrielcafe Yes it worked, but #6 began to be produced, when using AndroidScreen and dont keep activities, navigation is not restored after recreating the activity(

adrielcafe commented 2 years ago

@zakrodionov I'm having difficulties to reproduce this, even with "don't keep activities" on. Can you provide a sample project or code snippet please?

zakrodionov commented 2 years ago

@adrielcafe I have it reproduced in the library sample. The Basic Navigation works correctly, but when you go to the Android Navigation, the screen resets. In the developer settings, "Dont't keep activites", "No background process" are enabled. Exception:

java.lang.RuntimeException: Parcelable encountered IOException writing serializable object (name = cafe.adriel.voyager.sample.androidNavigation.ListScreen) Caused by: java.io.NotSerializableException: cafe.adriel.voyager.androidx.ScreenLifecycleHolder

https://user-images.githubusercontent.com/27068529/131084290-167fa845-0757-4d5e-8858-8f8eafa5e2b9.mp4

It looks like we cannot save to Bundle ViewModelStore, LifecycleRegistry, SavedStateRegistryController

adrielcafe commented 2 years ago

That was a tough one to fix. Everything seems good now, can you confirm? 1.0.0-beta09

zakrodionov commented 2 years ago

@adrielcafe Yes, it works. Thank you so much!

Faltenreich commented 9 months ago

I am experiencing the same problem on multiplatform, but there is no AndroidScreen available, even when including cafe.adriel.voyager:voyager-androidx. Is there another way to push the same Screen twice, with or without changed parameters?