Skip to content
This repository has been archived by the owner on Oct 17, 2023. It is now read-only.

Split generated AssistedInject_*Module to multiple modules #87

Closed
Nimrodda opened this issue Mar 16, 2019 · 15 comments
Closed

Split generated AssistedInject_*Module to multiple modules #87

Nimrodda opened this issue Mar 16, 2019 · 15 comments

Comments

@Nimrodda
Copy link

Currently the generated AssistedInject_*Module contains all the bindings for @AssistedInject.Factory annotated factories in one module. As a result, it's impossible to have dependencies that exist only in subcomponents. For example:

I have two ViewModels, where one of them takes an argument (GithubApi in this case), which exists only in one the subcomponent hosting these view models.

class CommitDetailViewModel @AssistedInject constructor(
    @Assisted handle: SavedStateHandle,
    privatel val githubApi: GithubApi // Should be provided only in CommitDetailActivitySubcomponent
) : BaseViewModel(handle) {
    fun loadDetail() {
        Timber.v("Request commit detail")
        // use githubApi here
    }

    @AssistedInject.Factory
    interface Factory : ViewModelAssistedFactory<CommitDetailViewModel>
}

class CommitListViewModel @AssistedInject constructor(
    @Assisted handle: SavedStateHandle
) : BaseViewModel(handle) {

    fun loadCommits() {
        Timber.v("Request commit list")
    }

    @AssistedInject.Factory
    interface Factory : ViewModelAssistedFactory<CommitListViewModel>
}

interface ViewModelAssistedFactory<T : ViewModel> {
    fun create(handle: SavedStateHandle): T
}

@AssistedModule
@Module(includes = AssistedInject_ViewModelAssistedFactoriesModule.class)
public abstract class ViewModelAssistedFactoriesModule {
}

The generated AssistedInject_ViewModelAssistedFactoriesModule contains the bindings for both factories above. Due to that, I have to include it in the ApplicationComponent so it's visible to my subcomponents. But then compilation fails since GithubApi's binding is only defined in one of the subcomponents.

@JakeWharton
Copy link
Member

How do we know which bindings are semantically grouped together?

@Nimrodda
Copy link
Author

I was initially thinking that splitting the generated module to one module per factory would be an option, but actually it's not that different than just setting up the bindings manually :/

@JakeWharton
Copy link
Member

JakeWharton commented Mar 20, 2019 via email

@davidliu
Copy link

Could we have the @AssistedInject.Factory take some sort of user-defined @AssistedGrouping annotation (somewhat conceptually like the @mapkey stuff in dagger) that the @AssistedModule would use in its creation?

@JakeWharton
Copy link
Member

Today you can choose not to use @AssistedModule and specify the @Binds methods yourself in whichever modules you want. That's not that far off from what you're asking.

Ideally the @ContributesAndroidInjector mechanism would be generalized so we could have @ContributesAssistedFactory Foo.Factory foo() and it would do the right thing.

@davidliu
Copy link

Today you can choose not to use @AssistedModule and specify the @BINDS methods yourself in whichever modules you want. That's not that far off from what you're asking.

Well the issue is still the extra boilerplate creation (which is why @AssistedModule even exists, I assume.) It'd be nice to not have to write those extra binds for every factory.

@davidliu
Copy link

@Nimrodda google/dagger#1781 might be related to your problem.

Re-including the assisted module in the subcomponents will let the assisted module search the subcomponent for provisions (in your case GithubApi).

Feels unintuitive to reinclude a module in a subcomponent, but it definitely solves the need for splitting the AssistedInject module into multiple modules.

@dmytroKarataiev
Copy link

Re-including the assisted module in the subcomponents will let the assisted module search the subcomponent for provisions (in your case GithubApi).

Could you please elaborate on that?

@davidliu
Copy link

Here's another example to be more specific:

@AssistedModule
@Module(includes = [AssistedInject_AssistedModule::class])
abstract class AssistedModule

@dagger.Component(modules = [AssistedModule::class])
interface AppComponent {
    fun commitDetailActivitySubcomponent(): CommitDetailActivitySubcomponent
}

// Reinclude AssistedModule so that the subcomponent can fulfill the binding.
@dagger.Subcomponent(modules = [AssistedModule::class, GithubModule::class])
interface CommitDetailActivitySubcomponent {
    fun vmFactory(): CommitDetailViewModel.Factory
}

@Module
static class GithubModule {
    @Provides
    fun githubApi(): GithubApi {
        return GithubApi()
    }
}

class CommitDetailViewModel @AssistedInject constructor(
    @Assisted handle: SavedStateHandle,
    private val githubApi: GithubApi
) : BaseViewModel(handle) {
    // ....
}

There's two things happening here.

  1. @Binds (and subsequently AssistedModule) doesn't actually impose the dependency requirement on the component it's attached to. To me, there's an assumption that it does, since Dagger is so strict with types elsewhere, it's actually a little surprising that it's okay with this. That means that even though there's a @Binds which requires GithubApi to fulfill, it's okay for that to exist in a component without the GithubApi, since no one actually uses that binding at the AppComponent level. So AssistedModule is safe to use, even in places where not all the dependencies exist yet.

  2. @Binds does not exactly carry over to subcomponents. If we consider dagger provisions to be sort of a pure mathematical formula (object A + object B lets you build object C), you'd figure that the declaration of this formula at the top level means you'd be able to use this formula anywhere. However, this assumption is also incorrect; the formula only executes with the component context where it's declared.

    For example, if we omitted the AssistedModule from the ActivitySubcomponent, the AssistedModule could only search the AppComponent for fulfilling dependencies, and would not be able to get GithubApi for fulfilling the CommitDetailViewModel.Factory binding, even though it's actually being provided.

@dmytroKarataiev
Copy link

dmytroKarataiev commented Jun 11, 2020

@davidliu my use case is a little bit more complicated.
I have multiple modules where I have subcomponents. I want to isolate dependencies from these subcomponents.

Here is an example of what I mean:
Now

That's what I want (and as far I understand it's not possible with a single AssistedModule):
Future

Is there a way to make it work still?
How can I manually create an assisted module? Or provide dependencies at a later time?

@davidliu
Copy link

Like I mentioned, the AssistedModule doesn't impose a dependency requirement. You can put it in each of your components, and it will be fine. Declaring the AssistedModule in your app component won't require you to have the MainRepo at the app level; you just also need to have the AssistedModule at the MainActivity level. Just try it out in code instead of trying to figure it out beforehand.

I'm not sure how to be any more clear in my explanation so I just made an example project for this:

https://github.com/davidliu/MultiLevelAssistedModule/tree/master/app/src/main/java/com/deviange/multilevelassistedmodule

@dmytroKarataiev
Copy link

@davidliu thank you for the answer and for the example. Your repo was extremely helpful!
Assisted module doesn't even need to be included anywhere except in a subcomponent that needs it.
My problem was that my assisted module also had dependencies listed in there (that were imposing a dependency requirement).

@GuilhE
Copy link

GuilhE commented Jun 12, 2020

Like I mentioned, the AssistedModule doesn't impose a dependency requirement. You can put it in each of your components, and it will be fine. Declaring the AssistedModule in your app component won't require you to have the MainRepo at the app level; you just also need to have the AssistedModule at the MainActivity level. Just try it out in code instead of trying to figure it out beforehand.

I'm not sure how to be any more clear in my explanation so I just made an example project for this:

https://github.com/davidliu/MultiLevelAssistedModule/tree/master/app/src/main/java/com/deviange/multilevelassistedmodule

Using the AssistedInject_AssistedModule approach worked like a charm.
@davidliu always saving the day 🙏

@GuilhE
Copy link

GuilhE commented Jan 27, 2021

Just to leave the note that for Dagger users the latest version (2.31) has Assisted injection included, so this problem no longer exists (I solved it by removing this dependency).

@JakeWharton
Copy link
Member

Yep. Realistically, we never were going to fix this. The correct fix is to either allow Dagger to automatically discover modules or to fully upstream the assisted feature. The latter happened.

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

No branches or pull requests

5 participants