amzn / kotlin-inject-anvil

Extensions for the kotlin-inject dependency injection framework
Apache License 2.0
274 stars 8 forks source link

Support `expect / actual` for generated factory functions using `@CreateComponent` #62

Closed vRallev closed 1 month ago

vRallev commented 1 month ago

Due to how Kotlin 2.0 and newer handles source sets, it's likely that the generated factory function for generated components cannot be referenced from common Kotlin code in KMP projects. This change adds a new annotation @MergeComponent.CreateComponent to allow defining an expect fun in common code where the actual fun will be generated. This closes the gap.

Fixes #20

chrisbanes commented 1 month ago

Nice! I don't like the ext-fun on KClass thing, but it's obviously a pattern used in kotlin-inject so I'm not against keeping it and staying consistent.

Personally I'd prefer something like:

@GenerateActualAccessors
expect fun createAppComponent(foo: String): AppComponent

...but it's not a blocker. The @GenerateActualAccessors annotation also makes this nice and explicit, rather than the magic-foo of creating an expect fun with the exact structure. I have no idea if that's possible with KSP though.

vRallev commented 1 month ago

Nice! I don't like the ext-fun on KClass thing, but it's obviously a pattern used in kotlin-inject so I'm not against keeping it and staying consistent.

Personally I'd prefer something like:

@GenerateActualAccessors
expect fun createAppComponent(foo: String): AppComponent

...but it's not a blocker. The @GenerateActualAccessors annotation also makes this nice and explicit, rather than the magic-foo of creating an expect fun with the exact structure. I have no idea if that's possible with KSP though.

After thinking about it more over the weekend, I fully agree with all your points. I'll introduce @MergeComponent.ComponentBuilder. The function may or may not have the KClass as receiver type.

I'll do this as a follow up. Any suggestions for the name? @ComponentBuilder or @ComponentCreatorFunction? I don't like @GenerateActualAccessors, but this could be a me thing. Unfortunately, it's also not possible to reuse the function.

vRallev commented 1 month ago

I updated this PR and introduced @MergeComponent.CreateComponent. I felt like this is the most consistent name with the "create" functions.

chrisbanes commented 1 month ago

Nice! :shipit: