InsertKoinIO / koin

Koin - a pragmatic lightweight dependency injection framework for Kotlin & Kotlin Multiplatform
https://insert-koin.io
Apache License 2.0
8.98k stars 710 forks source link

ViewModels scoping in composables is confusing (or broken?) #1787

Closed 85michal closed 6 months ago

85michal commented 7 months ago

In new versions of koin for compose it is impossible to maintain one instance of a view model between parent composable and child composable which started a new scope.

Yep, I know we are using very old version of koin compose (1.0.1) but I think that behavior difference is so important that it is worth some attention.

In the past koinViewModel function was using GlobalContext.get().scopeRegistry.rootScope by default (https://github.com/InsertKoinIO/koin/blob/compose-3.4.0/compose/koin-androidx-compose/src/main/java/org/koin/androidx/compose/ViewModelComposeExt.kt) and over time was changed to use LocalKoinScope, this change makes it impossible to maintain one instance of view mode, if some KoinScope was opened in between.

Solving that issue is probably possible on app side, but also, not that easily as scopeRegistry is an internal API, so some workaround would be needed. But even though, we are talking here about default behavior and it would be good if it is clear for the developers what to expect in that default case.

So the questions are:

  1. Can you please clarify what is intended default scoping for the view models in composables? In activity/fragment it was clear based on the extension functions for those components and, but in compose it seems to be more of a matter of some agreement that we are not sure of.

  2. Do you plan to enable as to maintain same instance of the view models between the scopes? If so, to what degree and how?

Pretty basic snippet:

`module { viewModel { SomeViewModel() } }

@Composable fun SomeComposition() { val viewModel: SomeViewModel = koinViewModel() KoinScope(scopeID = "SomeNewScopeId", qualifier("SomeNewScope")) { val viewModel: SomeViewModel = koinViewModel() // new instance of view model is returned here in new versions of koin and existing instance is returned in old versions } }`

arnaudgiuliani commented 6 months ago

it's has been reverted back. Fix is on 3.5.4 branch