InsertKoinIO / koin

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

feat: Instances interface #1778

Closed monosoul closed 3 months ago

monosoul commented 9 months ago

The goal of this PR is to provide an abstraction that can be used to interact with both org.koin.core.scope.Scope and org.koin.core.Koin.

It is especially convenient when you'd like to have extension functions usable with both entities.

Tbh, not having this abstraction has been annoying me for a while, so figured I'll give it a shot :smile:

hoc081098 commented 9 months ago

Nice 🎉

arnaudgiuliani commented 9 months ago

Still need to evaluate. Can you provide tests & more details samples? 🙏

monosoul commented 8 months ago

more details samples

@arnaudgiuliani so in tests we usually don't use a global Koin context (in fact, we usually don't use it in main source set either, but that's another story :slightly_smiling_face: ), instead we build a "local" context with koinApplication to spin up some modules, and then get bean instances from that context to run tests.

What I'd like to have is some extension functions that I can use to access beans from the context both when I declare a bean, and in the tests.

E.g.:

fun Scope.getServiceTokenProvider(
    audience: String,
    config: ServiceTokenProviderConfig? = null,
) = get<ServiceTokenProvider> {
    parametersOf(audience, config)
}

fun someModule() = module {
   factory<ServiceTokenProvider> { parameters ->
       val audience = parameters.get<String>()
       val config = parameters.getOrNull<ServiceTokenProviderConfig>()
       ServiceTokenProvider(audience, config)
   }.onClose {
       it?.close()
   }
   single {
       SomeClassUsingTokenProvider(getServiceTokenProvider("someAudience"))
   }
}

class TestClass {
   val koin = koinApplication {
            modules(someModule(), ...)
   }.koin
   fun someTestFun() {
       koin.getServiceTokenProvider("someAudience") // <-- can't do that now
   }
}

So what I'd like to have is this Instances interface to be able to declare the extension function with it as a receiver instead of Scope, like that:

fun Instances.getServiceTokenProvider(
    audience: String,
    config: ServiceTokenProviderConfig? = null,
) = get<ServiceTokenProvider> {
    parametersOf(audience, config)
}

fun someModule() = module {
   factory<ServiceTokenProvider> { parameters ->
       val audience = parameters.get<String>()
       val config = parameters.getOrNull<ServiceTokenProviderConfig>()
       ServiceTokenProvider(audience, config)
   }.onClose {
       it?.close()
   }
   single {
       SomeClassUsingTokenProvider(getServiceTokenProvider("someAudience"))
   }
}

class TestClass {
   val koin = koinApplication {
            modules(someModule(), ...)
   }.koin

   fun someTestFun() {
       koin.getServiceTokenProvider("someAudience") // <-- this will work now
   }
}
monosoul commented 8 months ago

I guess it might be good to also make KoinComponent interface extend Instnaces interface, but then there will be a clash between the get and inject methods on the interface and the extension functions. It would probably be great if we can include that interface as a breaking change, then we can drop duplicate functions like that.

Although, maybe if Koin will implement Instances, it's not necessary for KoinComponent to extend it.

rmcmk commented 8 months ago

This is wonderful. Hope to see this merged.

We have a project where we have loads of abstractions and Kotlin type abuse over KoinComponent, KoinScopeComponent, Koin and Scope for some custom scoping/injecting functionality. Basically injectable qualifiers which holds both a type-safe qualifier and the type to inject. Reduces a bunch of boilerplate for us. This would make those abstractions much easier to maintain (and more easily testable for us)

rmcmk commented 8 months ago

I guess it might be good to also make KoinComponent interface extend Instnaces interface, but then there will be a clash between the get and inject methods on the interface and the extension functions. It would probably be great if we can include that interface as a breaking change, then we can drop duplicate functions like that.

Although, maybe if Koin will implement Instances, it's not necessary for KoinComponent to extend it.

You make a valid point. If Koin already implements Instances internally, there's no necessity for KoinComponent to extend it. Instead, we could simply delegate back to the Koin context within KoinComponent. Nevertheless, implementing this change would still constitute a breaking change. If any KoinComponent extensions are relied upon for overriding functionality specific to any KoinComponent, the behavior might not remain consistent.

monosoul commented 8 months ago

@arnaudgiuliani so what do you say? Does it make sense? I just want to make sure we're on the same page before I invest more time into writing tests etc.

stale[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.