Closed jakoss closed 1 year ago
Thank you. I will look at it soon.
@s0nicyouth Bumping PR here, Koin 1.1.1 released fix that was detected. Now koin injection should be working properly
@jakoss I'm still trying to take a look at the PR but I have 2 problems with it. First of all I never worked with any of the DI frameworks covered there and second it seems too big. Could we please split it in 3 with the changes corresponding to each DI framework?
@s0nicyouth This PR has only 2 DI frameworks implemented and each of those have one class implementation and one class of test:
AnvilInjector
and AnvilInjectorTest
KoinInjector
and KoinInjectorTest
Most of the changes in PR are just examples, which are in their specific directory so it can be easily navigated. I could split anvil and koin, but i don't think that will help much. To see what processor is doing you can look at the KoinInjectorTest
.
Basically, the only thing injectors have to do is to add a annotation to generated mapper class. So it's @Single
or @Factory
for koin. Koin then will detect that annotation and automatically setup the container. And that's it, no other changes in processor itself. The rest of changes are examples, abstraction for injectors to handle multiple frameworks and the testload move to another directory. I know that generates a bit of noise in the PR but i think it's necessary so we can have multiple example projects.
Changes related to processor are contained here:
@jakoss And what about Scopes other than Single or Factory(if they exist in Koin)? Is it equivalent to Dagger where one can assign scope to any @Inject annotated class in a Module or how does it work here?
@s0nicyouth For Koin Annotations there is a Scope
annotation (https://insert-koin.io/docs/reference/koin-annotations/definitions#declaring-scopes-with-scope), but for now i wanted to keep things simple and expand it over the next pull requests.
Further in future i'd like to add support for scopes and @Named
annotation (https://insert-koin.io/docs/reference/koin-annotations/definitions#qualifier-with-named) as well
@s0nicyouth For Koin Annotations there is a
Scope
annotation (https://insert-koin.io/docs/reference/koin-annotations/definitions#declaring-scopes-with-scope), but for now i wanted to keep things simple and expand it over the next pull requests.But is it possible to reassign scope afterwards for this generated module. For example in Dagger I can do:
class SomeClass @Inject constructo...
and then somewhere in other module:@Binds @SomeOtherScope fun bindSomeClass(impl: SomeClass): SomeClass
? Further in future i'd like to add support for scopes and@Named
annotation (https://insert-koin.io/docs/reference/koin-annotations/definitions#qualifier-with-named) as well So, basically I think if the answer for the question above is yes then we can merge but if it's no then it would make sense to add support for assigning Scopes before enabling ability to generate the Module(it can be merged in 2 or more PRs however).
No, Koin does not allow "rebinding", each Dependency can be declared only once. I'll merge upstream to this PR and try to figure out how to pass the scope as well
No, Koin does not allow "rebinding", each Dependency can be declared only once. I'll merge upstream to this PR and try to figure out how to pass the scope as well
ok
@s0nicyouth Implementing scope is quite a challenge, i think i will have to create new module just for koin annotations so user will be able to set scopes and named. It's doable, but the scope of changes is quite significant.
So maybe it would be better to merge this PR and make the second one much smaller. I merged the branch from upstream so it's ready to merge
@s0nicyouth Implementing scope is quite a challenge, i think i will have to create new module just for koin annotations so user will be able to set scopes and named. It's doable, but the scope of changes is quite significant.
So maybe it would be better to merge this PR and make the second one much smaller. I merged the branch from upstream so it's ready to merge
@jakoss merged
This introduces automatic generation of annotations used to setup DI framework. For now we have 2 frameworks supported:
@Factory
or@Single
(by additional ksp optionarg("koinInjectionType", "single")
@ContributeBinding
and@Inject
(with@Singleton
support by additional ksp optionarg("anvilGenerateAsSingletons", "true")
). The scope of the binding have to be passed by ksp option likearg("anvilBindingScope", "<your.package>.AppScope")
.By default no annotations are generated. To enable this user have to pass ksp option
arg("injector", "<framework>")
where framework for now can be eitherkoin
oranvil
. I'll add all this information to the docs once it will be set up.While koin integration generates proper annotations, the koin annotation might not pick up those annotations since it does not handle multiple round processing properly. This PR should fix that (no further changes on the kmapper should be necessary).
Additionaly this PR introduces unit tests for generated code by using
Kotlin Compiler Testing
library (https://github.com/tschuchortdev/kotlin-compile-testing), which is very useful to catch any code generation regressions. I think we can test the basic mapping functionality as well.Fixes #4
PS. Hilt integration will be another PR, since this library is Android-specific and it requires additional effort (i need to generate hilt module file to create implementation-interface binding)