appyx icon indicating copy to clipboard operation
appyx copied to clipboard

ViewModel POC

Open KovalevAndrey opened this issue 2 years ago • 5 comments

Description

This PR demonstrates how VM can be used and scoped to a Node using reflection API.

By design VM is stored in a ViewModelStore class and lives as long as activity lives.

The desired behaviour is the following:

  1. VM is created when Node is created
  2. VM is persistent when Node is moved in DESTROY state and Activity is changing configuration
  3. VM is destroyed when Node is moved in DESTROY state and Activity IS NOT changing configuration

This behaviour can not be achieved using public API as it's not possible to clear only one VM from ViewModelStore. It has only one public method – ViewModelStore.clear()which clears allVMs.

I the meantime I'm exploring alternative solutions.

https://user-images.githubusercontent.com/5773436/207447633-65f79414-2313-4d7b-b453-464c4e51de30.mp4

Check list

  • [ ] I have updated CHANGELOG.md if required.
  • [ ] I have updated documentation if required.

KovalevAndrey avatar Dec 13 '22 21:12 KovalevAndrey

IMHO it should be implemented in the same way as in Fragment – each Node should be ViewModelStoreOwner with own ViewModelStore. Then throw everything into single FragmentManagerViewModel-like ViewModel that we will register once in activity integration point.

Anyway in both variants we have another problem – we change DI flow. We usually pass all dependencies into Node ready to use, but here we have to instantiate ViewModel inside Node, comparing to everything else that is done outside. Not like a bad thing, something that I want to come to later.

CherryPerry avatar Dec 16 '22 20:12 CherryPerry

IMHO it should be implemented in the same way as in Fragment – each Node should be ViewModelStoreOwner with own ViewModelStore. Then throw everything into single FragmentManagerViewModel-like ViewModel that we will register once in activity integration point.

Anyway in both variants we have another problem – we change DI flow. We usually pass all dependencies into Node ready to use, but here we have to instantiate ViewModel inside Node, comparing to everything else that is done outside. Not like a bad thing, something that I want to come to later.

This is what we're doing internally right now. I've published a sample demonstrating this

RationalRank avatar Dec 17 '22 03:12 RationalRank

IMHO it should be implemented in the same way as in Fragment – each Node should be ViewModelStoreOwner with own ViewModelStore. Then throw everything into single FragmentManagerViewModel-like ViewModel that we will register once in activity integration point.

Anyway in both variants we have another problem – we change DI flow. We usually pass all dependencies into Node ready to use, but here we have to instantiate ViewModel inside Node, comparing to everything else that is done outside. Not like a bad thing, something that I want to come to later.

yes, that's what I was gonna do as a second option

KovalevAndrey avatar Dec 17 '22 11:12 KovalevAndrey

The standard lifecycle viewModel functions for compose delegate to the composition local LocalViewModelStoreOwner. Wouldn't it just be a matter of coming up with a proper implementation of this which was aware of the navigation graph, and setting up the composition local for each node? (Yes, i know i'm making the implementation seem simpler than it probably would be)

I honestly wonder how compose navigation achieves this.

Update: I took a look and it seems like NavBackStackEntry itself implements several interfaces including the LifecycleOwner and ViewModelStoreOwner. So what i assume happens is that as each backstack entry is created, a completely new store is instantiated and registered as the local store owner. And when it's popped off, it is completely cleared. This makes a ton of sense to me obviously, and I think it could honestly be done in this library as well.

mattinger avatar Feb 21 '23 01:02 mattinger

The standard lifecycle viewModel functions for compose delegate to the composition local LocalViewModelStoreOwner. Wouldn't it just be a matter of coming up with a proper implementation of this which was aware of the navigation graph, and setting up the composition local for each node? (Yes, i know i'm making the implementation seem simpler than it probably would be)

I honestly wonder how compose navigation achieves this.

Update: I took a look and it seems like NavBackStackEntry itself implements several interfaces including the LifecycleOwner and ViewModelStoreOwner. So what i assume happens is that as each backstack entry is created, a completely new store is instantiated and registered as the local store owner. And when it's popped off, it is completely cleared. This makes a ton of sense to me obviously, and I think it could honestly be done in this library as well.

Hi @mattinger we're considering the same approach in this draft pr but the final decision whether to make it as a part of framework hasn't been made yet.

We recently introduced RetainedInstanceStore which allows you to implement ViewModel support the way you described right now

KovalevAndrey avatar Mar 02 '23 19:03 KovalevAndrey