Metro Migration and Contributes Subcomponent

[mattinger]

Description

I am evaluating the LOE it would take us to migrate from the ksp anvil fork to metro.

Let me start by saying that we have a huge project, and we're looking to minimize the blast radius on changes if we can. I'd like to make as many changes upfront as we can to the existing ksp anvil config and use compatibility mode to start using metro, and cleanup everything over time so we can turn that off.

My hopes had been to do a bunch of prep-work up front and avoid a PR with 100+ files changed. I'm just looking for creative ways to have a PR that doesn't trigger a mess of congitive overload on my team.

And i do realize the docs recommend migrating to @DependencyGraph if you use subcomponents.

All that said, i was trying to do some of that upfront work, and I encountered some issues with subcomponents and how they contribute their factories to the app component

I'm not sure if i'm doing something wrong, or if this is just a case where I have to bite the bullet on moving to DependencyGraph and having one big PR for all that migration (what we're trying to avoid).

Whatever the answer is, I'm just looking for some clarity on if I'm doing something wrong or not.

Self-contained Reproducer

app module component:

@MergeComponent(AppScope::class)
interface AppComponent {
    @MergeComponent.Factory
    interface Factory {
        fun create(@BindsInstance application: Application): AppComponent
    }
}

lib module contributed subcomponent:

@Retention(AnnotationRetention.RUNTIME)
annotation class LibScope

@ContributesSubcomponent(
    parentScope = AppScope::class,
    scope = LibScope::class
)
interface LibSubcomponent {
    @ContributesSubcomponent.Factory
    interface Factory {
        fun create(): LibSubcomponent
    }

    @ContributesTo(AppScope::class)
    interface FactoryCreator {
        fun libSubcomponentFactory(): Factory
    }
}

The resulting compile error

[Metro/MissingBinding] Cannot find an @Inject constructor or @Provides-annotated function/property for: com.metro.migration.lib.di.LibSubcomponent.Factory

    com.metro.migration.lib.di.LibSubcomponent.Factory is requested at
        [com.metro.migration.di.AppComponent] com.metro.migration.di.AppComponent#libSubcomponentFactory()

I can move the @ContributesTo on the subcomponent to the Factory interface, and that works, but it doesn't work with ksp anvil, so it's not a change i can make up front. At that point, i might as well just go full bore on using DependencyGraph instead.

    @ContributesSubcomponent.Factory
    @ContributesTo(AppScope::class)
    interface Factory {
        fun create(): LibSubcomponent
    }

Metro version

0.6.3

Context

No response

[ZacSweers]

I can move the @ContributesTo on the subcomponent to the Factory interface, and that works, but it doesn't work with ksp anvil

Correct, these work differently across the two libraries and a dual implementation isn't likely going to work.

[mattinger]

OK. So it seems unlikely that i'll be able to do a phased migration with stuff like this.

I'm trying to encourage us to get rid of our subcomponents anyway. We're a very large dev team, and subcomponents add a bunch of learning curve for devs that don't understand the DI system very well.

[mattinger]

I've noticed some other things as well, which i'll note in this discussion, just in case they are not intended behaviors:

It seems like @Component.Builder is not supported when includeDagger() is part of the plugin configuration. A simple component like this:

@Component
interface RawComponent {
    @Component.Builder
    interface Builder {
        fun application(@BindsInstance application: Application): Builder
        fun build(): RawComponent
    }
}

Csnnot be used with createGraphFactory, and yields this error:

`createGraphFactory` type argument 'com.metro.migration.di.RawComponent.Builder' must be annotated with a `@DependencyGraph.Factory` annotation.

I assume this is intentional, and it makes sense. Just wanted to mention it here.

I also tried removing the includeDagger and adding the dagger compiler to ksp to let it handle this stuff, but when i did that, the @BindsInstance type parameters on my anvil based component would generate an error:

MergeComponent.Factory abstract function parameters must be annotated with exactly one @Includes or @Provides.

So it seems like you kind of need to be an all or nothing in a give component hierarchy with metro. YOu either need to be all metro, or all dagger. Just an observation.

[ZacSweers]

Well, yes :). I'm not sure how metro interop could support builder annotations given that metro doesn't support builders. You're welcome to file a FR.

And yes, you aren't going to be able to mix and match running both code generators over the same source set because they may generate conflicting declarations or enforce different restrictions

[mattinger]

This is pretty much what i thought. Thanks for the confirmation. I'm preparing ahead of time at least by trying to remove any @Component.Builder occurrences. It's just tedious work, and it also involves removing daggermock (which i've been prodding the dev teams to do for a long time) in favor of proper test components.