google / dagger

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

Dagger Hilt with library modules gets ClassCastException, fix with enableExperimentalClasspathAggregation vs. enableAggregatingTask? #3004

Closed jeffpenga closed 3 years ago

jeffpenga commented 3 years ago

Intro

This is basically the same crash that's described in #2064 "Dagger Hilt with library modules", but my question is about using enableExperimentalClasspathAggregation vs. enableAggregatingTask as a means to fix the crash.

Background

We're using Dagger and Hilt, in a project that has an Android app module apk (code specific to one platform), which has a dependency on a library module base (general code applicable to all platforms), which has a dependency on a library module moduleC (code to implement one specific area of the app).

apk -> base -> moduleC

Here are excerpts from their build.gradle files.

apk/build.gradle

apply from: rootProject.file('dagger.gradle')

dependencies {
    implementation project(':base')
}

base/build.gradle

apply from: rootProject.file('dagger.gradle')

dependencies {
    implementation project(':moduleC')
}

moduleC/build.gradle

apply from: rootProject.file('dagger.gradle')

They each pull in the Dagger/Hilt dependencies from a central dagger.gradle file:

apply plugin: 'kotlin-android'
apply plugin: 'kotlin-kapt'
apply plugin: 'dagger.hilt.android.plugin'

dependencies {
    implementation "com.google.dagger:hilt-android:2.38.1"
    kapt "com.google.dagger:hilt-android-compiler:2.38.1"
}

and we're using AGP 4.2.1

        classpath 'com.android.tools.build:gradle:4.2.1'

Problem

Unfortunately, when we attempt to use Hilt in a Fragment in moduleC, we get the following crash when that Fragment is created: java.lang.ClassCastException: com.example.DaggerMyApplication_HiltComponents_SingletonC$FragmentCImpl cannot be cast to com.example.MyFragment_GeneratedInjector just like in #2064.

Potential Solution?

Now #2064 was closed as a duplicate of #1991 "Suggestion needed for using Hilt in library modules", so we tried to follow the advice given in https://github.com/google/dagger/issues/1991#issuecomment-765612679 to use enableExperimentalClasspathAggregation as described at https://dagger.dev/hilt/gradle-setup#classpath-aggregation, and that does fix the crash, if we add it like so:

dagger.gradle

apply plugin: 'kotlin-android'
apply plugin: 'kotlin-kapt'
apply plugin: 'dagger.hilt.android.plugin'

dependencies {
    implementation "com.google.dagger:hilt-android:2.38.1"
    kapt "com.google.dagger:hilt-android-compiler:2.38.1"
}

hilt {
    enableExperimentalClasspathAggregation = true
}

android {
    lintOptions {
        checkReleaseBuilds false
    }
}

Caveat?

However, we saw that enableExperimentalClasspathAggregation has been deprecated in favor of using enableAggregatingTask as described at https://dagger.dev/hilt/gradle-setup#aggregating-task, but when we tried using enableAggregatingTask instead, like this:

dependencies {
    implementation "com.google.dagger:hilt-android:2.38.1"
    kapt "com.google.dagger:hilt-android-compiler:2.38.1"
}

hilt {
    enableAggregatingTask = true
}

we do get the ClassCastException once again.

Extra Data Point

What's strange is that if we change our dependency on com.google.dagger:hilt-android to use api instead of implementation, like this:

dependencies {
    api "com.google.dagger:hilt-android:2.38.1"
    kapt "com.google.dagger:hilt-android-compiler:2.38.1"
}

hilt {
    enableAggregatingTask = true
}

then it actually does work (no ClassCastException).

Real Question / Potential Issue

So my question is, should using enableAggregatingTask = true be able to fix these apk -> base -> moduleC dependency problems, the way that the deprecated enableExperimentalClasspathAggregation = true does?

danysantiago commented 3 years ago

Yes, enableAggregatingTask should fix dependency problems, but note that it only does it for the Dagger component generating and compiling task, unlike enableExperimentalClasspathAggregation which overexposed implementation dependencies up the chain which is bad for build performance and compilation avoidance.

If you do expose types in your base and moduleC libraries that come from dependencies then it should be declaring an api dependency to them. You can read a bit more of when to use api vs implementation in the Gradle docs: https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_recognizing_dependencies

The 'extra point' is super odd, I'm not sure how it fixes the issue since you are adding hilt-android in all modules already, it would have make more sense if you added it to only a few of the modules (say base and moduleC) but not to others and using api would propagate Hilt's runtime.

