Constructor DSL prefers injecting over passing parameters and does not iterate over them.

[Nek-12]

Describe the bug First, this is a global problem with how constructor DSL parameter resolution works Constructor DSL tries to inject all parameters that are of primitive types, skipping everything else and giving priority to injection over parameter passing

This leads to the following issues:

1. https://github.com/InsertKoinIO/koin/issues/1422 <- This person tried to both inject and pass a single type as a parameter. As a result, constructor DSL passes his injected dependency to the requesting class, instead of giving priority to the dependency specified in the parametersOf() block. Here, it's obvious that parameters must take priority over injected classes. This would allow people to override dependencies, or it would at least behave as intended.
2. https://github.com/InsertKoinIO/koin/issues/1352 <- Here koin ignores null values as parameters, because instead of iterating through parameters, it just tries to randomly find a value of a specified type. It's obvious this person wanted the first parameter of his parametersOf() block to be passed to the first place where it would have been required in his class at the moment of creation.
3. https://github.com/InsertKoinIO/koin/issues/1372 <- Here this is another error caused by storing parameters incorrectly (actually discovered by me long ago, but not addressed at all). Multiple people tried to pass parameters of the same type to a class, and again, instead of iterating through them, Koin just discards every value of the same type and passes the first parameter everywhere. This leads to no crashes, no warnings, and a random, unpredictable broken behavior in the app.
4. Here's another bug I discovered: Again, because the parameters are not given priority over injected dependencies and not iterated through, when I tried to inject (nullable) java.util.UUID as a parameter to one of my dependencies, I now get crashes because Koin tries to resolve an injectable dependency of that class instead of just simply looking at parameters and finding it there.

Expected behavior

1. Koin should accept null and any class as a parameter
2. Parameters should be given priority over injected dependencies when creating the dependency.
3. Parameters should be iterated through, resolving them by type additionally, instead of just trying to get the first one that matches given type. Example:

class Injected
class Param

class Dependency (
  val p1: Param,
  val p2: Injected
  val p3: Param?
)

Then:

single<Injected>()
parametersOf(Param(), null)

Should lead to -> Dependency( params.get(0), get<Injected>(), params.get(1) ) // Param, Injected, null

single<Param>()
single<Injected>()
parametersOf(null)

should lead to -> Dependency(get<Param>(), get<Injected>(), params.get(0))

Koin project used and used version (please complete the following information): Any version since constructorDSL and up to 3.2.2

[igor-ye]

Same problem with injecting nullable parameter. Koin definitely should allow injection of nullable instances.

[arnaudgiuliani]

fixed in 3.4.x

[Nek-12]

Can this be reopened please? I'm still facing the same issue on 3.4.2.

My definition:

viewModel { (id: Uuid?) ->
    MyViewModel(param = id, injected = get())
}

When I changed it to

viewModelOf(::MyViewModel)

Still fails if the parameter that I provided is null. This does not address the issue fully. Please refer to the original post description for test cases.

[Siwy160]

I've noticed that it was crashing for me when I had added both koin-android and koin-androidx-compose dependencies. Just koin-androidx-compose turned out to be all I need.

[Nek-12]

@arnaudgiuliani Can this be reopened please? The issue is not solved

[arnaudgiuliani]

@Nek-12 can you bring more details?

[Nek-12]

Please see my comment https://github.com/InsertKoinIO/koin/issues/1456#issuecomment-1586226745

I'd like for the parameter resolution to try and pass null for parameters that

1. Have a nullable type
2. Have been provided a null value

I'd like to NOT try to resolve an injected dependency if a parameter has been provided with a null value so as to not get unpredictable injection configuration.

Right now having null as a parameter value is not supported at all since it will attempt to resolve a dependency and throw regardless of whether the parameter has been passed.

[arnaudgiuliani]

Yes nullable parameters is not taken into account for constructor DSL. This might be place to look to improve.

[arnaudgiuliani]

This could go for Koin 4.1 as we could provide a better DSL, better that just filling everything with get. Let's follow-up

[Nek-12]

In my humble opinion only the behavior should be corrected. No new API is necessary. Please focus on addressing the issue, a new dsl is not what is asked for in this request, but rather resolving the need for many manual calls and other crashes caused by this.

[arnaudgiuliani]

Let me check I can go with it for 4.0 else, I'll reschedule for 4.1

[arnaudgiuliani]

With current singleOf DSL style, it's really close to current types. Then it's either T or T? ... we can't mix both or we explode combination of arguments.

I'm starting a proposal to rework this DSL proposal. Currently it's JVM only reflection based, but we target KFunction type directly. I will look at kotlin compiler at this level. Experiment call koin-fu: https://github.com/InsertKoinIO/koin/pull/1973/files#diff-161f00b213207f6a3909898981c55d93ab998445b8aafbc5234556d24ea9b16bR21

In pure DSL approach with detecting metadata, we need to do things under the hood.

If anyone wants to investigate on this part, to replace the build part with kotlin compiler, here we can go 👍