-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ViewModel lifecycle on SwiftUI against Compose #231
Comments
After some experiences, purely managing lifecycle from the viewmodel is so wrong with SwiftUI. the After some exploration : |
I'm not really sure as I'm still a KMP noob and such. But. For the case of initialization... I know it's pretty much an anti pattern/not recommended to do stuff as part of a ViewModels init method. I have typically always used init for this sort of initialization work but I got into trouble with this a few months ago and brought it up on kotlinlang. Basically I was modifying snapshot state before the VM was actually ready and touching snapshot state in a VM init was crashing at like a 5% rate in production... Only for a specific VM though. Very weird. But now I don't use init in VM anymore. Lol Also. I think Ian Lake said that even though VM and lifecycle are now compiled for KMP, they aren't actually hooked into iOS lifecycle and someone (jet brains?) would still actually have to do that work. (I don't want to misrepresent what Ian said so take that with a grain of salt). |
It's not about only the init of the viewmodel but the lifecycle. |
I think destroying the VM ( As for the initialization it depends on the navigation logic used in SwiftUI. In terms of behaviour a "Swift approach" that doesn't perform (much) work in a VM constructor would also work for Compose/Kotlin. IMO VMs created by SwiftUI before the view is ever required is a bug, and I have worked around that even in pure Swift projects. |
If we could do that easly, it would be great. I thought, manually cancelling the
I think, we should emulate the KMP ViewModel lifecycle from SwiftUI Something like this : @StateObject var viewModel = KTViewModel<MyViewModel>(.init())
class KTViewModel<T : Lifecycle_viewmodelViewModel> : ObservableObject {
let instance: T
init(_ viewModel: T) {
self.instance = viewModel
}
deinit {
self.instance.onCleared()
}
} override fun onCleared() {
super.onCleared()
if (viewModelScope.isActive) {
println("[onCleared] Cancelling viewModelScope")
viewModelScope.cancel("Cancelling viewModelScope for iOS Target")
}
println("[onCleared] MainScreenViewModel")
} Reproducing the expected lifecycle of SwiftUI fix a lot of things, as the ViewModel is now correctly initialized/deinit, and we have the same behavior as a swift ViewModel.
|
@joreilly The main issue here : The viewmodel behavior between iOS/Android is currently different, but it should be the same to avoid bugs. On Compose, it's not an issue as the viewmodel lifecycle is properly handled, but on iOS it must be manually handled. |
Been a bit crazy here and haven't had chance to catch up with this yet (among other things :) ) . If you do have particular changes in mind then would definitely welcome a PR . |
Thanks for the reply, I have explain an solution #231 (comment) It's about wrapping the Kotlin I worked on my side to do it the cleanest way, I will do a PR. |
@frankois944 one question I have from above is related to the SKIE issue you mentioned and comment from @TadeasKriz about using |
I'm not sure tbh why I'm using |
But, is it the As the stateflow lifecycle is bound to the Just add the You can do some testing on the See my PR: #237 |
It was me, I did the update but I didn't understand the origin of the issue well. |
Ah, ok....good example of what a terrible memory I have :) |
@joreilly The new But I've been prototyping a similar thing to the new |
It would be great if there are more compatibility between Swift and the Android ViewModel as it was not originally made for working with SwiftUI. |
Yeah, it's one of the reasons why I'm not enjoying Android ViewModel myself (iOS developer alert). In my current prototyping I'm trying our Swift macros to see if that would be something we can leverage. There's always the |
Yes me too (and also SwiftUI ViewModel) 😄 For now, I will stick on my solution by wrapping the KMP/Android viewmodel and wait for a smoother way. |
Chatting to @marcellogalhardo and he had some suggestions around use of |
Needs cleanup but this is example based on @marcellogalhardo's suggestion #239. |
That's been merged now |
@joreilly Wow, long way to access at the internal I think the |
About this issue, it also applies on UIKit project, it's easier as the lifecycle of a ViewController is simpler than SwiftUI. At least, we need a common entry point which is inside But as the current project is in SwiftUI it's not the subject. |
Absolutely. There's a few known issues right now around lifecycle/scope etc. @marcellogalhardo' I believe is also actively working on how best to structure this (in general) including for example figuring out how this would work with navigation setup. |
@joreilly FantasyPremierLeague/ios/FantasyPremierLeague/FantasyPremierLeague/Fixtures/FixtureListView.swift Lines 8 to 9 in 0e9508d
You shouldn't put the ViewModel into a The usage of the Android Viewmodel is the same as a SwuiftUI ViewModel (at least of the instance) |
Thanks. This was hacked together pretty quickly and definitely needs a few updates. Tadeas had also mentioned @State issue to me as well....will update later today |
@TadeasKriz I'm also personnaly going on macro for removing the the boilerplate on init the instance and data binding @sharedViewModel(ofType: MainScreenViewModel.self,
publishing: (\.mainScreenUIState, MainScreenUIState.self), (\.userId, String?.self)
)
class MyMainScreenViewModel: ObservableObject {} Using SKIE Combine API, i can generate the code for binding ( |
You don't need to declare One very important thing to keep in mind, |
@frankois944 re.
I had done that in initial cut of the code but was concerned if issue in having to do that retrieval every time there's a recomposition of the view. |
It just like SwiftUI viewmodel, You should explore the lifecycle of the ObservableObject by printing some logs on init/deinit of the @StateObject or @State, you will see a big difference of the behavior. Edit : It's better when there are some navigation for triggering the deinit 😄 |
This is my playground, kind of dirty but I'm exploring https://github.com/frankois944/kmp-mvvm-exploration. My main goal is to be close of the usage of the SwiftUI MVVM with the Android ViewModel. |
As your sample, we want to share the viewmodel on SwiftUI and Compose.
But there is a big difference between them, it's how a viewmodel is initialised, used and destroyed.
On SwiftUI, for exemple, the struct containing the view definition is initialised even the view is not displayed, as intended on SwiftUI with navigationLink, a good exemple is a list containing for each item a navigation link to a view containing a viewmodel.
So we can't put heavy code in the viewmodel constructor because it can be useless or cause memory waste, like data preloading or listener...
It's not the case on Compose, the viewmodel is initialised when the view will be displayed.
Currently, viewmodel data are loaded when it's bound/subscribed to the view.
Because of
SharingStarted.Lazily
,We can share logic between SwiftUI and Compose, but the lifecycle is clearly different; the
constructor
and thefun onCleared()
don't have the same meaning.I found some dirty solutions that could work, but no proper solution.
Let's talk about it
The text was updated successfully, but these errors were encountered: