Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support anvil-like annotations like ContributesBinding and ContributesMultibinding #212

Open
Kernald opened this issue Jul 27, 2022 · 11 comments

Comments

@Kernald
Copy link
Contributor

Kernald commented Jul 27, 2022

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
Copy link
Owner

evant commented Jul 28, 2022

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
Copy link
Owner

evant commented Jul 29, 2022

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
Copy link
Contributor

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
Copy link
Owner

evant commented Aug 5, 2022

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
Copy link
Contributor Author

Kernald commented Aug 7, 2022

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:

  • Anvil is a code generator for a code generator (Dagger). That intermediate step sometimes makes it really hard to debug when things go wrong, and is likely not as clean as t could be at runtime (I don't really know if that last concern is valid).
  • The fact that they're separate and published by different organisations seems to reduce Anvil's adoption. It's arguably very similar to Hilt in slightly less opinionated/not Android specific, and despite that and being older than Hilt, it's definitely not the more popular out of the two.

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
Copy link
Owner

evant commented Sep 23, 2022

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
Copy link

jakoss commented Mar 28, 2023

@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:

  • kotlin-inject-extensions - could contain things like mentioned ContributesMultibinding as well as ContributesBinding (for like Anvil for kotlin-inject)
  • kotlin-inject-android - could contain special annotations for viewmodels, fragments and workers, generate multibindings for those and provide factories that would work without additional work (so like Hilt/Whetstone)
  • kotlin-inject-ios - something similar to android one, but it's not really my specialty

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
Copy link
Owner

evant commented Mar 29, 2023

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
Copy link

jakoss commented Mar 29, 2023

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

@evant evant changed the title Class-level multi-bindings support anvil-like annotations like ContributesBinding and ContributesMultibinding Dec 7, 2023
@ZacSweers
Copy link

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

  • Hilt works by

    • Running a gradle task to inspect the classpath and aggregate contributed types/bindings/modules/etc into a generated source file that hilt's processor then looks for
    • Running an extra javac task after the standard compilation pipeline to actually run their processor, which is necessary because there may be new contributed things generated during that project's regular compilation. This is also why you can't reference hilt-generated sources and need a bytecode processor to link those post-compilation.
  • Anvil works by

    • Running custom AnalysisHandlerExtension implementations in the frontend to generate factories, contributed hints, etc
    • Running an IR plugin in kapt stub generation to aggregate these from the ModuleDescriptor's visible classpath

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
Copy link

vRallev commented Apr 18, 2024

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: #221 (comment)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants