rickclephas / KMP-NativeCoroutines

Library to use Kotlin Coroutines from Swift code in KMP apps
MIT License
1.03k stars 31 forks source link

1.0.0-ALPHA-4 doesn't expose types properly for generic abstract classes #90

Open tfcporciuncula opened 1 year ago

tfcporciuncula commented 1 year ago

I have a minimum reproducible example here: https://github.com/tfcporciuncula/native-coroutines-kotlin-18

Let's say we have something like this:

abstract class AbstractClass<T>(private val initial: T) {
  val channel = Channel<T>(Channel.CONFLATED)
  @NativeCoroutines val testFlow: Flow<T> = channel.receiveAsFlow()

  val _testStateFlow = MutableStateFlow(initial)
  @NativeCoroutines val testStateFlow: StateFlow<T> = _testStateFlow.asStateFlow()

  @NativeCoroutines suspend fun testSuspend(): T {
    delay(1000)
    return initial
  }
}

What I get from KMP-NativeCoroutines (Swift 5 Interface counterpart) is this:

extension AbstractClass {

    open func testSuspend() -> (@escaping (Any?, KotlinUnit) -> KotlinUnit, @escaping (Error, KotlinUnit) -> KotlinUnit, @escaping (Error, KotlinUnit) -> KotlinUnit) -> () -> KotlinUnit

    open var testFlow: (@escaping (Any?, @escaping () -> KotlinUnit, KotlinUnit) -> KotlinUnit, @escaping (Error?, KotlinUnit) -> KotlinUnit, @escaping (Error, KotlinUnit) -> KotlinUnit) -> () -> KotlinUnit { get }

    open var testStateFlow: (@escaping (Any?, @escaping () -> KotlinUnit, KotlinUnit) -> KotlinUnit, @escaping (Error?, KotlinUnit) -> KotlinUnit, @escaping (Error, KotlinUnit) -> KotlinUnit) -> () -> KotlinUnit { get }

    open var testStateFlowValue: Any? { get }
}

The generic T becomes Any?. I have a branch on Kotlin 1.7.20 and KMP-NativeCoroutines 0.13.1 so we can easily compare what we would get before, which is this:

open class AbstractClass<T> : KotlinBase where T : AnyObject {

    public init(initial: T?)

    /**
     * @note This method converts instances of CancellationException to errors.
     * Other uncaught Kotlin exceptions are fatal.
    */
    open func testSuspend(completionHandler: @escaping (T?, Error?) -> Void)

    /**
     * @note This method converts instances of CancellationException to errors.
     * Other uncaught Kotlin exceptions are fatal.
    */
    open func testSuspend() async throws -> T?

    open func testSuspendNative() -> (@escaping (T?, KotlinUnit) -> KotlinUnit, @escaping (Error, KotlinUnit) -> KotlinUnit) -> () -> KotlinUnit

    open var _testStateFlow: Kotlinx_coroutines_coreMutableStateFlow { get }

    open var _testStateFlowNative: (@escaping (T?, KotlinUnit) -> KotlinUnit, @escaping (Error?, KotlinUnit) -> KotlinUnit) -> () -> KotlinUnit { get }

    open var _testStateFlowNativeValue: T? { get }

    open var channel: Kotlinx_coroutines_coreChannel { get }

    open var testFlow: Kotlinx_coroutines_coreFlow { get }

    open var testFlowNative: (@escaping (T?, KotlinUnit) -> KotlinUnit, @escaping (Error?, KotlinUnit) -> KotlinUnit) -> () -> KotlinUnit { get }

    open var testStateFlow: Kotlinx_coroutines_coreStateFlow { get }

    open var testStateFlowNative: (@escaping (T?, KotlinUnit) -> KotlinUnit, @escaping (Error?, KotlinUnit) -> KotlinUnit) -> () -> KotlinUnit { get }

    open var testStateFlowNativeValue: T? { get }
}

Here we can see T is there so we didn't lose anything. Is there anything I might be missing on my end or is that potentially a regression on the latest version?

KwabenBerko commented 1 year ago

Created a similar ticket in the KMMViewModel project: https://github.com/rickclephas/KMM-ViewModel/issues/15

rickclephas commented 1 year ago

@tfcporciuncula this is an unfortunate side effect of the new code generation approach (with KSP). v1.0 generated extension properties/functions. Which in case of a generic class results in a generic function/property which isn't supported in Objective-C.

I am going to take a look at this to see if there is a way we can improve this 😄.

harry248 commented 8 months ago

@tfcporciuncula Did you find a workaround? @rickclephas What do you recommend to tackle this issue?

tfcporciuncula commented 8 months ago

Nope, I don't think there's a workaround. We didn't have a lot of cases where we were exposing flows, so we just minimized them even more and handled them ourselves without a library. You could also consider SKIE as I believe it doesn't have this limitation, but I haven't verified it myself.

rickclephas commented 8 months ago

@harry248 unfortunately there isn't really a proper solution. However there are some ways to workaround this limitation:

and as @tfcporciuncula mentioned SKIE doesn't have this limitation since it doesn't use extensions.