evant / kotlin-inject

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

Expect Actual Ancestor Components and Common Descendant Components #361

Closed eygraber closed 2 months ago

eygraber commented 4 months ago

I have a component hierarchy that has an expect component in the middle of it, and the actual implementations provide platform specific bindings. That requires that all downstream components be generated for all platforms, even if they themselves are common, because common generated code wouldn't be able to see these platform specific bindings.

Pre Kotlin 2.0 this isn't a problem in some cases because common source sets can (incorrectly) reference generated platform code. However once Kotlin 2.0 is released this won't work anymore for any of the cases.

To solve the issue, all downstream components (which is ~99% of the components in my project) will need to specify expect/actual declarations for the create functions for each platform (this project supports 5 platform targets) in each module (and I typically have 1 downstream component per module).

That's kind of painful, and I'm wondering if there's a way for kotlin-inject to detect (or be told about) this situation, and generate the expect/actual declarations for create.

I'm not sure if it could be done in the same processor, or if it would need a different processor, or if it's even possible with KSP at all (I think it would need something like Support source set-specific code generation in Multiplatform projects).

Other options that (hopefully) don't require changes in KSP are:

  1. A flag for kotlin-inject that will mark the generated create function as actual and then the user just needs to specify a common expect create function
  2. A new annotation can be introduced to use with descendant components that live in common, and the presence of that annotation will:
    • generate a common expect create function
    • mark generated platform create functions as actual
eygraber commented 4 months ago

I pushed some code to https://github.com/eygraber/kotlin-inject/tree/expect-actual-ancestor-components highlighting the issue. Running ./gradlew integration-tests:ksp-kmp:app:assemble should error with:

e: [ksp] Cannot find an @Inject constructor or provider for: me.tatarka.inject.test.Foo

Commenting out the common KSP dependency and uncommenting the JVM KSP dependency and the JS KSP dependency and rerunning the gradle command should then work. However if you use Kotlin 2.0 then it won't work anymore (because the common source set can't see the generated platform source sets anymore).

eygraber commented 4 months ago

In case you took a look at the branch already, one piece was missing that was making both scenarios fail. I pushed a fix for that, so it should be good as of now.

eygraber commented 4 months ago

To restate, the problem is that when created by a common source set, a component or any of its ancestors that have an expect type somewhere in the type hierarchy, the following needs to hold true:

  1. KSP is run against all affected target compilations (so that the actual implementations of the components is used)
  2. KSP is not run against the common target compilation (because of the redeclaration issue)
  3. There is an expect create function in common
  4. There is an actual create function for each affected target

The goal is to have actual create functions generated so that the user doesn't have to manually create source sets and write the actual create functions for all affected targets.

I've been trying out a few different ways to model this so that there needs to be little to no action taken by the user, but I don't think that's (currently) feasible:

  1. There's no reliable way to detect if a component class is expect or actual because KSP relies on the keyword being present in source, so if the component is coming from a different module/library that information isn't present
  2. There's no way to determine which compilation is being processed (e.g. there's no way to tell if the generated code will be for the common source set, or a platform source set)
  3. There's no way to generate code for a source set other than the one that is being processed

So going back to the proposed solutions from the OP:

  1. A flag for kotlin-inject that will mark the generated create function as actual and then the user just needs to specify a common expect create function
  2. A new annotation can be introduced to use with descendant components that live in common, and the presence of that annotation will:
    • generate a common expect create function
    • mark generated platform create functions as actual

Given the constraints, 1 would be the best solution. It would be nice to pair that with 2 so that the user doesn't have to do anything at all (i.e. write the common expect create function) but that would have to use a separate processor that is only applied to the common compilation, and I think it's easier for users to write the common expect create function that going through that whole setup.

One issue with 1 would be that if the user is running KSP against the common compilation, there will be actual create functions generated in the common source set, which doesn't make any sense, and would probably lead to unexpected errors. I would like to provide a better error in those cases, but I don't think that's possible considering KSP's constraints. However, in this scenario running KSP against the common compilation is technically an error anyways. Also this behavior would be opt-in through the Option so the documentation for that can call this out.

@evant I can put up a PR implementing solution 1 if that's something you're OK with.

eygraber commented 4 months ago

I put up a PR to see what it would look like.

eygraber commented 3 months ago

To close the circle, there could be another processor added to kotlin-inject that generates an expect create fun in commonMain based on Component annotations (kind of like what I did here). It would have to be added to KSP's commonMain configuration, and together with the Option to generate actual functions, the whole issue should be resolved.