evant / kotlin-inject

Dependency injection lib for kotlin
Apache License 2.0
1.19k stars 52 forks source link

support anvil-like annotations like ContributesBinding and ContributesMultibinding #212

Open Kernald opened 2 years ago

Kernald commented 2 years ago

From what I understand, multi-bindings must currently be defined in a component. From the README:

@Component
abstract class MyComponent {
    abstract val fooMap: Map<String, Foo>

    @IntoMap
    @Provides
    protected fun provideFoo1(): Pair<String, Foo> = "1" to Foo("1")

    @IntoMap
    @Provides
    protected fun provideFoo2(): Pair<String, Foo> = "2" to Foo("2")
}

It would be great if instead, the @IntoMap and @IntoSet annotations could be applied to an injectable class. Anvil allows this kind of syntax:

@ContributesMultibinding(Scope::class, boundType = Foo::class)
@StringKey("my-first-foo")
class Bar @Inject constructor() : Foo {}

@ContributesMultibinding(Scope::class, boundType = Foo::class)
@StringKey("my-second-foo")
class Baz @Inject constructor() : Foo {}

And the component would as a result contain a Map<String, Foo>, itself containing {"my-first-foo": Bar(), "my-second-foo": Baz()}

This helps with composition, as adding an entry to the set/map only requires defining a type (or adding a dependency), no changes are needed to the component code itself.

evant commented 2 years ago

The currently plan is to not include any anvil-like features to keep things simple and flexible but allow something like anvil to be built on top.

Note: map keys are implemented a bit differently than dagger so you don't be able to do them with just annotations, but something like:

@ContributesMultibinding(Scope::class, boundType = Foo::class)
fun barEntry(bar: Bar): Pair<String, Foo> = "my-first-foo" to bar

may work?

evant commented 2 years ago

In fact I don't think there's anything blocking someone making an 'anvil' for kotlin-inject right now, it should already support multiple rounds properly. You would just be generating the @Component classes based on your annotations. If anyone want's to take a stab at it I'd be glad to answer any questions/fix an issues related to it.

PaulWoitaschek commented 2 years ago

I really love anvil, but I dislike that it's not really integrated with the native dagger. I would prefer it, if kotlin-inject would just discover the multibindings. The thing I like most about it, is that it lets you define the DI setup of a class in the file itself. This makes refactorings way easier. Also with the current approach it's very easy to forget setting up the dependency boilerplate correctly. (which has lead to a bug for us already).

evant commented 2 years ago

but I dislike that it's not really integrated with the native dagger

Can you elaborate on that? In my mind it would be just as easy as adding another dependency.

(which has lead to a bug for us already)

Also curious about this if you would like to share details. Would like to see if there's anything we could do to make that better.

Kernald commented 2 years ago

but I dislike that it's not really integrated with the native dagger

Can you elaborate on that? In my mind it would be just as easy as adding another dependency.

I'm not the person who wrote the previous comment, but I have a few reasons to dislike the separation myself:

While Koin is probably what most people look at at the moment when they're looking for a Kotlin DI framework, it's more of a service locator, with its pros and cons. I suspect quite a few people (including myself) keep using Dagger (and maybe Anvil or Hilt) because nothing seems to be a good alternative right now. Having one solid library rather than a scattered set of tools working together would make that transition much easier, I think - it already seems risky enough to transition from a library published and used by Google to a seemingly random library with way less users, never mind two libraries.

With that said, you're the author and maintainer, I already appreciate the work you've done and will respect your decision either way :-)

evant commented 1 year ago

Note that Hilt is implemented in much the same way as Anvil as a separate code generator, really the only difference is that it's first-party in dagger where anvil is not. I will make no promises on future additions like this but if I where to do something first-party it would probably follow the same pattern. In the meantime, I'd fully support any project built on top of this that implemented some or all of anvil's feature-set. Extensibility is a goal and I think it's a good way to allow experimentation and more complex use-cases without complicating the core library.

jakoss commented 1 year ago

@evant Are you open for contributions to this repository as extensions to kotlin-inject? This way such features would not be a separate entity, but could stay separate as another modules.

What i'm thinking is a few modules:

Keeping all that in a single repo in my opinion could somewhat solve the issue @Kernald wrote about (and i do agree with, Dagger focuses on supporting Hilt and not Anvil). Every module could be separate but all of those could be seen as a nice playing ecosystem. I would be glad to contribute to both extensions and android modules since i did similar work in my previous projects.

The issue of projects scattered around became very apparent to me when i was using Tangle which got pretty much abandoned last year. For some time it does not support newer versions of Anvil. That forced me to migrate to Whetstone which works for now, but this whole thing got me thinking about going back to hilt or koin, which just seem like a complete package out of the box. I would feel anxious to try out kotlin-inject with yet another independent library build on top of that.

evant commented 1 year ago

Hm, ok I could get behind that if you are interested in contributing. My biggest concern is there are many unanswered questions on how the api should work but if we clearly communicate that it's experimental that would be fine. My short term focus is on bug fixes and some shoreing up of existing api's (see milestones and linked issues for details) but I would be glad to provide direction and make sure we have the right extension points for these things. Note: I had started on kotlin-inject-android but it's incomplete and not a focus at the moment. I'd be glad to share my thoughts on it.

jakoss commented 1 year ago

Sure, we can take this discussion to kotlin slack for example :). For starters i would like to add ContributesMultibinding and ContributesBinding, since making that will almost fully eliminate the need to manually declare components

ZacSweers commented 8 months ago

Just wanted to chime in to say that it's not simple supporting this. Both hilt and anvil do weird things to make this kind of thing work

Anvil is not currently compatible with dagger KSP or K2. I'm working on both via KSP, but it's tedious and involved because KSP want processors to do the type of classpath scanning that anvil uses from ModuleDescriptor.

If all that can be made to run on top of KSP, it should be possible to replicate that with kotlin-inject. But it's somewhat of a big if at the moment.

vRallev commented 4 months ago

We implemented all Anvil features for kotlin-inject for our internal project. KSP provides all necessary APIs and there were no changes required for kotlin-inject itself. Generally, I'm very happy with the result and I wrote a summary here: https://github.com/evant/kotlin-inject/discussions/221#discussioncomment-9147711

I'll give a talk about this effort at Droidcon SF and I hope to open source these extension and maybe upstream them into this repo directly.

IlyaGulya commented 1 month ago

@vRallev Hi! Can you please give an update regarding open sourcing your extension? 🙂

vRallev commented 1 month ago

I gave a talk about our extensions: https://ralf-wondratschek.com/presentation/extending-kotlin-inject.html I filed the request to open source them last week and don't expect any push back. Hopefully soon™.