google / dagger

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

Upgrading Hilt 2.42 -> 2.44 adds a new deprecation warning #3601

Open liutikas opened 1 year ago

liutikas commented 1 year ago

Upgrading Hilt 2.42 -> 2.44 adds a new deprecation warning.

When we upgraded androidx from Hilt 2.42 to 2.44 (https://android-review.googlesource.com/c/platform/frameworks/support/+/2242564) we started getting a new compile warning:

  > Task :hilt:hilt-navigation-fragment:hiltJavaCompileDebugAndroidTest
  $OUT_DIR/androidx/hilt/hilt-navigation-fragment/build/generated/hilt/component_sources/debugAndroidTest/androidx/hilt/navigation/fragment/HiltNavGraphViewModelLazyTest_TestComponentDataSupplier.java:16: warning: [deprecation] applicationContextModule(ApplicationContextModule) in Builder has been deprecated
          .applicationContextModule(
  1 warning

Test class https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:hilt/hilt-navigation-fragment/src/androidTest/java/androidx/hilt/navigation/fragment/HiltNavGraphViewModelLazyTest.kt;l=62

Chang-Eric commented 1 year ago

Hm, I suspect this might have to do with if something change such that nothing ever uses the @ApplicationContext Context binding. Could you try adding a usage of that binding in the test and see if the deprecation warning goes away?

danysantiago commented 1 year ago

For Dagger its fine if it adds @Deprecated to builders methods when the binding is not used since one can control the builder invocations, but if using Hilt one has no control when building the component so I think Hilt generated code should suppress the warning.

This is the generated file:

public final class HiltNavGraphViewModelLazyTest_TestComponentDataSupplier extends TestComponentDataSupplier {
  protected TestComponentData get() {
    return new TestComponentData(false, testInstance -> injectInternal((HiltNavGraphViewModelLazyTest) testInstance), Collections.emptySet(), Collections.emptySet(), (modules, testInstance, autoAddModuleEnabled) -> DaggerDefault_HiltComponents_SingletonC.builder()
         // ---> Calling a method that is marked as @Deprecated <---
        .applicationContextModule(
            new ApplicationContextModule(Contexts.getApplication(ApplicationProvider.getApplicationContext())))
        .build());
  }

  @SuppressWarnings("unchecked")
  private static void injectInternal(HiltNavGraphViewModelLazyTest testInstance) {
    ((HiltNavGraphViewModelLazyTest_GeneratedInjector) ((GeneratedComponentManager) Contexts.getApplication(ApplicationProvider.getApplicationContext())).generatedComponent()).injectTest(testInstance);
  }
}
Chang-Eric commented 1 year ago

Yea, I am looking to confirm that this is the actual cause of the problem before looking how to suppress this in Hilt (and/or change the deprecation behavior of Dagger).

kuanyingchou commented 1 year ago

Hi, @Chang-Eric , the warning does go away after adding a usage of Context: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:hilt/hilt-navigation-fragment/src/androidTest/java/androidx/hilt/navigation/fragment/HiltNavGraphViewModelLazyTest.kt;l=137?q=hilt%2Fhilt-navigation-fragment%2Fsrc%2FandroidTest%2Fjava%2Fandroidx%2Fhilt%2Fnavigation%2Ffragment%2FHiltNavGraphViewModelLazyTest.kt

Chang-Eric commented 1 year ago

Thanks, I think the right thing to do here then is to make Dagger not add that deprecation for bindings based on pruning, but just on whether the module itself will ever need an instance (i.e. if it only has static/abstract members).

TWiStErRob commented 1 year ago

I'm integrating Hilt for the first time to an existing project, and being greeted with this warning in code I didn't write is less than ideal.

Thanks @kuanyingchou for the workaround! For future users finding this, you can add a usage in the App too, no need for a ViewModel:

@HiltAndroidApp
public class App extends Application {
    // TODO Delete when https://github.com/google/dagger/issues/3601 is resolved.
    @Inject @ApplicationContext Context context;
@HiltAndroidApp
class App : Application() {
    // TODO Delete when https://github.com/google/dagger/issues/3601 is resolved.
    @Inject @ApplicationContext lateinit var context: Context
}
ffimnsr commented 6 months ago

This still happens today with the newly setup hilt android project. The current version is 2.51. Thanks for the current workaround @kuanyingchou and @TWiStErRob. Waiting for fixes on dagger#3601.