google / dagger

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

Suggestion needed for using Hilt in library modules #1991

Closed kalaiselvan369 closed 3 years ago

kalaiselvan369 commented 4 years ago

In my sample app I have created a separate library module called core. In the core module I am using network and db related dependencies like OkHttp, Retrofit and Room. Hence to access these instances as singleton, I declared them in the hilt module.

import com.mobile.app.core.BuildConfig
import com.mobile.app.core.remote.RemoteService
import com.mobile.app.core.remote.RemoteServiceInterceptor
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.android.components.ApplicationComponent
import okhttp3.OkHttpClient
import okhttp3.logging.HttpLoggingInterceptor
import retrofit2.Retrofit
import retrofit2.converter.gson.GsonConverterFactory
import javax.inject.Singleton

@Module
@InstallIn(ApplicationComponent::class)
object CoreModule {

    @Provides
    fun provideOkHttpClient(): OkHttpClient {
        return OkHttpClient
            .Builder()
            .addInterceptor(RemoteServiceInterceptor())
            .addInterceptor(HttpLoggingInterceptor().apply {
                level =
                    if (BuildConfig.DEBUG) HttpLoggingInterceptor.Level.BODY
                    else
                        HttpLoggingInterceptor.Level.NONE
            }).build()
    }

    @Provides
    @Singleton
    fun provideRetrofit(okHttpClient: OkHttpClient): Retrofit =
        Retrofit.Builder()
            .baseUrl("https://jsonplaceholder.typicode.com")
            .client(okHttpClient)
            .addConverterFactory(GsonConverterFactory.create())
            .build()

    @Provides
    @Singleton
    fun provideRemoteService(retrofit: Retrofit): RemoteService =
        retrofit.create(RemoteService::class.java)
}

In my app module I have my application class as mentioned below:

import android.app.Application
import dagger.hilt.android.HiltAndroidApp

@HiltAndroidApp
class MyApp : Application() {

    override fun onCreate() {
        super.onCreate()
    }
}

My app build.gradle also implements core module:

dependencies {
implementation project(path: ':core')
}

But when I build my project, it throws error as:

Task :app:kaptDebugKotlin error: cannot access OkHttpClient class file for okhttp3.OkHttpClient not found Consult the following stack trace for details.

cannot access OkHttpClient

I can overcome this error only if I change implementation to api for OkHttp client dependency which is delcared in the core module build.gradle. Is there anything I'm missing here.

Want I am trying to achieve is to have separate library modules for network service, local storage and firebase. So that I can implement this to app module as needed. Hence I started declaring Hilt module in each of these library modules like NetworkModule, StorageModule, FirebaseModule, etc. But I couldn't figure this out.

danysantiago commented 4 years ago

Using api is correct here. This is related to https://github.com/google/dagger/issues/970

In essence, because Hilt aggregates your modules into your root app Gradle project where Dagger generates the component implementation and because such code generated references all of your binding types in other modules, then the root app Gradle project must have visibility in its classpath to all of those other Gradle modules where you define Dagger modules along with its dependencies if they are used in those Dagger modules. It's a bit hard to see, but the factories Dagger generates for your bindings end up being public APIs used downstream so restricting those at the Gradle module boundaries using implementation is not quite right.

There is no current workaround in Hilt that I know off. We are aware of the implications this causes, such as leaking classes into other Gradle modules and possibly build performance impact with regards to compile avoidance. We've have some ideas on how to fix this and we hope to get them in before declaring Hilt stable.

kalaiselvan369 commented 4 years ago

Thanks @danysantiago

JavierSegoviaCordoba commented 4 years ago

The workaround in the issue mentioned by Dany was working the last time I checked it. Someone is mentioning it is not working with AGP 4 but I think I tried it even with AGP 4.1 with no problems.

karthikeyan1241997 commented 4 years ago

I had faced the same problem with my project. Instead of changing implementation to api (Not Recommended), you can add dependencies of missing libraries in app's build.gradle and rebuild the project. In your case, you can add Retrofit and OkHttp dependency in app's build.gradle.

kalaiselvan369 commented 4 years ago

@karthikeyan1241997 May be. But I am not sure why app module should be aware of these dependencies(Retrofit+Okhttp) when the are in separate library module in my case.

AleksaMatic-TomTom commented 3 years ago

Is this fixed now considering https://github.com/google/dagger/issues/970 is closed

bcorso commented 3 years ago

