Dagger: Support for set multibindings with component provision methods of component dependencies

Created on 18 May 2018  路  5Comments  路  Source: google/dagger

Dagger documentation says that:

The following are available as dependencies and may be used to generate a well-formed component:

  • The component provision methods of the component dependencies

And also that:

A binding in a subcomponent can depend on a multibound set or map from its parent, just as it can depend on any other binding from its parent. But a subcomponent can add elements to multibound sets or maps that are bound in its parent as well, by simply including the appropriate @Provides methods in its modules.

Would it be possible to add support for multibindings from component dependencies?

I suppose there is no support for that right now, I tried:

@Component(modules = [ModuleA::class])
interface ParentA {
    @ElementsIntoSet
    fun strings(): Set<String>
}

@Module
class ModuleA {
    @Provides
    @IntoSet
    fun string() = "A"
}

@Component(modules = [ModuleB::class])
interface ParentB {
    @ElementsIntoSet
    fun strings(): Set<String>
}

@Module
class ModuleB {
    @Provides
    @IntoSet
    fun string() = "B"
}

@Component(dependencies = [ParentA::class, ParentB::class])
interface Child {
    val strings: Set<String>
}

But I get this error:

e: com/example/dagger/ParentB.java:10: error: Multibinding annotations may only be on @Provides, @Produces, or @Binds methods
    @dagger.multibindings.ElementsIntoSet()
    ^
e: com/example/dagger/ParentA.java:10: error: Multibinding annotations may only be on @Provides, @Produces, or @Binds methods
    @dagger.multibindings.ElementsIntoSet()
    ^
e: com/example/dagger/Child.java:10: error: [Dagger/DuplicateBindings] java.util.Set<java.lang.String> is bound multiple times:
    public abstract java.util.Set<java.lang.String> getStrings();
                                                    ^
      @org.jetbrains.annotations.NotNull @dagger.multibindings.ElementsIntoSet Set<String> com.example.dagger.ParentA.strings()
      @org.jetbrains.annotations.NotNull @dagger.multibindings.ElementsIntoSet Set<String> com.example.dagger.ParentB.strings()

      java.util.Set<java.lang.String> is provided at
          com.example.dagger.Child.getStrings()

My use case is that I have an interface like AccountRepository and it's implemented differently in two components. I need a component that uses both AccountRepository implementations to get all accounts from both of them.

I was trying to achieve something similar by using modules directly instead, but there were some scoping issues I wasn't able to resolve.

Update

After some more experimenting, this idea might not be that useful after all. If ParentA and ParentB have other provision methods with overlapping types, I wouldn't be able to set them both as dependencies anyway... But maybe in some other scenario this would be useful.

Most helpful comment

@ronshapiro Can you please clarify how #1112 would solve this? Would I have to write e.g.

val childComponent = DaggerChildComponent.create()
val rootComponent = DaggerRootComponent.builder()
   .includeElementsFromSet(childComponent.someSet)
   .build

where includeElementsFromSet would have @ElementsIntoSet annotation, is that correct?

I'm curious because we still need to add the child component. Plus if we want to provide more than one multibinding we'll have something like:

val childComponent = DaggerChildComponent.create()
val secondChildComponent = DaggerChildComponent.create()
val rootComponent = DaggerRootComponent.builder()
   .plusChild(childComponent)
   .plusSecondChild(secondChildComponent)
   .includeElementsFromSet(childComponent.someSet)
   .includeElementFromAnotherSet(childComponent.anotherSet)
   .includeElementFromSecondChild(secondChildComponent.alsoSet)
   .build

So for each component we'll have to both add it to the root component _and_ specify each multibinding provision separately. Seems like it would be pretty annoying, since we now need to capture the child component in a variable and be sure to explicitly add each multibinding provision (even though it's already defined in the component). Any thoughts on this?

All 5 comments

+1 for the feature request. I do think it's a useful idea regardless of the overlapping types.

I just ran into this too - you can accomplish something with a module's subcomponents, but then you loose the graph isolation, the ability to create the component on its own, and there's a bunch of extra boiler plate to construct the component in the module and re-bind its values into a new set.

For using dagger 2 on the server this feature would be valuable to aggregating isolated working packages into bigger containers. Each component may need to tie into framework capabilities, i.e. provide a HealthCheck or Job. I don't think I would worry about overlapping types because there is a contract designed to allow for multiple installation.

Here's an example of how I would use it.

/**
 * Implement this interface to install framework features.
 * Install ServiceAspectEmptyModule to provide null bindings.
 */
interface ServiceAspect {
    @ElementsIntSet Set<HealthCheck> getHealthChecks();
    @ElementsIntSet Set<ScheduledJob> getJobs();
    @ElementsIntSet Set<BindableService> getGrpcServices();
}
...

// all of these aspect also extend ServiceAspect
@Component(dependencies = {AuthorizationAspect.class, HelloWorldAspect.class, CoffeeMakerAspect.class})
interface Application extends ServiceAspect {
}

I'd love to see that feature as well! Working with subcomponents here just doesn't cut it if the specific components should work standalone as well.

I think #1112 (allowing mutlibinding annotations on component builder methods) is a better approach to this.

Sticking multibinding annotations on a dependency request site (the component method) seems confusing. I'm afraid it's attempting to overload concepts that are best to be kept separate.

@ronshapiro Can you please clarify how #1112 would solve this? Would I have to write e.g.

val childComponent = DaggerChildComponent.create()
val rootComponent = DaggerRootComponent.builder()
   .includeElementsFromSet(childComponent.someSet)
   .build

where includeElementsFromSet would have @ElementsIntoSet annotation, is that correct?

I'm curious because we still need to add the child component. Plus if we want to provide more than one multibinding we'll have something like:

val childComponent = DaggerChildComponent.create()
val secondChildComponent = DaggerChildComponent.create()
val rootComponent = DaggerRootComponent.builder()
   .plusChild(childComponent)
   .plusSecondChild(secondChildComponent)
   .includeElementsFromSet(childComponent.someSet)
   .includeElementFromAnotherSet(childComponent.anotherSet)
   .includeElementFromSecondChild(secondChildComponent.alsoSet)
   .build

So for each component we'll have to both add it to the root component _and_ specify each multibinding provision separately. Seems like it would be pretty annoying, since we now need to capture the child component in a variable and be sure to explicitly add each multibinding provision (even though it's already defined in the component). Any thoughts on this?

I'm interested in seeing this as well - #1112 doesn't solve this because even in @lwasyl's example, if there are two child components that both provide the same set, the parent can't be constructed due to conflicting bindings of the same set type.

@Component
interface ChildA {
    val strings: Set<String>
}

@Component
interface ChildB {
    val strings: Set<String>
}
val childA = DaggerChildA.create()
val childB = DaggerChildB.create()
val rootComponent = DaggerRootComponent.builder()
    .childA(childA)
    .childB(childB) // ERROR: Multiple bindings for Set<String>
    .includeElements(childA.strings)
    .includeElements(childB.strings)
    .build()
Was this page helpful?
0 / 5 - 0 ratings

Related issues

makaroffandrey picture makaroffandrey  路  3Comments

SAGARSURI picture SAGARSURI  路  3Comments

rciovati picture rciovati  路  3Comments

blackberry2016 picture blackberry2016  路  3Comments

SteinerOk picture SteinerOk  路  3Comments