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

correctErrorTypes causes DataBindingComponent compile errors #2603

Closed
dmapr opened this issue May 10, 2021 · 8 comments
Closed

correctErrorTypes causes DataBindingComponent compile errors #2603

dmapr opened this issue May 10, 2021 · 8 comments

Comments

@dmapr
Copy link

dmapr commented May 10, 2021

Not sure whether this is Hilt, Kapt or both, but I was migrating a project from Dagger Android to Hilt and followed the guide by @nuhkoca in #2030. However my project refused to compile the generated custom data binding component implementation. I found that a Github repo was created based on @nuhkoca's gist and was able to narrow it down to this option. I've forked the project to demonstrate the problem to https://github.com/dmapr/HiltDataBindingSample, look in app/build.gradle

@bcorso
Copy link

bcorso commented May 13, 2021

Thanks for the sample app @dmapr!

TL;DR:

Using correctErrorTypes true gives the correct error message.

In particular, Hilt does not allow extending the interface when using @DefineComponent, so you should not extend CustomBindingComponent with DataBindingComponent.

Details

When you use correctErrorTypes true, you get the correct error message:

HiltDataBindingSample-master/app/build/.../CustomBindingComponent.java:10: error: [Hilt]
public abstract interface CustomBindingComponent extends DataBindingComponent {
                ^
  @DefineComponent com.example.hiltdatabindingdemo.di.CustomBindingComponent, cannot extend a super class or interface. Found: DataBindingComponent

When using correctErrorTypes = false, the reason it seems to "work" is because Kapt will silently remove the DataBindingComponent for you because it's an error type (since it hasn't been generated yet). This is what the stub file looks like with correctErrorTypes = false:

@dagger.hilt.DefineComponent(parent = dagger.hilt.components.SingletonComponent.class)
@BindingScoped()
@kotlin.Metadata(mv = {1, 4, 2}, bv = {1, 0, 3}, k = 1, d1 = {"\u0000\n\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\bg\u0018\u00002\u00020\u0001\u00a8\u0006\u0002"}, d2 = {"Lcom/example/hiltdatabindingdemo/di/CustomBindingComponent;", "", "app_debug"})
public abstract interface CustomBindingComponent {}

Thus, in Dagger's processing we won't see the super type at all (it may be present in the Kotlin @Metadata but doing that check everywhere is expensive and error-prone). In general, I think you just always want to use correctErrorTypes = true.

(There's also a separate issue here that when you do use correctErrorTypes true you get an additional error because DataBindingComponent has not been generated yet. We could probably fix this on our side by delaying processing a round to wait for DataBindingComponent to be generated, but the only real benefit in this case is that you'd remove 1 unnecessary error message.)

@dmapr
Copy link
Author

dmapr commented May 13, 2021

@bcorso thanks for the detailed explanation! The confusing (at least to me) part was that by the time I get the error (with correctErrorTypes true) the DataBindingComponent interface has already been generated.

So I guess now for the real question — what's the best way to handle DataBindingComponent with Hilt? Just use a plain Dagger component as before the migration?

@bcorso
Copy link

bcorso commented May 13, 2021

So I guess now for the real question — what's the best way to handle DataBindingComponent with Hilt? Just use a plain Dagger component as before the migration?

Rather than having the @DefineComponent extend DataBindingComponent you can have your entry point extend it, which is effectively the same thing.

@EntryPoint
@InstallIn(CustomBindingComponent::class)
interface CustomBindingEntryPoint: DataBindingComponent {
   override fun getImageBindingAdapter(): ImageBindingAdapter
}

(Note: technically, you shouldn't need the override since Dagger should see the method defined in DataBindingComponent. However, it's likely related with DataBindingComponent not being generated yet. I'll have to look further to see if there's anything we can do about it on our side).

Then, to set the data binding component you can use EntryPoints:

DataBindingUtil.setDefaultComponent(
    EntryPoints.get(bindingComponentProvider.get().build(), CustomBindingEntryPoint::class.java))

Or if you really want you can just cast:

DataBindingUtil.setDefaultComponent(bindingComponentProvider.get().build() as DataBindingComponent)

@bcorso
Copy link

bcorso commented May 13, 2021

Also, it might help to clarify why we didn't just allow extending an @DefineComponent type directly if using an entry point is effectively the same thing.

The reason is that @DefineComponent annotated types are referenced in a lot of locations (e.g. they are the input to @InstallIn annotations). Thus, we want those classes to be cheap to build and have few dependencies to avoid affecting build performance. Extending an entry point should be less expensive because fewer classes will depend on the entry point.

@dmapr
Copy link
Author

dmapr commented May 13, 2021

Thanks @bcorso! My understanding is that we need the overrides because of #665. However I have noticed that after updating to the latest (Android Studio/AGP 4.2, Dagger 2.35.1) even declaring the custom binding component in Java no longer helps. I didn't have time to try and pinpoint whether it was AS/AGP or the latest Dagger that broke it, although I suspect the former as it went from Java 8 to Java 11.

@dmapr
Copy link
Author

dmapr commented May 13, 2021

@bcorso I tried removing the individual method overrides from the CustomBindingEntryPoint and I didn't run into any manifestations of the #665 mentioned above, even when running ./gradlew clean assembleDebug --no-build-cache. Score one for Hilt, I guess :)

@EntryPoint
@BindingScoped
@InstallIn(CustomBindingComponent::class)
interface CustomBindingEntryPoint: DataBindingComponent

@bcorso
Copy link

bcorso commented May 13, 2021

Thanks! The override issue does indeed look like #665. So just to wrap things up, the main takeaways are:

  1. Use correctErrorTypes = true
  2. Extend an entry point with DataBindingComponent instead of extending the @DefineComponent type directly (Example: correctErrorTypes causes DataBindingComponent compile errors #2603 (comment)).
  3. There's an issue of needing to override the methods in DataBindingComponent, but this is covered by Dagger doesn't implement component method if the method is used only in generated code #665.

If that all sounds correct, then I think we can close this issue now since there's nothing for us to do outside of #665.

@dmapr
Copy link
Author

dmapr commented May 13, 2021

@bcorso — yep, I think all the questions were answered. Closing. Thanks again!

@dmapr dmapr closed this as completed May 13, 2021
nuhkoca added a commit to nuhkoca/HiltDataBindingSample that referenced this issue May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants