rickclephas / KMP-ObservableViewModel

Library to use AndroidX/Kotlin ViewModels with SwiftUI
MIT License
569 stars 29 forks source link

onReceive binding must conform to publisher #74

Closed itstheceo closed 2 months ago

itstheceo commented 2 months ago

I am migrating to a new version, seeing if can remove some of my prior glue code along the way. I am trying to use onReceive however getting the following error about conforming to Publisher. Previously I handled this by having a wrapper class which contained @Published var and used the createPublisher(...) convenience methods however I was hoping to get rid of this as it always felt hacky. Is there a better way of doing things? Inside onReceive I would check the state (sealed interface) and map into native SwiftUI properties accordingly.

Using @ObservedViewModel var viewModel = SessionViewModel() in my SwiftUI view.

Screenshot 2024-06-10 at 10 50 38

The shared view model is setup like this:

class SessionViewModel() : ViewModel() {

    private var _effect = MutableStateFlow<Effect>(viewModelScope, Effect.Initial)
    private var _state = MutableStateFlow<State>(viewModelScope, State.Initial)

    @NativeCoroutinesState
    val state: StateFlow<State> = _state.asStateFlow()

    @NativeCoroutinesState
    val effect: StateFlow<Effect> = _effect.asStateFlow()

   ...
}

Hoping it's something simple I've just missed. Cheers!

rickclephas commented 2 months ago

Depending on your project requirements there are a couple of ways to solve this. I guess the easiest would be to use onChange instead of onReceive.

Btw instead of creating a wrapper class you can also subclass the viewmodel in Swift.

itstheceo commented 2 months ago

onChange has a annoying requirement too, requiring conformance to Equatable. Tricky because using sealed interfaces for the State, I can't add extensions to them in Swift; Extension of protocol 'SessionViewModelState' cannot have an inheritance clause. Same for the base State they inherit from.

Screenshot 2024-06-11 at 07 11 29

Subclassing in Swift is an interesting idea; here's my wrapper class for reference, it works for all my view models as the state / effect inherit from a base interface from an abstract viewmodel class.

class KMPObservableViewModelPublisher<T: BaseViewModel>: ObservableObject {

  @Published var state: BaseViewModelState?
  @Published var effect: BaseViewModelEffect?

  private var viewModel: T?

  init(viewModel: T?) {
    guard let viewModel = viewModel else {
      return // swiftUI previews
    }
    self.viewModel = viewModel

    createPublisher(for: viewModel.stateFlow)
      .assertNoFailure()
      .assign(to: &$state)

    createPublisher(for: viewModel.effectFlow)
      .assertNoFailure()
      .assign(to: &$effect)
  }

  public func dispatch(action: BaseViewModelAction) {
    viewModel?.dispatch(action: action)
  }
}

Then for usage, it acts as a wrapper and exposes the published values straight from the Kotlin types.

@ObservedObject var viewModel = KMPObservableViewModelPublisher(viewModel: SessionViewModel())

Example Kotlin types for this case, which is a bit convoluted but other view models can be more complex.

sealed interface State: BaseViewModel.State {
        data class Loaded(val session: Session) : State
        data object Initial : State
        data object Loading : State
    }

Where BaseViewModel.State is just an empty interface interface State on an abstract superclass.

It's got me thinking maybe there's a better approach for modelling UI state for kotlin multiplatform projects 🤔 I'll have a think but, will close this as no issue with the library just nice to have another set of eyes, thank you.

rickclephas commented 2 months ago

A couple of notes:

Currently the library works best if the UI state is stored in the StateFlow properies directly. But I would love to know more about other usecase to see if/how those can be supported as well.

Would you use the onReceive approach in a pure Swift project as well? Or is this approach a result of having the viewmodel in Kotlin shared code?

itstheceo commented 2 months ago

The onReceive stuff, and wrapper class is purely for mapping viewModel changes into native SwiftUI properties. On the Android side, the Kotlin viewmodels are consumed directly in composable functions. Is there much in the way of lifecycle management on the iOS side?

