koin icon indicating copy to clipboard operation
koin copied to clipboard

Double instantiation of ViewModel loses arguments in SavedStateHandle

Open MaxMichel2 opened this issue 1 year ago • 2 comments

Describe the bug I am working on a KMP project where we are using Koin for the DI framework. There is no issue with Koin and the KMP side of the project, but there is one with the Android side.

When setting up a ViewModel with navigation arguments, Koin and the Compose Destinations navigation library, there seems to be a duplicate ViewModel instatiation that causes the application to crash. The crash occurs in the navigation library but is not caused by it.

Here is an extract from the logcat showing the issue:

2024-08-08 18:15:11.372 29662-29662 System.out              com.example                 I  NavArgs: MyNavArgsDataClass(test="A test string")
2024-08-08 18:15:11.408 29662-29662 [Koin]                  com.example                 E  * Instance creation error : could not create instance for '[Factory:'com.example.MyViewModelWithArguments']': java.lang.RuntimeException: 'test' argument is mandatory, but was not present!

As you can see, within less than 50 ms, I have one output indicating that I correctly retrieve the arguments from the savedStateHandle parameter of the ViewModel and another output indicating that it could not create an instance of my ViewModel (because it was missing arguments but this is a non-issue for me).

What I believe is the root cause of the problem is that somewhere in the Koin framework (whether it is linked to the use of this navigation library or not I don't know), the ViewModel is being instantiated a second time but this time has no more arguments (since they were already retrieved in the first ViewModel instance).

A workaround I found is to use single { ... } instead of viewModelOf(...) or viewModel { ... }. This alone makes me believe that the issue lies in the handling of ViewModel injection rather than the navigation library used

To Reproduce Steps to reproduce the behavior:

  1. Open/Create a new project with two screens (at least) and their view models
  2. Add a navigation argument to the destination screen via the savedStateHandle parameter
  3. Implement the Compose Destinations navigation library
  4. Add the navArgs property from this library i. The implementation should look something like this: val navArgs = savedStateHandle.navArgs<MyNavArgsDataClass>()
  5. Implement the Koin Module for the ViewModels (See example below)
fun module() = module {
    viewModelOf(::MyViewModelWithArguments)
}
  1. Add the ViewModel dependency via the navigation library (see example below)
DestinationsNavHost(
    // Other parameters omitted for clarity
    dependenciesContainerBuilder = {
        // Other dependencies
        dependency(MyScreenWithArgumentsDestination) { koinViewModel<MyViewModelWithArguments>() }
    }
)
  1. Run the app and navigate to the second screen. This should cause a crash.

Expected behavior I expect that when injecting a ViewModel with either DSL (viewModelOf or viewModel), the ViewModel then provided via the koinViewModel Composable should not be recreated (or whatever is happening)

Koin module and version: koin-bom:3.5.6 koin-core koin-android koin-androidx-compose

Snippet or Sample project to help reproduce All the necessary information to reproduce should be above.

Here are, however, the versions for the other potentially impactful libraries: androidx-compose-bom:2024.06.00 androidx-compose-compiler:1.5.14 compose-destinations:1.10.2 kotlin:1.9.24 kotlinx-serialization:1.7.1 android-gradle-plugin:8.5.0

MaxMichel2 avatar Aug 08 '24 16:08 MaxMichel2

Yeah nobody's going to look at these issues. I'm working on a manual workaround for now.

You'd be better off doing a manual solution, or switching to another iOC provider. It's been over a week, and no response at all from the authors!! @MaxMichel2

TheArchitect123 avatar Aug 20 '24 23:08 TheArchitect123

If you have any way to reproduce it (a sample project), this would clearly help me to fix this 👍

arnaudgiuliani avatar Aug 27 '24 10:08 arnaudgiuliani