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

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

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

vRallev
Copy link
Contributor

@vRallev vRallev commented Oct 26, 2024

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

@vRallev vRallev marked this pull request as ready for review October 26, 2024 02:17
@vRallev vRallev requested a review from cahaverl October 26, 2024 02:17
@chrisbanes
Copy link

chrisbanes commented Oct 28, 2024

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
Copy link
Contributor Author

vRallev commented Oct 28, 2024

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
Copy link
Contributor Author

vRallev commented Oct 28, 2024

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

@vRallev vRallev changed the title Support expect / actual for generated factory function Support expect / actual for generated factory functions using @CreateComponent Oct 28, 2024
@chrisbanes
Copy link

Nice! :shipit:


private val kclassFqName = KClass::class.requireQualifiedName()

@Suppress("ReturnCount")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this @Suppress can go away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, will remove.

// https://github.com/ZacSweers/kotlin-compile-testing/issues/298
//
// The correctness is verified through the sample app of this project for now.
class CreateComponentProcessorTest {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check the successful path? Or is there no real way to do that with expect / actual?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not right now, because the compiler will pick up all generated files and then complain about that these keywords cannot be used within the same module / source set. It's a limitation of the compile testing library right now. We should help and add support for this.

Copy link

@cahaverl cahaverl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of nits.

Base automatically changed from rwo/8 to main October 28, 2024 22:06
…ateComponent`

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
@vRallev vRallev merged commit 0a96521 into main Oct 28, 2024
7 checks passed
@vRallev vRallev deleted the rwo/expect branch October 28, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Components declared in iosMain can not find the merged interface on Kotlin 2.x
3 participants