I'm also interested in learning more about this base module library, is it a Kotlin Multiplatform module? My suspicious is around it, the way we 'aggregate' deps into the component compiling task is via configuration resolution plus some compile tasks outputs, so it can be possible if the module is multiplatform that we are not using the right resolving attributes which ends up not aggregating moduleC. I'll take a look at this.

My other question to you is, where is your @HiltAndroidApp? Is it in the apk module ?

jeffpenga commented 3 years ago

Actually our base module is not a Kotlin Multiplatform module, it is itself simply another Library module, much like moduleC (it is perhaps somewhat confusingly named).

apk (Android Base module) -> base (Android Library module) -> moduleC (Android Library module)

And our @HiltAndroidApp is in the apk module, yes.

jeffpenga commented 3 years ago

Regarding allowing base and moduleC to expose types that come from their dependencies (by importing those dependencies using api instead of implementation):

Our set up is that apk references types in base and base references types in moduleC, but we (at least currently) aren't intending for apk to directly reference any types declared in moduleC, which is why we don't declare a direct dependency from apk to moduleC.

Is my understanding of Hilt correct in that, if our @HiltAndroidApp Application class is in apk, and there are @AndroidEntryPoint Fragments in moduleC, then Hilt itself will require/cause there to be a dependency from apk to moduleC?

danysantiago commented 3 years ago

Yes - But you shouldn't have to worry about it. Let me explain...

Hilt creates the @Component interfaces that Dagger later generates an implementation for in the same module where you have @HiltAndroidApp, in this case it would be the apk module. When you annotated a Activity / Fragment with @AndroidEntryPoint Hilt basically generates the following interface / entry point:

@GeneratedEntryPoint
@InstallIn(FragmentComponent.class)
public interface ModuleCFragment_GeneratedInjector {
  void injectModuleCFragment(ModuleCFragment moduleCFragment);
}

This will get picked-up by the Hilt processor in apk and to put it simply a @Component in apk will end implementing that interface so that the generated Hilt_ModuleCFragment in base performs field injection. Remember that the Gradle Plugin Hilt will do a bytecode transform so that your fragment extends the generated Hilt_ModuleCFragment.

So in apk Hilt generates the components definitions and it somewhere there is a:

@Subcomponent
class FragmentComponent implements ModuleCFragment_GeneratedInjector

You can actually find these generated sources in build/generated/hilt and build/generated/source/kapt.

As you can see, indeed a class in apk references a class in moduleC, but you don't need to expose moduleC to apk because the @Component sources are compiled in a different task, different from :apk:compileDebugKotlin / :apk:compileDebugJavaWithJavac. Specifically the Hilt Gradle Plugin creates a :apk:hiltJavaCompileDebug task that generates the @Component implementations and compiles them and the classpath to such task is the aggregation of all direct and transitive deps of apk. This (and a few other things) are the big differences between enableAggregatingTask and enableExperimentalClasspathAggregation, the latter doing the same aggregation in the main compile task.

The reason we deprecated enableExperimentalClasspathAggregation is because it degrades build times, it has a big impact in it because any change in any transitive module will cause a recompilation of apk. Where as with enableAggregatingTask aggregation is done in an isolated task and if you make changes in deep modules (such as moduleC) and they don't affect the DI graph, or are changes that don't affect ABI (like changing a method body) then compilation avoidance makes it so apk is not recompiled.

Now consider how in 'vanilla Dagger' (a.k.a not Hilt), you manually write the @Component and thus you end up writing the member injecting method fun injectFragmentC(fragment: FragmentC), which would force you to make apk depend on moduleC so that you can reference the fragment type. Hilt does this for you automatically, which has the benefits of not over-exposing other classes in moduleC just because you have to wire DI for the fragment.

Anyhow, in practical terms, use implementation as much as possible, and if necessary or if it makes sense, use api to expose transitive dep of a library.

danysantiago commented 3 years ago

As for you current issue, the bad news is I can't easily reproduce, see https://github.com/danysantiago/Hilt3004. I've created a 'similar' module chain, app -> base -> modulec but things compile fine and there is no crash at runtime. Hilt is 2.40, AGP is 4.2.2 and Gradle is 7.0.2.

jeffpenga commented 3 years ago

Aha! Our project was using the 2.38.1 version of Hilt, and once I upgrade to 2.40 it fixes the problem! (In retrospect, I probably should've tried upgrading the Hilt version as my first investigation step, that's an oversight on my part.)

Thank you for all your help Dany, it was very informative!