rickclephas / KMP-NativeCoroutines

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

Transform createdPublisher into SwiftUI's @Published #32

Closed ln-12 closed 1 year ago

ln-12 commented 2 years ago

I am wondering if and how I could transform a AnyPublisher to a @Published property. Let's say I have a property in my swift view model like @Published var isNetworkConnected: Bool which is defined as val isNetworkConnected: MutableStateFlow<Boolean> in my shared code repository. I now would like to do something like:

self.isNetworkConnected = createPublisher(for: self.repository.isNetworkConnectedNative)

Or even better, I would like to do something like this:

Text(appViewModel.repository.connectivityStatus.isNetworkConnectedNative)

Of course the variable should update automatically and take care of cancelling the publisher when the view is deleted by the SwiftUI framework. Is that somehow possible or could be achieved in the future? It would be a massive simplification when using KMM with SwiftUI as the biolerplate code in the view model would not be needed.

rickclephas commented 2 years ago

I have been thinking about this a little and the short conclusion is that IMO this is more of a Combine - SwiftUI integration challenge than a Coroutines - Combine one.

I think the biggest integration challenge between Coroutines and Combine is how cancellations work. Coroutines uses the CancellationException which means that all suspend function are throwing functions in ObjC/Swift world. For example the Clock sample has a StateFlow that won't throw an exception: https://github.com/rickclephas/KMP-NativeCoroutines/blob/73d96e6f491709921c765d189a257fabc7dd868c/sample/shared/src/commonMain/kotlin/com/rickclephas/kmp/nativecoroutines/sample/Clock.kt#L15-L16

However when you collect such a StateFlow the collect functions can/will throw a CancellationException:

val job = GlobalScope.launch {
    Clock.time.collect() 
}
job.cancel() // This will make the collect function throw a CancellationException

This in turn means that (for now) all Combine publishers have an error type. Meaning the Combine publisher for time is an AnyPublisher<KotlinLong, Error> where in Combine world it should probably be an AnyPublisher<KotlinLong, Never>.

Such an error can either be replaced with replaceError(with:) or removed with assertNoFailure(_:file:line:). If there is no way that the collection could be cancelled from the Kotlin side than using assertNoFailure shouldn't be an issue I think. Cancelling the Combine publisher will prevent it from receiving further values including the CancellationException.

But to comeback to the SwiftUI part, once you have a Combine publisher without errors you still can't use it directly from SwiftUI (as far as I know). You would still have to use the assign(to:) function to assign the values to a @Published property. Meaning you would have the same challenge even if your publisher was created in Swift itself.

This is however a very interesting topic so if you have any more information on how this would work in plain Swift (if we just forgot about Kotlin for a sec) than maybe we can find a solution that would make the Kotlin publishers work in a similar way 😄 .

ln-12 commented 2 years ago

Thanks for your detailed reply. I had the same thoughts regarding You would still have to use the assign(to:) function to assign the values to a @Published property. But that part could be solved by generating the code automatically. If you have time and generate timeNative on the Kotlin side, you could also generate timePublisher on the Swift side. But I agree that a "direct" mapping would be preferable. If I find out more, I will comment here again.

rickclephas commented 2 years ago

I am going to close this for now, as I don't think there is much that can be done in KMP-NativeCoroutines. Feel free to leave a comment if you have any thoughts about how the usage of Kotlin Coroutines from SwiftUI could be improved 😄.

ln-12 commented 2 years ago

To give you a short update on this topic, I want to point out this comment from moko-mvvm. Using the mentioned extensions (especially this one), I can directly consume the CStateFlow in my views which (as far as my testing goes) works perfectly fine.

rickclephas commented 2 years ago

Thanks for the update! Interesting approach. One thing I noticed is that this relies on the ViewModel being recreated with the SwiftUI view. So depending on the logic in your ViewModel that might not be desired. Anyway like I said the biggest challenge is in the SwiftUI part, not necessarily with Kotlin Flows.

ln-12 commented 2 years ago

You do not necessarily have to bind the view model's lifecycle to the view. A different approach (that I am using) is to create a @StateObject of the vm at the root of your view hierarchy and pass it down as @EnvironmentObject.

Actually, the linked approach is the SwiftUI part. You can simply consume it in your view like this and the state is handled automatically from the CStateFlow (I just discovered this possibility, so this might not be the ideal usage):

struct LoginScreen: View {
    @EnvironmentObject var appViewModel: AppViewModel

    var body: some View {
        let loginState: ViewModelLoginState = appViewModel.state(\.loginState)

        Text(loginState.someMessage)
    }
}

In this example, appViewModel was created before and appViewModel.loginState is a CStateFlow of type ViewModelLoginState in that vm. In Kotlin it can be used just as a regular StateFlow. You can find a ready to use implementation here.

rickclephas commented 2 years ago

Ah you are calling the state function from the view directly. Interesting.

ln-12 commented 2 years ago

Ok, so after some more testing, I figured out that consuming the flows using your approach and mapping them to @Published vars in my viewmodel is more reliable (and improves autocompletion in XCode). Most of the time, calling .state() from the view works. However, sometimes the state change does not update the view until for example some user interaction which forces the view to be recreated.

Maybe I am missing something here, but for now I will stick to using your library and mapping my flows manually to @Published vars which seems way less hacky than getting the state inside the view body.

ln-12 commented 2 years ago

I now have a good enough solution which I just want to share here in case anyone is interested in it later on. I am now using this extension in Swift:

import SwiftUI
import KMPNativeCoroutinesCore
import KMPNativeCoroutinesAsync

extension View {
    func map<Output, Failure: Error, Unit>(from nativeFlow: @escaping NativeFlow<Output, Failure, Unit>, to binding: Binding<Output>) -> some View {
        return self.task {
            do {
                let stream = asyncStream(for: nativeFlow)
                for try await value in stream {
                    await MainActor.run {
                        binding.wrappedValue = value
                    }
                }
            } catch {
                print("Failed to collect flow value with error: \(error)")
            }
        }
    }
}

In any SwiftUI view, I can then use it like this:

struct CustomScreen: View {
    @EnvironmentObject var appViewModel: AppViewModel // passed down from the root view

    @State var someState: AppViewModel.SomeState = AppViewModel.SomeStateLoading() // local view state

    var body: some View {
        VStack {
            Text(someState.something)
        }
        .map(from: appViewModel.someStateNative, to: $someState) // map the flow in the VM to local state
   }
}

In my Kotlin view model, I use combine to merge multiple state flows into a screen state:

sealed class SomeState {
    object Loading: SomeState()
    data class Success(val something: String): SomeState()
    data class Error(val message: String): SomeState()
}

var someState = combine(
    something,
    isLoading
) { something, isLoading ->
    if(isLoading) {
        SomeState.Loading
    } else if (something == null) {
        SomeState.Error("Some error")
    } else {
        SomeState.Success(something)
    }
}.asStateFlow(SomeState.Loading)

Additionally, I have this extension to generate the public properties which are transformed to native ones by your library:

fun <T> Flow<T>.asStateFlow(initialValue: T) = this.stateIn(
    scope = viewModelScope,
    started = SharingStarted.WhileSubscribed(),
    initialValue = initialValue
)

Using task from SwiftUI in combination with SharingStarted.WhileSubscribed() from Kotlin/Coroutines, the flow is automatically collected when the view appears and cancelled when the view is not in the hierarchy anymore so that I get the full benefit of stopping flow updates when no view is subscribed to the state (this was not the case with my former approach of having all properties duplicated in my SwiftUI view model with flow collectors running all the time).

I would still prefer to have something like the collectAsState() extension from Android on iOS to map the flow directly to a @State, but with this approach I only have one line of boilerplate code per state which is acceptable in my opinion.

And thank you again for your input/thoughts! :)

rickclephas commented 2 years ago

Nice! You could even set the initial value to the current value from the StateFlow:

@State var someState: AppViewModel.SomeState = appViewModel.someStateNativeValue

would still prefer to have something like the collectAsState() extension from Android on iOS to map the flow directly to a @State

That would be really awesome! Will think about some ways we can achieve that 🙂.

ln-12 commented 2 years ago

You could even set the initial value to the current value from the StateFlow

I also thought of that, but it is not allowed to use the view model reference to initialize another property:

Cannot use instance member 'appViewModel' within property initializer; property initializers run before 'self' is available

You could of course use a dedicated initializer, but that obviously adds more boiler plate code (and comes down to personal preference):

init() {
    self.state = appViewModel.someStateNativeValue
}
rickclephas commented 2 years ago

Ah that's disappointing and that will make something like collectAsState more difficult.

rickclephas commented 2 years ago

I have been playing with this a little and came up with the StateFlow property wrapper. It can be used to consume a Flow (or Publisher) in a SwiftUI view.

In an ideal scenario it can be as short as:

@StateFlow var time: String

init(clock: Clock) {
    _time = StateFlow(clock.timeNative, initialValue: clock.timeNativeValue)
}

It's a shame we can't reference self in a property initializer. That basically makes a oneliner impossible.

The following is an example that consumes the current time (KotlinLong) and transforms it to a string:

https://github.com/rickclephas/KMP-NativeCoroutines/blob/85b737a49282d3fff971e9d9f18eb880708b7388/sample/SwiftUI/StateFlowView.swift#L22-L35

ln-12 commented 2 years ago

That looks also quite interesting! I think that's as close as we can currently get to minimal boilerplate code. For me, both options we found are totally fine so I would consider this issue are completed for now. Thank you again for your help and ideas :)

rickclephas commented 1 year ago

While the @StateFlow property wrapper works, it isn't as good as I would like (which is why I didn't release it). I have been working on a way to use Kotlin ViewModels in SwiftUI, which I think is a better and cleaner approach.

Please have a look at KMM-ViewModel instead.

Combining KMP-NativeCoroutines, KMM-ViewModel and Kotlin 1.8 will allow for pretty seamless usage of Kotlin ViewModels in SwiftUI.

ln-12 commented 1 year ago

I already tested your KMM viewmodel library in my project. It works without any issues so far and makes shared viewmodels extremely simple to implement. As far as I can see, the only thing missing to make it perfect is a custom viewmodel factory to instantiate it using val viewModel: MyViewModel by viewModel() in compose.

And I would mention in the readme that you have to use different imports to use stateIn and MutableStateFlow.

Awesome work! :)

rickclephas commented 1 year ago

Nice, thanks for the feedback! Will take a look at that. Would you mind creating an issue for the ViewModel factory? KMMViewModel inherits the Jetpack ViewModel class, so it should work out of the box.

ln-12 commented 1 year ago

Nevermind, I found that the issue was to use the by delegate. When using val viewModel: MyViewModel = viewModel() it works just fine.