Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflicting documentation for dagger.android and Hilt optional inject #4232

Closed
arekolek opened this issue Jan 31, 2024 · 3 comments
Closed

Comments

@arekolek
Copy link

arekolek commented Jan 31, 2024

https://dagger.dev/dev-guide/android.html#when-to-inject says:

For this reason, DaggerActivity calls AndroidInjection.inject() immediately in onCreate(), before calling super.onCreate(), and DaggerFragment does the same in onAttach(), which also prevents inconsistencies if the Fragment is reattached.
It is crucial to call AndroidInjection.inject() before super.onCreate() in an Activity, since the call to super attaches Fragments from the previous activity instance during configuration change, which in turn injects the Fragments. In order for the Fragment injection to succeed, the Activity must already be injected

while https://dagger.dev/hilt/optional-inject.html has this code snippet:

  override fun onAttach(activity: Activity) {
    super.onAttach(activity)  // Injection will happen here, but only if the Activity used Hilt
    if (!OptionalInjectCheck.wasInjectedByHilt(this)) {
      // Get Dagger components the previous way and inject
    }
  }

(it was also mentioned in #2132 (comment))

So this means having:

  // in activity:
  override fun onCreate(savedInstanceState: Bundle?) {
    super.onCreate(savedInstanceState)
    if (!OptionalInjectCheck.wasInjectedByHilt(this)) {
      AndroidInjection.inject(this)
    }
  }

  // in fragment:
  override fun onAttach(context: Context) {
    super.onAttach(context)
    if (!OptionalInjectCheck.wasInjectedByHilt(this)) {
      AndroidSupportInjection.inject(this)
    }
  }

But isn't that a problem if you call AndroidInjection.inject(this) (or AndroidSupportInjection.inject(this)) after super.onCreate (or super.onAttach)? (Like it says it is crucial to avoid it in dagger.android docs). I can't reproduce the problem now, but I seem to remember that I've seen those issues in the past.

On the other hand, if you do it like in dagger.android docs, it simply doesn't work at all, so...

@Chang-Eric
Copy link
Member

Chang-Eric commented Jan 31, 2024

Sorry I'm not entirely following. Aren't the dagger.android docs talking about when you should inject the Activity while the Hilt docs are talking about when the Fragment is injected?

@arekolek
Copy link
Author

arekolek commented Feb 1, 2024

Hilt docs have a code snippet with Fragment code, yes, but discuss any @AndroidEntryPoint that has a parent, which means it could be an Activity too:

If you mark an @AndroidEntryPoint class with @OptionalInject (...) This gives you the chance to provide dependencies in a different way

dagger.android docs talk about both too. Part about fragments is this:

before calling super (...) DaggerFragment does the same in onAttach(), which also prevents inconsistencies if the Fragment is reattached.

Furthermore, even if it was like you say, let's say we want to follow dagger.android advice for an Activity: If I try checking OptionalInjectCheck.wasInjectedByHilt(this) before super.onCreate(savedInstanceState), then it will simply not work, because wasInjectedByHilt will return correct value only if it is called after super

I edited OP to try and make this more clear

@Chang-Eric
Copy link
Member

Ooh I got it now, thanks. This is a good question!

This is actually fairly tricky, but I think to do it you for an Activity you would want to register an OnContextAvailableListener and do the injection logic in there. As long as your OnContextAvailableListener is registered after the Hilt one (and this should happen because we register our listener in the Hilt base class's constructor, so even doing it in your constructor should put yours after ours since super constructors would be called first), then it should work. These listeners get called within the onCreate of the Android Activity base class and so should also satisfy the dagger.android docs where these are called before fragments are attached.

I can update the documentation, thanks for pointing this out!

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

No branches or pull requests

2 participants