Multiple instances of android ViewModel

[warcello]

I've noticed a bug that is very similar to the #145

If ViewModel is created as a constructor parameter of another ViewModel then it can be created multiple times.

To Reproduce

1. Create a module with two ViewModels (for example FirstViewModel and SecondViewModel ) where SecondViewModel has FirstViewModel as a constructor parameter.
   Example:

val koinModule: Module = module {
    viewModel { FirstViewModel() }
    viewModel { SecondViewModel(*get()*) } //<---- this parameter is a FirstViewModel instance (maybe here should be sth like `getViewModel()` instead of `get()` ?)
}

2. Create some class and inject both viewModels

class SomeClass {
    val firstVM: FirstViewModel by viewModel()
    val secondVM: SecondViewModel by viewModel()
}

3. In effect SomeClass has different instance of FirstViewModel than SecondViewModel

Expected behavior
Only one instance of FirstViewModel should be created.

Koin project used and used version (please complete the following information):
koin-core version 2.0.1
koin-androidx-viewmodel version 2.0.1

[cetonek]

My theory is that this is expected behavior, because for viewModel to be injected and hooked to correct ViewModelStoreOwner it needs to be injected from that context -> ie by viewModel {} or by sharedViewModel {} in activity or fragment. This is how koin resolves a viewModel:

/**
 * resolve instance
 * @param parameters
 */
fun <T : ViewModel> Koin.getViewModel(parameters: ViewModelParameters<T>): T {
    val vmStore: ViewModelStore = parameters.owner.getViewModelStore(parameters)
    val viewModelProvider = rootScope.createViewModelProvider(vmStore, parameters)
    return viewModelProvider.getInstance(parameters)
}

Notice parameters:

class ViewModelParameters<T : Any>(
    val clazz: KClass<T>,
    val owner: LifecycleOwner,
    val qualifier: Qualifier? = null,
    val from: ViewModelStoreOwnerDefinition? = null,
    val parameters: ParametersDefinition? = null
)

typealias ViewModelStoreOwnerDefinition = () -> ViewModelStoreOwner

This way koin is provided LifecycleOwner and ViewModelStoreOwner (optionally) from which it will correctly resolve wheter to create new viewModel or return existing instance for given activity/fragment. I think for your constructor injection Koin just handles it like a regular object and simply creates new instance.

But your use case seems weird to me, If I want to communicate between two viewModels I do so through some repository or some kind of shared data holder that has scope "longer" than given viewModels - I avoid communicating direcly between them

[rexih]

as explained by @Ejstn

i think maybe you can try:

val koinModule: Module = module {
    viewModel { FirstViewModel() }
    viewModel {  (model: FirstViewModel) -> SecondViewModel(model) }

class SomeClass {
    val firstVM: FirstViewModel by viewModel()
    val secondVM: SecondViewModel by viewModel{ parametersOf(firstVM)}
}

[mcungl]

viewModel { FirstViewModel() } is creating a factory bean definition, which means you always get a new instance.

Maybe you could use the single bean definition combined with scopes tying it to the activity.

[arnaudgiuliani]

ViewModel are more intended to be tied to the Activity/Fragment lifecycle stuff ... else extract component from them. Don't try to inject VM between them, not sure this is a good use case 🤔