Unfortunately no, the general issue still exists (but as @danysantiago mentions in https://github.com/google/dagger/issues/1991#issuecomment-661245887 we're still looking into it).

The issue in #970 was a very specific case of this problem where api was only needed to check validation of @Component.dependencies for cycles. For that specific case we decided to add a flag that disables that validation so that implementation would work (with the understanding that the flag should probably only be disabled in dev).

joshfriend commented 3 years ago

It seems to me like Hilt is breaking everyone's ability to take advantage of gradle compile avoidance, and is polluting gradle modules with transitive dependencies.

If all of my module dependencies have to be declared api, and I can no longer have an @Module in my libraries without it being required to have @InstallIn added, what is even the point of having separated gradle modules when using hilt?

danysantiago commented 3 years ago

There are still great benefits to making separate gradle modules, for one you can have smaller tests on them, including 'small test APKs' where the dependencies set and the modules in the classpath are reduced.

AFAIK compile avoidance is not completely lost when using api vs implementation. It's also important to clarify that this is an issue present not only in Hilt but in Dagger in general. The difference in Hilt is that it can be easier to encounter since modules are aggregated into the components whereas in vanilla Dagger you would have to specify the Dagger module class in the @Component-annotated interface, which would ultimately require you to either expose those Gradle modules via api dependencies or make the app module (or wherever the component is) depend directly on the Gradle modules containing the Dagger modules.

There are workarounds by introducing indirections in the bindings that Dagger can provide, at the cost of an increased complexity in the Gradle modules setup, one example of this is explained and shown by the following talk in Droidcon: Android at Scale @Square

snepalnetflix commented 3 years ago

It seems to me like Hilt is breaking everyone's ability to take advantage of gradle compile avoidance, and is polluting gradle modules with transitive dependencies.

One way to mitigate this problem is to have your application gradle module be tiny. The application gradle module only contains your Application class and defines the root of your dependency injection graph.

You then compose your application by pulling in the necessary dependencies. If you have build variants, this approach makes it easy to have different features in different variants.

I find that the following organization works very well:

app/build.gradle looks like this:

feature1/module/build.gradle looks like this:

With this approach we never run into class file not found errors.

kalaiselvan369 commented 3 years ago

Hi @snepalnetflix do you have any sample repository implementing the structure you have mentioned above. It would be great if you could share one. Thank you.

snepalnetflix commented 3 years ago

@kalaiselvan369 I'm working on a Hilt Android example. It's not working yet but you can get an idea of the structure. See: https://github.com/snepalnetflix/hilt-giphy

In this project all feature/module projects declare dependencies on their api and impl using:

I thought that this would be enough to prevent class not found errors. Unfortunately I ran into one class not found error:

error: cannot access PagingDataAdapter
  class file for androidx.paging.PagingDataAdapter not found

I was surprised by this since I haven't really run into errors like this on my real project. I think this is an infrequent enough problem that explicitly adding the paging library as a dependency on the app module is acceptable.

datanappsprojects commented 3 years ago

You have to use

@Provides fun provideRemoteService(retrofit: Retrofit): RemoteService = retrofit.create(RemoteService::class.java)

don't use @Singlton

massivemadness commented 3 years ago

Any news about this?

danysantiago commented 3 years ago

@massivemadness, in Hilt 2.31.x-alpha we added an experimental flag to be able to use Hilt in a layered multi-module project. See the docs at https://dagger.dev/hilt/gradle-setup#classpath-aggregation. Please try it out and let us know if you run into trouble, its experimental because it has build performance implications we want to solve before completely committing to it.

sanginovs commented 3 years ago

@danysantiago any timeframe on when you are working on improving build performance issue in the classpath-aggregation?

jeffpenga commented 2 years ago

@danysantiago

I see that according to https://dagger.dev/hilt/gradle-setup#classpath-aggregation enableExperimentalClasspathAggregation is now deprecated in favor of enableAggregatingTask.

However, when I use

hilt {
    enableAggregatingTask = true
}

I get still get the

java.lang.ClassCastException: com.example.DaggerMyApplication_HiltComponents_SingletonC$FragmentCImpl cannot be cast to com.example.MyFragment_GeneratedInjector

error described in #2064, which doesn't happen if I continue to use

hilt {
    enableExperimentalClasspathAggregation = true
}

Should enableAggregatingTask be able to fix this bug the same way that enableExperimentalClasspathAggregation does?

One extra thing to note, my project is using version 4.2.1 of the Android Gradle Plugin, perhaps that affects things?

danysantiago commented 2 years ago

@jeffpenga the issue as you describe and the exception seems to be a different than this bug, do you mind filing a new issue so we can investigate?

Additionally, can you please specify if you are applying the Hilt compiler in all your Gradle modules that use Hilt and if you proguard / r8 your app (debug or prod) and if turning off proguard / r8 makes any difference?

jeffpenga commented 2 years ago

@danysantiago Yes, We are applying the Hilt compiler in all the gradle modules that use Hilt and we aren't using Proguard/R8 at the moment.

And I've filed #3004 as a new issue, detailing what I've discovered. Thanks for your help!

Mohsents commented 2 years ago

In my case i had many terrible to fix this. Finally i notice one dependence missing in app module so i applied the api over implementation, in library module and successfully overcome to this. You can also declare that particular dependency directly in your module.