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

Getting error "Used to extend current API with Koin API" 2.2.0-rc-3 #934

Closed ildar2 closed 4 years ago

ildar2 commented 4 years ago

I've tried to update koin to 2.2.0-rc-3 and got a lot of indescriptive errors: "Used to extend current API with Koin API" for all classes with their parent implementing KoinComponent interface. Apparently I need to add @KoinApiExtension to all those classes Is that really necessary or am I doing something wrong?

The error doesn't provide much detail and I'd prefer not to modify about 100 classes just to add @KoinApiExtension and fix imports, so what are the options?

jeziellago commented 4 years ago

Hi @ildar2 From now, that is really necessary. @arnaudgiuliani explains that You shouldn’t use KoinComponent apart of really using it, not being part of your application design. KoinComponent are used where you can't have access to Koin & you need to extend Koin for SL API In this post we can see that:

Among the raised issues in Koin 2.0 & 2.1, was the inconsistency ofstartKoin initialization. This is now fixed, bringing simpler Koin Context building. Please note that now,startKoin is using GlobalContext by default. KoinContextHandler has been deprecated. One important requirement was to lock most internal APIs of the project. Check your code if you were using internal Koin API.

The other thing I’ve seen around is the misuse of KoinComponent interface to extend service locator API. Be aware that as a Koin framework user, you shouldn’t need to use this interface. KoinComponentinterface must be used to extend Koin API or “bootstrap” component where we don’t have Koin APIs for that. In your app, use constructor style injection as much as possible. For such sensible API functions, we introduce the @KoinApiExtension annotation to enforce users to opt-in such API.

harry248 commented 4 years ago

Personally I don't like that change - after all it has been an easy solution to inject (or look up) dependencies in classes whose instance creation you don't control. And the documentation (https://doc.insert-koin.io/#/koin-core/koin-component) didn't give the impression that "you shouldn’t need to use this interface" until now.

jeziellago commented 4 years ago

Personally I don't like that change - after all it has been an easy solution to inject (or look up) dependencies in classes whose instance creation you don't control. And the documentation (doc.insert-koin.io/#/koin-core/koin-component) didn't give the impression that "you shouldn’t need to use this interface" until now.

I agree with you. Check my question here about that https://kotlinlang.slack.com/archives/CRF31N2HW/p1602782429021400

ildar2 commented 4 years ago

In the core of my app structure there is a CoroutineProvider class:

/**
 * Used in [BaseViewModel] to make coroutine scope
 * should be mocked in tests
 * @see DemandCreatorPresenterTest
 */
class CoroutineProvider : KoinComponent {
    val Main: CoroutineContext by inject(named("main"))
    val IO: CoroutineContext by inject(named("io"))
}

It provides a convenient way to inject CoroutineContext to viewModels at runtime and in tests It is used in BaseViewModel:

abstract class BaseViewModel(
    protected val scopeProvider: CoroutineProvider = CoroutineProvider(),//shows error if I add @KoinApiExtension to CoroutineProvider. also error to all BaseViewModel's children
    protected var coroutineJob: Job = SupervisorJob(),
    protected val scope: CoroutineScope = CoroutineScope(coroutineJob + scopeProvider.IO),
    protected val uiCaller: UiCaller = UiCallerImpl(scope, scopeProvider)
) : ViewModel(), UiCaller by uiCaller {

With the help of delegation I can fracture complex viewModels using uiCaller. Each piece can have VM's scope and utilities provided by uiCaller.

I'm maintaining about 6 projects each having 50-100 viewModels and classes using uiCaller. Not including tests, where we inject mock CoroutineContext, so all work is done in test thread.

Now in order to update, I need to modify all those classes to continue injecting scope to my viewModels. I chose Koin for its convenience, but now it brings me extra work for no reason. Is there an easy way to update without having to modify too much of a code? I might agree adding KoinApiExtension to CoroutineProvider and maybe to BaseViewModel, but why do I have to annotate each and every viewModel?

arnaudgiuliani commented 4 years ago

I agree enforcing you to OptIn this kind of API is a bit a lot ... on the other side, we need a way to tell you that you are extending Koin capacity not making proper DI configuration.

arnaudgiuliani commented 4 years ago

I will pass the KoinApiExtension to warning, not error 👍

already on master for next release: https://github.com/InsertKoinIO/koin/commit/faec57b010d5bfe0f0e817464eb43329039ab9f5

Carlos-Ot commented 3 years ago

@arnaudgiuliani Now my question about this change is for those who use Koin to make Libraries, where we want to isolate the Library Context from the App/Project Context. We still are able to use the KoinComponent, but is this the right way of doing it? [EDIT] In my case I'm using like described in Koin - Context Isolation. And now I'm getting this Warning in the project that uses my library. Is there any alternative to make this warning stops in the library "boundaries"

laim2003 commented 3 years ago

I'm a bit confused. Does this mean I should definetely not use KoinComponent until it's 100% required? I used it as an easy way to get application context in any required class without having to pass the Context down to the consuming class from the top most layer. And also the docs sound like it's totally ok and a valid design to extend any clas by KoinComponent if required: "If your are using an API and want to use Koin inside it, just tag the desired class with KoinComponent interface." So... am I supposed to use it or not?

EDIT

I may also recite the Dagger to Koin migration guide which clearly says NOT to use constructor injection:

Tag Thermosiphon & CoffeeMaker classes with KoinComponent interface to lazy inject Heater into a property, instead of getting it from its constructor

class CoffeeMaker @Inject constructor() : KoinComponent {

    // Lazy inject from Koin
    val pump: Pump by inject()
    val heater: Heater by inject()

    fun brew() {
        heater.on()
        pump.pump()
        println(" [_]P coffee! [_]P ")
        heater.off()
    }
}
class CoffeeApp : KoinComponent {
    val maker: CoffeeMaker by inject()
}

Source article on medium Source code

max-grossmann commented 3 years ago

Until there is a better solution: You can disable/suppress the warning in build.grade

    android {
        ...
        kotlinOptions {
            freeCompilerArgs += "-Xopt-in=org.koin.core.component.KoinApiExtension"
        }
       ...
    }
jemshit commented 3 years ago

@arnaudgiuliani I do not agree with "In your app, use constructor style injection as much as possible".

Why would i want to create all the objects of parameters at the time i create the class? There are tons of cases where you do not use class parameters for each function call. X function may use a,b parameters and Y function may use c,d parameter objects. If user only needs X function call at this session, i do not want to initialize c,d parameters, in other words, i want to defer initialization. (Just like by viewModel in Activity)

Without using KoinComponent interface, how to lazily initialize dependencies inside the class?

deinlandel commented 3 years ago

Requiring this annotation for KoinComponents is confusing. I create some objects in runtime, dynamically, and they are not activities, etc., so how should I inject them with Koin without using KoinComponent? How do you even do contructor injection outside of module definition? I haven't found any proper examples.