rickclephas / KMP-ObservableViewModel

Library to use AndroidX/Kotlin ViewModels with SwiftUI
MIT License
588 stars 27 forks source link

Storing ViewModel in parent-view and wrapping in child throws error on rerender of child #11

Closed JonasHiltl closed 1 year ago

JonasHiltl commented 1 year ago

When creating a ViewModel in a parent view and passing the reference to a child view which wraps it with any of the property wrappers from KMM-Viewmodel, the app crashes on a rerender of the child view with the error:

Uncaught Kotlin exception: kotlin.IllegalStateException: KMMViewModel can't be wrapped more than once

I know that the error is intentionally thrown since the stored ViewModel is already wrapped and not recreated on rerender of the child, but to me this looks like a common pattern that should be supported. Why is it currently not supported and is it technically possible?

This is my setup, where the parent handles navigation between some screens and also holds the ViewModels for each screen. I don't want to recreate the ViewModels on each recreation of the view, as I want to save the state between rerenders.

class NavigationViewModel: ObservableObject {
    @Published var destination: Destination = .home

    let mapVM = MapViewModel()

    enum Destination: Hashable {
        case home
        case map(MapViewModel)
        case profile
    }
}

struct MainNavigationView: View {
    @ObservedObject var viewModel: MainNavigationViewModel

    var body: some View {
        VStack {
            Group {
                // this is from the swiftui-navigation library, it just shows the view for the current `destination`
                IfCaseLet($viewModel.destination, pattern: /MainNavigationViewModel.Destination.home) { $vm in
                    HomeView()
                }

                IfCaseLet($viewModel.destination, pattern: /MainNavigationViewModel.Destination.map) { $vm in
                    MapView(
                        viewModel: vm
                    )
                }

                IfCaseLet($viewModel.destination, pattern: /MainNavigationViewModel.Destination.profile) { $vm in
                    ProfileView()
                }
            }
            .frame(maxWidth: .infinity,maxHeight: .infinity)
        }
    }
}

The Subview wraps the passed ViewModel with ObservedViewModel

struct MapView: View {

    @ObservedViewModel var viewModel: MapViewModel

    var body: some View {
        Text("SubView")
    }
}
rickclephas commented 1 year ago

Why is it currently not supported and is it technically possible?

While technically possible, it would require a lot of state management which is otherwise handled by SwiftUI.

The idea is that you wrap the Kotlin view model once (which the library does automates for you) and use the projected value in Swift to forward the view model to other views.

However in this case you don't have a projected value, since the view model property doesn't have a property wrapper.

I don't want to recreate the ViewModels on each recreation of the view, as I want to save the state between rerenders.

Are you able to use StateObject/StateViewModel? That would keep the state between rerenders.

Will take a look at an easy way to move the view model wrapping to a custom property. Such that the property wrappers can skip the wrapping and prevent this error 😄.

JonasHiltl commented 1 year ago

Sorry, I used rerender a bit wrong, what I actually meant by rerender was the navigation between screens and the resulting recreation of the e.g MapView, since that calls the ObservedViewModel wrapper again.

Before I was triggering a navigation like this:

func goToMap() {
    destination = .map(.init())
}

which works fine, since the MapViewModel is always a new instance. But know I want to navigation like this, to keep the state between navigation:

let mapVM = MapViewModel()

func goToMap() {
    destination = .map(mapVM)
}

but here the same instance get's wrapped again on each navigation, which throws the error.


Below is a simpler reproducible example:

class NavigationViewModel: ObservableObject {
    let mapVM = MapViewModel()
}

struct MainNavigationView: View {
    @ObservedObject var viewModel: MainNavigationViewModel

    @State var showChild = false

    var body: some View {
        VStack {
            if (showChild) {
                MapView(viewModel: viewModel.mapVM)
            }
            Button("Toggle child") {
                showChild.toggle()
            }
        }
    }
}

struct MapView: View {
    @ObservedViewModel var viewModel: MapViewModel

    var body: some View {
        Text("SubView")
    }
}
rickclephas commented 1 year ago

v1.0.0-ALPHA-7 adds support for easy forwarding of the ViewModel to other views.

The only remaining requirement is that there should be a single source of truth for the observable ViewModel. This basically means that you either need to store the ViewModel via one of the property wrappers, or use observableViewModel(for:) to store the raw ObservableViewModel.