google / dagger

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

InjectProcessingStep was unable to process #3959

Closed yoobi closed 11 months ago

yoobi commented 11 months ago

Hello I'm having a weird issue (not sure if this is a bug or not)

To showcase this issue I've created this repository which reproduce the error. It fails at compilation with the following error message:

error: InjectProcessingStep was unable to process 'errorHandlerRepository' because 'io.yoobi.analytics.AnalyticsRepository' could not be resolved.

  Dependency trace:
      => element (CLASS): io.yoobi.base.fragment.BaseFragment
      => element (FIELD): analytics
      => type (ERROR field type): io.yoobi.analytics.AnalyticsRepository

  If type 'io.yoobi.analytics.AnalyticsRepository' is a generated type, check above for compilation errors that may have prevented the type from being generated. Otherwise, ensure that type 'io.yoobi.analytics.AnalyticsRepository' is on your classpath.

InjectProcessingStep was unable to process 'errorHandlerRepository' because 'io.yoobi.analytics.AnalyticsRepository' could not be resolved.

When I remove either @Inject lateinit var analytics: AnalyticsRepository or @Inject lateinit var errorHandlerRepository: ErrorHandlerRepository the project compiles fine.

I've also noticed that if I add implementation project(":analytics") in the build.gradle of my feature-home the project compiles correctly.

bcorso commented 11 months ago

Hi @yoobi, This can happen when you don't have access to the type on the classpath during annotation processing, but I would need more information about your setup to be able to tell you more about why its failing in your particular case.

One example where you could hit this is if you have your Gradle libraries organized as follows:

FooFragment (lib1) -> BaseFragment (lib2) -> AnalyticsRepository (lib3)

Then you could run into this error while compiling lib1. The fix is to change the lib2 -> lib3 dependency from implementation to api so that lib1 has access to it during annotation processing.

If this example doesn't help you solve your issue then please give more details of your setup and I can give more specific advice.

yoobi commented 11 months ago

Hello @bcorso my case is exactly like the example https://github.com/yoobi/hiltbugrepository in my first message. Is this a bug from dependency injection or is it my implementation which is wrong ?

I'd like to avoid using api in my architecture.

bcorso commented 11 months ago

Ah, sorry I somehow missed that in your first message.

Is this a bug from dependency injection or is it my implementation which is wrong?

In this case your implementation is wrong. Dagger needs all classes it's processing to be on the classpath during annotation processing and you control the classes on the classpath via how you set up your dependencies in Gradle.

In this case, the proper fix is to expose :analytics via an api dependency instead of implementation here:

https://github.com/yoobi/hiltbugrepository/blob/2fdec97d74127c64dfbfd05e56a08acecc5da665/base-fragment/build.gradle#L36-L37

For example:

dependencies {
    api project(":analytics")
}
yoobi commented 11 months ago

To make sure I understand the issue correctly, I'll reuse your example HomeFragment (lib1) -> BaseFragment (lib2) -> AnalyticsRepository (lib3) As lib2 implements libs3 and libs1 implements libs2, during annotation processing libs1 is missing libs3 which provides AnalyticsRepository and it can't find it and so it is causing the current issue ? Won't the enableAggregatingTask = true fix this ?

bcorso commented 11 months ago

As lib2 implements libs3 and libs1 implements libs2, during annotation processing libs1 is missing libs3 which needs AnalyticsRepository but doesn't find it and so it is causing the current issue?

Yes, that's correct.

Won't the enableAggregatingTask = true fix this?

No, enableAggregatingTask only aggregates hilt generated dependencies, it does not aggregate all of your transitive dependencies onto the classpath. That's actually what the old flag (enableExperimentalClasspathAggregation) used to do, but that flag is not recommended because it's bad for build performance (it essentially turns all of your implementation dependencies into api, whereas you should be able to only use api where needed if you do it manually).

yoobi commented 11 months ago

Thank you a lot ! I've got a clearer understanding of what's going on and why implementation project(":analytics") from feature-home module "fixed" it.

I'll have to use api in this particular use case.

Side question: Why does the DI not generate classpath from the end to the beginning ? I mean why doesn't it first generate classpath from lib3to lib2 then lib2 to lib1 ? (Not sure if this makes sense to you)

bcorso commented 11 months ago

Side question: Why does the DI not generate classpath from the end to the beginning ? I mean why doesn't it first generate classpath from lib3to lib2 then lib2 to lib1 ? (Not sure if this makes sense to you)

I think what you're asking is why we don't aggregate transitive dependencies automatically on the classpath, similar to what we do for Hilt?

If so, there's a few different reasons for this.

First, Hilt's enableAggregatingTask is really solving a much simpler issue of just aggregating Hilt's generated classes, it's not trying to aggregate all of the transitive dependencies we would need for annotation processing.

Second, the classpath is typically controlled by the user (via api and implementation dependencies). The reason we're able to tweak it in Hilt is only by using the Hilt Gradle Plugin). Unfortunately, even with a Gradle plugin, I don't think there's a good way for us to know all of the transitive dependencies Dagger would need to generate your component. The best we could do is something like enableExperimentalClasspathAggregation where we just add all transitive dependencies to the classpath.

yoobi commented 11 months ago

I think what you're asking is why we don't aggregate transitive dependencies automatically on the classpath Yes that's a better wording for it !

Thank you for your time and the detailed explanation