This viewModel is used for acquiring a session. A root view observes the state, handling swapping between sign in screen and the rest of the app when authenticated. The views on both Android and iOS are mostly only changing based on loading, or displaying an alert based on any emitted effects; errors during form validation, or from the network response.

I'll try to simplify things and do away with the sealed classes all together; having the viewModel expose a StateFlow for loading, and a StateFlow for error information, doing away with any wrappers / publishers.

Edit: I am actively moving a lot of the code into multiplatform compose, however the sign in views are highly custom using native shaders for subtle background and blur effects. I anticipate they will remain native.

rickclephas commented 2 months ago

Is there much in the way of lifecycle management on the iOS side?

Not really. It's mainly making sure the viewmodel is cleared/the viewmodelscope is cancelled

Edit: I am actively moving a lot of the code into multiplatform compose, however the sign in views are highly custom using native shaders for subtle background and blur effects. I anticipate they will remain native.

That's also a very nice use case. Let me know how that goes!

itstheceo commented 2 months ago

Sorry for the delayed response. I don't often get to work on my project but I have completed as we discussed, removing the wrapper class from the picture. Things feel a lot nicer!

I did wonder about the use case for a SwiftUI Binding into a child view however, maybe you have some thoughts? Currently I use an onChange to map the loading flow into the @State property which then gets bound to the @Binding in the child view. It would be cool if there was an easy way to just map the view model's loading flow directly to the binding.

Here are some very watered down snippets, of the view model and view respectively:

class SessionViewModel : ViewModel(), KoinComponent {
  ...
  @NativeCoroutinesState
  val error: StateFlow<Error?> = _error.asStateFlow()

  @NativeCoroutinesState
  val loading: StateFlow<Boolean> = _loading.asStateFlow()

  @NativeCoroutinesState
  var session: StateFlow<Session?> = _session.asStateFlow()
  ...
}

And:

struct SignInView: View {
  @StateViewModel var viewModel = ViewModels().getSessionViewModel() // handles for koin singleton

  @State private var alertMessage = ""
  @State private var isLoading = false
  @State private var isShowingAlert = false
  @State private var username = ""

  var body: some View {
    VStack {
      SignInTextField(
          value: $username,
          isDisabled: $isLoading, // here it would be cool, to directly bind the viewModel.loading Flow
          placeholder: "Username"
        )
      ...
    }
      .onChange(of: viewModel.loading, { oldValue, newValue in
        isLoading = newValue
      })
      .onChange(of: viewModel.error, { oldValue, newValue in
      guard let alert = newValue else {
        return
      }
      alertMessage = alert.message
      isShowingAlert = true
    })
    ...
  }
}

struct SignInTextField: View {
  @Binding var value: String
  @Binding var isDisabled: Bool
  ...
}

Edit: added some screenshots of the errors when naively trying it:

Screenshot 2024-06-17 at 20 52 48 Screenshot 2024-06-17 at 20 53 16 Screenshot 2024-06-17 at 20 54 51
rickclephas commented 2 months ago

I did wonder about the use case for a SwiftUI Binding into a child view however, maybe you have some thoughts?

This can be achieved with $viewModel.loading. The error message you are seeing is correct, only mutable properties can be used as bindings.

Currently I use an onChange to map the loading flow into the @State property which then gets bound to the @Binding in the child view.

In that case the loading from the ViewModel isn't really the same as isLoading. isLoading can be updated by the view itself (through the binding). Such changes won't be reflected in the ViewModel.

Changing your StateFlow to a MutableStateFlow solves that and will allow you to use the property directly:

@NativeCoroutinesState
val loading: MutableStateFlow<Boolean> = _loading
SignInTextField(
  value: $username,
  isDisabled: $viewModel.loading,
  placeholder: "Username"
)
itstheceo commented 2 months ago

oh that's cool! 🚀 thanks!