google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.41k stars 2.01k forks source link

Feature Request: Dynamic scope for @Binds/@Provides bindings #2014

Open trevjonez opened 4 years ago

trevjonez commented 4 years ago

Similar to @Reusable dynamically finding a correct scope to live in, it would be nice for writing bindings that are able to respond dynamically based on where a binding is included in a component hierarchy.

Given a module:

@Module
interface SomeFeatureModule {

  @Binds
  @AutoScope // some new advanced semantics style scope
  fun SomeConcreteType.asAbstractType(): SomeType
}

If included on a component the dynamically scoped binding would be scoped to the component that includes them:

@Singleton
@Component(modules = [SomeFeatureModule::class])
interface HierarchyRoot {

  val foo: SomeType //Singleton scoped instance
}

To some effect this would make a module behave almost like a template and could provide a nice composition pattern and the ability for a library to provide a default configuration module that doesn't assume anything about the overall shape of a component hierarchy. It only states these bindings should be scoped at the same level for correct usage.

Though I think it should only be applicable to @Binds and @Provides functions within a module. And is some regard this is similar to how @BindsInstance behaves.

Naming ideas for this special scope type: @AutoScope, @InheritScope, @DynamicScope or drop the Scope suffix. @Dynamic...

bcorso commented 4 years ago

Usually, I find that a feature library usually expects all of its scoped bindings to be installed in the same component. With @AutoScope, the user could accidentally install two modules into two separate components. Dagger would compile, but the app would behave strangely.

An (existing) alternative is for the library to define its own scope that the user adds to the component.

@SomeFeatureScope
@Singleton
@Component(modules = [SomeFeatureModule::class])
interface HierarchyRoot {
  val foo: SomeType //Singleton scoped instance
}
trevjonez commented 4 years ago

That seems to me to be complexity and risk produced by having multiple component hierarchies not necessarily unique to this concept.

if it were to happen in the same hierarchy I would expect the normal duplicate binding exceptions to kick in at compile time

trevjonez commented 4 years ago

Also the concrete inclusion of a type @SomeFeatureScope is exactly what I want to avoid with this type of feature. In a high module count project having good graph composition can get difficult when you are trying to minimize the depth of inter project dependency.

bcorso commented 4 years ago

Say you have two modules:

@Module
interface SomeFeatureFooModule {
  @SomeFeatureScoped
  @Provides
  static Foo providerScopedFoo() { ... }
}

@Module
interface SomeFeatureOptionalBarModule {
  @SomeFeatureScoped
  @Provides
  static Bar providerScopedBar() { ... }
}

In your feature library, you can assume Foo and Bar are scoped to the same component because there's no way to do otherwise in Dagger.

However, if you swap those with @AutoScoped then users can install those modules into two separate components in the same hierarchy. There is no duplicate binding error because Foo and Bar are separate bindings.

@Component(modules = SomeFeatureFooModule.class)
interface MyComponent {
  MySubcomponent mySubcomponent();
}

@Subcomponent(modules = SomeFeatureOptionalBarModule.class)
interface MySubcomponent {}
trevjonez commented 4 years ago

Ok I see what you are saying.

I suppose that ends up landing on the library consumer to know the integration caveats of a library if dynamic scope elements were introduced. Or if such a constraint really needs to exist between Foo & Bar then the library should do just as you suggest with a new concrete scope rather than a dynamic one. You as the author would be able to decide how much risk of integration error is acceptable in that case. And even if the consumer didn't want to use the concrete scope they could always reimplement the module with dynamics so long as type visibility didn't prohibit it.

I am not convinced that these two concepts should be mutually exclusive.

bcorso commented 4 years ago

Also the concrete inclusion of a type @SomeFeatureScope is exactly what I want to avoid with this type of feature.

Do you have more details about the pain point of adding the scope annotation to the component causes? Since you have to add the module to the component anyway, it seems adding the scope wouldn't be too painful. Or is the problem more about users knowing when they need to add it? If it's the latter, would having better error messages help?

trevjonez commented 4 years ago

The existing errors are fine since it will complain about using scoped bindings at a component that lacks the scope.

Let me try and formulate a cleaner picture of the project I was working in when I concocted this idea.

If we have ~20 different projects that all roughly consist of: kotlin jvm gradle project, dozen or so data classes and retrofit interfaces for json + custom moshi adapters if needed. A module to contribute any custom moshi configuration via set bindings and to @Provides all the retrofit.create calls.

And let's say ~5 higher level domain features that consume those different api projects.

Integrating some of those apis into a feature component, results in something like this:

@FooApiScope
@BarApiScope
@BazApiScope
...
@IntegratingFeatureScope
@Subcomponent(modules = [
  FooModule::class,
  BarModule::class,
  BazModule::class,
...
  FeatureModule::class
])
interface IntegratingFeatureComponent {
....
}

Where ideally we would communicate in terms of only "Integrating Feature" but the act of actually integrating all the pieces ends up fairly clunky with a growing number of scope labels on the component.

