google / dagger

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

Feature Request: Allow defining parent of built in components #1920

Open digitalbuddha opened 4 years ago

digitalbuddha commented 4 years ago

Hi folks, thank you for Hilt, particularly the documentation. I had a feature request/question. Is there any reason why @DefineComponent allows declaring the parent component but the built in components do not? Ideally I'd like to do the following:

Declare a UserComponent as a child of ApplicationComponent Declare ActivityComponent to be the child of UserComponent

From what I gather this means I'd need to maintain the Manager around the UserComponent as well as any Activity/Fragment components that I create. I'd like to delegate to ActivityComponentManager and FragmentComponentManager.

Thank you kindly, I'm sure there is a reason why this does not work out of the box

digitalbuddha commented 4 years ago

I made a minimal sample of what I think it takes to create a user scope. https://gist.github.com/digitalbuddha/4c63034969c107af3c98e098ae5ffbe7

Chang-Eric commented 4 years ago

So there are a few technical challenges that make this difficult to support.

The first is that custom components inserted in the tree are very likely to have arguments that need to be passed to the component builder. In your case, this would be the user id. The issue here is that because Hilt creates your components for you, we would have to figure out a way to get those arguments at the right time.

There’s also a question of storing the components we created for you. For example, is your inserted component one instance for the application or one instance per activity (or something else like one instance per user id?) Because Hilt hides the components, adding in hooks to control this would be complicated.

Finally, there’s a question of compatibility with extensions/libraries. For example, there are two basic paths for the activity component. The first is using the same dagger.hilt.android.components.ActivityComponent and just changing the parent somehow. That is difficult though because even ignoring the fact that the parent is statically defined, what does this mean for a library using @AndroidEntryPoint that your app uses. Do they suddenly become injected from the user component as well? It likely isn’t always safe to make a library activity account-aware as maybe it could access account data in a way that isn’t apparent. The alternative is only your @AndroidEntryPoint activities you declare are injected from your ActivityUserComponent (like you defined in your example). But that also has problems because now things like the ViewModel extension don’t work because they installed bindings in the standard Hilt ActivityComponent.

Anyway, I guess that is kind of a long way to say that supporting this would be very hard.

digitalbuddha commented 4 years ago

Thank you for explaining. The difficulty of the problem is one of the reasons I think it should be a base experience. Even if it is a single account/user scope similar to the retained activity scope. Thank you for verifying the gaps I had in knowledge

Chang-Eric commented 4 years ago

I think adding a built-in account component would be tough because there's a lot of stuff that goes into managing accounts that I think starts to get out of the scope of Dagger. For example, for an account component to make sense you'd need something managing the lifetimes of those components which starts to get into tricky questions like how are account switches handled with respect to the activity lifetime, etc.

There's also issues of if the AccountComponent is a parent of say the Activity, then it suddenly requires people to think about/handle accounts immediately when using Hilt even if their app doesn't have accounts.

With that said, I'm going to try to add some more docs to the site on this topic because it is a complex topic and one that is likely to come up for many people. Hope this all helps.

totrit commented 3 years ago

We encountered similar issue when we try to assess feasibility of incorporating Hilt into our app which also involves user accounts. With plain Dagger we are able to throw away the overarching dependency graph upon user's logout so that we will not have any residual state.

With Hilt it's impossible to do this for now. Hilt doesn't facilitate a way for us to control lifetime of user account related injectables whilst let them have easy mutual access to other standard Hilt components especially SingletonComponent.

Can there be another option other than the one discussed above: allow us to control the re-creation of SingletonComponent so that we can have a clean dependency graph upon user logout/switching?

Thank you for your time and great Hilt.

Chang-Eric commented 3 years ago

I think it would be hard to do this because this would really change the definition of @Singleton within Hilt. Libraries that expect @Singleton to mean one instance for the life of the Application may be surprised by such a change and it'd be hard to roll out such a change safely at this point. It also only addresses one particular way of handling accounts (e.g. it probably only works for apps where switching the current account is a uncommon flow since it is so destructive, like all activities/services would need to be recreated since those stem from the SingletonComponent), so we wouldn't even really be fully solving the problem.

totrit commented 3 years ago

Hi @Chang-Eric many thanks for your instant reply. I realize controling lifetime of SingletonComponent won't be everyone's desire. Do you think it'd be good to provide an API for us to manually call to recreate the SingletonComponent and it'll be an optional thing to do. So potentially those who don't need this feature can still go on their way?

Thank you for your time.

Chang-Eric commented 3 years ago

I think the hard part is that Hilt is looking to facilitate different libraries to build upon the standard scopes and components. As an example, the WorkManager extension might have some @Singleton in it and even allowing the possibility for this to be recreated by some apps would impact how the authors of that extension write their code. It would be a big and disruptive shift in the invariants we guarantee so we would hopefully only do that for equally big gains. I'm not sure this meets that bar since it doesn't even fully solve the accounts problem.

I think managing your own component using @DefineComponent would still be the best way to go as that doesn't affect any of the current standard Hilt components, though of course I recognize that it will be harder to manage because it won't be a parent of our @ActivityComponent and stuff for normal Android types. In these cases, you'd have to use an entry point to access the account bindings you want. Sorry there isn't really a better answer at this time.