evant / kotlin-inject

Dependency injection lib for kotlin
Apache License 2.0
1.14k stars 51 forks source link

Prepare for K2 #349

Closed eygraber closed 2 weeks ago

eygraber commented 4 months ago

Was investigating using KSP 2 and figured I'd start tracking this separate from that.

eygraber commented 4 months ago

The integration tests got kind of screwy.

The issue is that currently KSP is configured to generate code for each platform. It is also configured to generate code for commonMain, however there is no code in commonMain because common-companion is a commonTest source set, and so there is no common code generated.

In K2, common source sets can't see code from platform source sets, so the tests in common and common-companion don't see the generated platform code.

It's expected that common code cannot reference generated code in the compilation of platform code. Generated codes are treated as platform code and K2 explicitly disallow references from common to platform (you'll have to use expect/actual).

To get around that I separated common and common-companion into the common code from them which becomes a commonMain source set, and their tests, which remain a commonTest source set.

Because it's using commonTest I believe tests still run for each platform, it's just that the generated code will be in the commonMain source set.

The only other solution that would work would be to have code and tests duplicated for each platform. You can't make multiple source sets out of the same directory, so there would probably have to be a script that generates them at runtime, or do some kind of trickery with symlinks.

evant commented 4 months ago

The integration test setup is known to be kludge, it's only set up that way because I couldn't find another way to make it work.

Because it's using commonTest I believe tests still run for each platform, it's just that the generated code will be in the commonMain source set.

👍 That feels right to me and closer to the ideal way I wanted it set up.

eygraber commented 4 months ago

@evant are you OK with extracting the integration test changes and merging into main now? It should work fine on current main and would prevent conflicts as new tests are added.

evant commented 4 months ago

I was just going to ask to do that, these changes are quite large and it would be easier to review if the integration test changes were pulled out

eygraber commented 4 months ago

https://github.com/evant/kotlin-inject/pull/355

eygraber commented 1 month ago

@evant this should be good to go, other than the question of whether the library should impose Kotlin 2.0 on consumers.

evant commented 1 month ago

I think I want to get a release out with the current changes before merging this but happy to do right after

CoreFloDev commented 4 weeks ago

And so that's blocking the current release?

evant commented 3 weeks ago

Released, so this can be moved out of draft