Dagger: Allow @Multibinds to be scoped

[cesar1000]

Multibindings in dagger 2.21 are unscoped, which causes the set/map to be recreated anytime they are requested. Applying a scope annotation to a multibinding definition doesn't seem to modify the injection behavior and yet it's not flagged as an error.

It would be ideal if scopes were allowed in multibinding definitions (in which case extension from a subcomponent should be flagged as an error). As a stopgap, flagging an error if a scope annotation is applied to a multibinding definition would help avoid unexpected behavior.

[netdpb]

Oh. Thanks for pointing out the fact that scopes are ignored for @Multibinds. We will work on making that an error.

If you want to use a mutlibinding while ignoring any contributions from subcomponents, you could scope the _consumer_ of the multibinding. That consumer wouldn't ever see contributions from subcomponents that are descendants of the scoped component.

But can you help me understand why you want to prevent subcomponents from adding contributions to a multibinding, if you don't already need to scope the consumer of the multibinding?

[Ethan1983]

FWIW, i have added to multibinding from subcomponents.

In case of Android ViewModels, i didn't want the provider (using multibinding) to live longer than the corresponding activity/fragment. If the activity/fragment is destroyed, so is the corresponding view model. If view model is destroyed, why bother having the provider stay in memory?

[netdpb]

I'm not sure whether the provider you're talking about is one of the entries/elements in the multibinding or a user of the multibinding. In either case, however, adding scope only _extends_ the lifetime of an object created by a binding. An unscoped binding's object is never held directly by the component, but a scoped one is, for the life of its component.

Generally, I advise to use scope only if the object's identity is important (for instance if it has mutable shared state), or possibly if it's expensive to produce (in which case, consider using @Reusable scope instead of a component-bound scope).

[cesar1000]

I would like to use a scoped multibinding if I want to avoid multiple instantiations of it, since recreating the set or map can be expensive - especially if the production of the values is expensive. Also, I may want to do it for correctness, if I need to guarantee that all values are produced only once. Finally, scoping the consumer would not be possible if a multibinding is consumed from multiple places (or exposed in a provision method).

Re preventing subcomponents from adding contributions: I just thought it would be needed if the multibinding is tied to a certain scope. Alternatively, you may add some sort of flag to the @Multibinding annotation to ensure that a multibinding in only instantiated once within each scope where it's consumed.

[netdpb]

Recreating the set or map isn't expensive. If recreating any individual element or value is expensive, you can and should scope that contribution by applying a scope to the @IntoSet or @IntoMap binding.

Can you give a concrete example of a case where you need all values to be created only once, where that set or map is also injected into multiple consumers?

[cesar1000]

Let me back up a bit - and sorry about the late response.

We are using multibindings, for example, for acquiring a graph builder that's associated to each activity or fragment in our Android application. The map is available from the application component, and it may contain dozens of entries. A simplified version of this looks a bit like this:

Component.Builder builder = ApplicationComponent.get().getBuilderMap().get(getClass())

ApplicationComponent.get() returns a dagger graph, and getBuilderMap() returns the multibound map. Since I had annotated my builder map with @ApplicationScope, I expected the builder map would be created upon first call and reused in every subsequent call - instead, that simple line of code is recreating a huge map just for the purpose of a single lookup.

The solution I adopted was to create an application-scoped wrapper around the multibound map and use that instead, but it feels reasonable to me that dagger would allow scoping of multibound sets and maps, like it does with every other binding.

[netdpb]

If the values of that map are intentionally mutable, then it's important that each of them be properly scoped, and the best place to do that is close to that mutable implementation (either a @Provides @IntoMap binding or the @Inject-constructed class that is the target of a @Binds @IntoMap binding). That way if a given mutable object is injectable in several ways (e.g., the map and a qualified @Binds), the proper scope is always applied.

In general, some entries in a map might be scoped and others unscoped, so it has to be possible to scope each entry. And there's value in having only one way to declare the scope of a binding.

Question for you: Do users of your map generally touch one entry each, or do most of them iterate through the entries? If it's the former, and you're worried about creating map values that maybe aren't used right now, then you could provide/inject a Map<Key, Provider<Value>>, which defers creation of unused values. Creating that map, even with dozens of entries, should be fast, since the providers are themselves fast to create. On the other hand, if all the entries are scoped, then creating the second and further instances of the regular Map<Key, Value> should be super-fast, since the values are all cached and all that has to happen is to create the map itself.

Maybe here's another question that could help me: Why are users of the map calling an entry point method on the component instead of injecting the map they need?

[cesar1000]

Yeah, it makes total sense to me that the scoping of each individual item is determined by each @Provides @IntoMap binding or the target of the @Binds @IntoMap binding, as you suggest. Also, it makes sense to me that there should be no obligation for the multibinding itself to be scoped, in order to support the use case you describe where some entries are scoped and other unscoped.

Multibound sets are usually traversed, but with maps we usually just look up a single entry. Map<Key, Provider<Value>> is certainly a possibility if we can make the assumption that creating a new map on every lookup is not a concern performance-wise (which may not be the case in mobile applications). As for your second question - we're obtaining an application component statically from an Android activity, since we can't use injection in that context. That said, injecting the map would cause a new instance of the map to be created all the same, right?

Ultimately, I don't think there's an actual _need_ to allow scoping of multibindings since, as you suggested, it's just as easy as wrapping the map into a scope object. An error rejecting a scope annotation when applied to a multibinding would prevent misuse, which is the biggest problem. It does feel inconsistent that all other bindings can be scoped though.

[ronshapiro]

This is now an error, so I'm going to rename the issue to be about allowing @Multibinds to be scoped

[doniwinata0309]

Hi any update on this if we can scope @Multibinds ? Thank you

[netdpb]

Given the conversation above, I don't think there's a strong case for allowing multibindings themselves (maps or sets) to be scoped. If you want that behavior, you can always scope the user of the multibinding.

Feel free to reopen this issue if you have further arguments in favor of it.