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

How to multiple instance viewmodel for same compose fun #7

Closed
zakrodionov opened this issue Aug 18, 2021 · 18 comments
Closed

How to multiple instance viewmodel for same compose fun #7

zakrodionov opened this issue Aug 18, 2021 · 18 comments

Comments

@zakrodionov
Copy link

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.

Video.2021-08-17.11-34-03-1.mp4
@davidbilik
Copy link

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

@adrielcafe
Copy link
Owner

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

@davidbilik
Copy link

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

Since we are in the same activity the ViewModelStoreOwner is the same between screens (InsertKoinIO/koin#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
Copy link

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

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

Working on that, shouldn't take long.

@adrielcafe
Copy link
Owner

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

Works like a charm! Thanks :)

@zakrodionov
Copy link
Author

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

@adrielcafe adrielcafe reopened this Aug 25, 2021
@adrielcafe
Copy link
Owner

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

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

adrielcafe commented Aug 25, 2021

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

@zakrodionov
Copy link
Author

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

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

zakrodionov commented Aug 27, 2021

@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

Video.2021-08-27.09-43-49-1.mp4

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

@adrielcafe
Copy link
Owner

adrielcafe commented Aug 27, 2021

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

@zakrodionov
Copy link
Author

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

@Faltenreich
Copy link

Faltenreich commented Aug 13, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants