Adds ViewModel support to the Arch. Comp. library #405
Conversation
dependencies.gradle
Outdated
| @@ -43,6 +43,7 @@ ext { | |||
| autodispose = "com.uber.autodispose:autodispose:$autodisposeVersion" | |||
|
|
|||
| archComopnentsLifecycle = "android.arch.lifecycle:runtime:$archComponentsVersion" | |||
| archComopnentsExtensions = "android.arch.lifecycle:extensions:$archComponentsVersion" | |||
There was a problem hiding this comment.
thanks @jshvarts I fixed the typo and removed the runtime import, as the extensions already depend on it so it's not required to explicitly add it
|
|
||
| private ViewModelStore viewModelStore = new ViewModelStore(); | ||
|
|
||
| protected ViewModelProvider viewModelProvider() { |
There was a problem hiding this comment.
new ViewModelProviders.DefaultFactory() is deprecated. Can it be dropped here?
There was a problem hiding this comment.
Thanks! I did not realize 1.1.0 deprecated the method. It was replaced with https://developer.android.com/reference/android/arch/lifecycle/ViewModelProvider.AndroidViewModelFactory.html
|
I just tried running the Arch Components View Model demo and got a crash: |
|
I submitted a change, I suspect the issue is that ViewModelDemo was not public but package-private. Surprisingly it worked for me all the time. |
|
Making |
|
How one can share the |
|
@esafirm I'd not share the same instance of a ViewModel across views (doesn't matter if they are Controllers, Activities or Fragments). You could of course have different instances of the same ViewModel class on as much views as you need. (Example: you have a ViewModel for a user profile, you can use this ViewModel on different screens where you need to display the user profile, but each one will have its own instance). The question is also frequently asked regarding sharing VM across activities: android/architecture-components-samples#29 As well, the best way to ensure that the same data is used on all instances of the same ViewModel is by having a data repository that can store any state or api data that you need to display, and the ViewModels use that data repository. |
|
|
||
| public abstract class ViewModelController extends LifecycleController { | ||
|
|
||
| private ViewModelStore viewModelStore = new ViewModelStore(); |
There was a problem hiding this comment.
I think viewModelStore.clear() should be called on Controller onDestroy
There was a problem hiding this comment.
Agree, as the docs say: If an owner of this ViewModelStore is destroyed and is not going to be recreated, then it should call clear() on this ViewModelStore, so ViewModels would be notified that they are no longer used. https://developer.android.com/reference/android/arch/lifecycle/ViewModelStore.html#clear() I'd look to add that to the ViewModelController onDestroy method.
The issue is about sharing What i'm asking is sharing a AFAIK it is even mentioned on |
|
OK, I never use child Controllers so I cannot go into details. As far as they can access the parent Controller instance, they should be able to use the same ViewModelProvider/ViewModelStore, then get the same ViewModel instance. Then I guess we need to make the viewModelProvider method public not protected. If you give it a try let me know! |
|
As I needed this for another project I made a small library: https://github.com/miquelbeltran/conductor-viewmodel which also provides the Lifecycle events to handle the LiveData observer subscription properly to unsubscribe then the view is destroyed. |
|
I'd make eg. this is currently not possible because |
|
I agree. I'll make the change! |
As suggested here: bluelinelabs/Conductor#405 (comment)
|
I've been having a play with this PR and I have to say, this appears to tick all of the boxes for AC+Conductor interop (with the above exception). Good job mate. I also was not able to reproduce the duplicated observer thing in either Fragment or Controller, it may have been fixed in AC 1.1.1 or something? |
|
Thanks! I am thinking about rebasing this and make it 1:1 to what I have in https://github.com/miquelbeltran/conductor-viewmodel I added my own LifecycleProvider. See in here: https://github.com/bluelinelabs/Conductor/pull/405/files#diff-b7e5adc69ad34fd475e7fad2af3275a7 Btw I am using the conductor-viewmodel library on a couple of production projects, it's working great :) |
* A ViewModelController that has its own ViewModelStore * Also provides a ViewModelProvider factory method * The Architecture Components library is bumped to 1.1.0 * Includes demo usage in ViewModelDemoController * Includes its own LifecycleOwner that provides Lifecycle events required for LiveData * It's located in the com.bluelines.conductor.viewmodel package * Added updated documentation on README.md * Add viewModelStore.clear in onDestroy As the docs say: ``` If an owner of this ViewModelStore is destroyed and is not going to be recreated, then it should call clear() on this ViewModelStore, so ViewModels would be notified that they are no longer used. ``` https://developer.android.com/reference/android/arch/lifecycle/ViewModelStore.html#clear() * viewModelProvider method is public so it can be called by child controlers
|
I have rebased the work, and as well moved the code to the com.bluelinelabs.conductor.viewmodel package. As said, I have added my own ViewModelLifecycleOwner because the one found in the library is not handling the required Lifecycle events correctly. It's important that |
|
Hello all, since there has been no movement for months and I have a working solution as an independent library: https://github.com/miquelbeltran/conductor-viewmodel I've decided to close this PR. If you are interested in the included functionality please feel free to use my library in conjunction with Conductor! |
|
@miquelbeltran I currently working on a 5 years old project that uses a conductor. Why should I use Conductor over fragment? Why not replace it with fragments? I have been asking myself this question, but I need an expert to tell me what they think about it. |
|
The question is: Why would you want to use fragments? |
|
@PaulWoitaschek I'm trying to follow what Google recommends and try to have an MVVM architecture, but with conductors, you don't have that. I like how you can manage the data in the view model and retrieve it in any fragment. Now, I would like to know your perspective and how you see the new trend of MVVM architecture. |
|
It's important to note that MVVM existed far before Google released the component that they call a ViewModel. It's also very important to note that the thing they call a ViewModel is not an MVVM view model. It's a thing that survives configuration changes and nothing more. You can use it as an MVC controller, an MVP presenter, or a handy place to store random state fields. The ViewModel class is very badly named and has mislead countless developers into thinking it's needed for MVVM. It's not. Its sole purpose is to wallpaper over the fact that activities and fragments recreate on configuration change. Given that Conductor Controllers do not have this issue, the class provides no value. You don't need a base class called ViewModel to have a ViewModel. You can roll your own with no real effort. What I've done in the past is create a base class with a CoroutineScope (rx works too, or livedata if you really had to) that cancels on Controller lifecycle events. That's really all you need. |
Based on the discussion in #395
This solution is also available as standalone library: https://github.com/miquelbeltran/conductor-viewmodel/tree/master which also fixes the Lifecycle events to handle the LiveData observer subscription properly.
Currently this PR depends on #383 as well, because the current implementation of the LifecycleController can't be used to observe LiveData as the LiveData observer is not being removed in onDestroyView.