So functionally the existing scope naming system can be made to work, but it feels like the tool is starting to really get in the way of what we are trying to describe in the code.

Or if you use @Reusable you can cut down the verbosity, but I would argue it risks causing a sort of memory leak as a user leaves one part of the app to the next where an instance isn't needed anymore and may be a longer time period before needed again. I would like to retain the ability to throw some of those instances into the GC if given the opportunity.

So if given the 5 high level features and 20 api groups used within those, we end up with around 25 custom scopes where the problem we are trying to solve really only needs around 5 to communicate effectively.

This feature would reduce the above down to just:

@IntegratingFeatureScope
@Subcomponent(modules = [
  FooModule::class,
  BarModule::class,
  BazModule::class,
...
  FeatureModule::class
])
interface IntegratingFeatureComponent {
....
}

There might also be a good argument to support dynamic feature modules on android, though I don't have experience there so that is speculative at best.

I think the slowly growing verbosity and potential communication ambiguity at larger scales is the best justification for this feature request I will be able to materialize. . . . . .

I mentioned issues of inter project dependency in an earlier comment, that was mostly a product of having a mix of jar projects and aar projects where some of those aar projects included ndk/jni builds (thus non trivial to convert from aar to jar) so using a previously existing scope in that aar project in a new jar project forced us to create projects under them, and move classes around, when really it didn't matter what the scope was just that the bindings in the modules were all eventually the same scope. So while this feature request could help that distant edge case, it is not a strong justification.

The decoupling effort was motivated by trying to cut down build times. So we did end up having a project low in the build graph with just scopes and stub looking interface for components to extend, that is fast to compile, but I would argue that the lower level details shouldn't be depending on higher level ones directly. and the feature level scopes were certainly higher level ones that the api libraries could do without.

Mostly just issues that crop up in stumbling and learning while tearing down a monolith project into pieces to try and deal with team and feature growth.

Chang-Eric commented 4 years ago

So I definitely agree that trying to integrate a bunch of libraries all with their own custom scopes can be pretty clunky.

For the main problem of how does a library scope an object, I think there are a few general solutions out there right now, but all of them have some tradeoffs.

  1. Define a custom scope annotation as mentioned above. Tradeoff is this can quickly explode when you have a lot of libraries as mentioned.
  2. Define a globally known scope annotation. This is the method Hilt uses and so you probably wouldn't be surprised that this is what I tend to recommend. This also helps give a the library author a better understanding of what the lifetime of their object really is. The downside is that it requires coordination that doesn't always exist between many apps and libraries.
  3. Use an internal subcomponent in the library to manage all the deps. Here you can use your own scoping annotation that doesn't get seen by your users. Then the hosting app controls the scope by holding on to your subcomponent or a wrapper holding it (this like a FooLibraryEngine class) to their desired lifetime. The downside is that you may need to rework your API some.
  4. Don't use scopes in the library at all and require the app to scope/hold on to your objects as needed. Depending on the functionality/depth of your library this may work, but it tends to break down if you have a lot of entry points into your library.

I think the issue with adding an AutoScope is that it doesn't really give the library author any real guarantees, while at the same time not necessarily giving the app owner a clear picture of how long something is held since it may be hard to trace the module inclusion statically. There's also no error checking in the sense that if you pull it into the wrong component it will just continue to "work" silently.

I usually suggest going with 2 or 3 as a solution.

trevjonez commented 4 years ago

In thinking through your list of suggestions you have given me another idea: What if subcomponents can also have dependencies in the same form as @Component#dependencies.

I think I could POC this in a separate processor tool to generate a module within the library.

Define a component interface, though it doesn't even need to strictly be a @[Sub]Component, but could be.

@AutoModule
interface LibComp {

  val foo: Foo
}

resulting in

@Generated("com.hypothetical.AutoModuleProcessor")
@Module
object LibComp_AutoModule {

  @Provides
  fun LibComp.foo() = foo
}

in the later compilation unit consumer

@Subcomponent(modules = [LibComp_AutoModule::class]) // :( generated code ref
...
@BindsInstance instance: LibComp

At that point the scope could have a name but wouldn't need to be explicitly included or known on the subcomponent that is using it.

This might actually be super useful for hilt as a way to insert scopes between layers in its hierarchy. Though I am way down in the details of modeling domain in a dagger graph which is basically the complete opposite of the hilt strategy as far as I can tell.

The component management would be the last tricky piece to solve, but would always be custom to how you want to compose the graph anyway. Hell you could even implement LibComp as a subcomponent in some later compilation unit. In a way it also allows you to easily replace a module with a test version due to the dependency inversion boundary LibComp creates.

Given that dagger already knows how to deal with a component dependency bindings on a root component, would it even add much new complexity to support this type of thing directly?

I believe this type of composition between orthogonal life-cycles of domain and platform within a sub component hierarchy is one of the last missing pieces in the puzzle to really making the most of dagger. Where as dagger-android and hilt both ignore all the domain lifecycles.