Decouple State and Args

[airbnb]

Decouple State and Args

[BenSchwab]

@gpeal I'm not sure I see how state restoration changes the interface (or mental model). Args should be the same in the state restoration case vs. the normal case.

@Mishkun I do see that I misread the suggestion a bit. I don't think we should make the ViewModelFactory responsible for creating initialState. I was more imagining just one extra arg from before:

 class MyViewModel(private val args: MyArgs, initialState: MyState) : BaseMvRxViewModel(initialState) {
    init {
       // just use the args
       args.let { // ... }
    }
    companion object : MvRxViewModelArgFactory<MyState, MyViewModel> {
        @JvmStatic
        override fun create(activity: FragmentActivity, initialState: MyState, args: MyArgs): MyViewModel {
            return MyViewModel(args, intialState)
        }
    }
}

[BenSchwab]

Thanks for the suggestion. This seems reasonable to me. We initially believed that giving access to the activity (and in turn DI frameworks such as Dagger) would largely cover this, but I actually had an engineer at Airbnb have a similar need yesterday. Any concerns @gpeal?

[gpeal]

@Mishkun @BenSchwab The tricky part that led us to the current world is that MvRx handles process restoration so your ViewModel may be instantiated with non-initial state (restored state).

We wanted the mental model to be args -> initial state -> subsequent state

In the state restoration case, you would be given initial args + subsequent state which creates a potentially confusing world. I see the advantage of passing in args here and I'm not opposed to it, we just need to make sure that it makes sense for the view model restoration case as well.

[Mishkun]

@gpeal good note about state restoration, haven't think about that. To handle that case, we can add to our factory interface a restore method, that would give us the args and restored state so we can initialize viewmodel properly.
To reduce the boilerplate in simple cases we can also provide a default implementation that will call create and then set the restored state.

[hellohuanlin]

Sometimes args is not enough to construct the state. My preference is to create the initial state inside view model. This decouples state and args, and will give us enough flexibility.

class MyViewModel(
  private val x: X, 
  private val y: Y,  
  private val foo: Foo
): BaseMvRxViewModel<MyState>() {
  override val initialState = MyState(
    x = x, 
    y = y, 
    foo = foo
  )

  companion object : MvRxVMFactory {
    override fun create(args: MyArgs) {
        val someDaggerComponent = ...
        return MyViewModel(args.x, args.y, someDaggerComponent.foo)
    }
  }
}

Or this

class MyViewModel(
  private val args: MyArgs, 
  private val foo: Foo
): BaseMvRxViewModel<MyState>() {
  override val initialState = MyState(
    x = args.x, 
    y = args.y, 
    foo = foo
  )

  companion object : MvRxVMFactory {
    override fun create(args: MyArgs) {
        val someDaggerComponent = ...
        return MyViewModel(args, someDaggerComponent.foo)
    }
  }
}

Another example is with generic state:

data class MyState<T>(
  val x: Foo,
  val t: T
)
class MySuperViewModel<T>(
  val x: Foo, 
  val initialT: T
) {
  override val initialState = MyState<T> {
    x = x,
    t = initialT
  }
}

class MySubViewModel(
  val x: Foo
): MySuperViewModel<String>(x = x, initialT = "Hello") {}

@BenSchwab @gpeal @elihart

[Mishkun]

@hellohuanlin You can instantiate state in the factory method of a ViewModel. This would be much more cleaner approach involving real DI instead of fake one.
Real DI constructs everything needed to object A completely outside this object.
Fake DI gives A some deps that A glues together
Also you loose benefits of state management done by MvRx
With suggested approach in initial proposal you can easily move initial state creation to the ViewModelFactory and still be compliant with all the framework features

[hellohuanlin]

@Mishkun yes moving it into ViewModel feels like a better approach.

[bearprada]

I got the same question when I want to reuse the same ViewModel with a different configuration in the same activity.

[gpeal]

@bearprada you can use keyFactory for that:
by fragmentViewModel(keyFactory = { "foo" })

Every key will create a separate ViewModel.

[bearprada]

What if we have two lists on the same page(Fragment). One list shows category A's items, another list shows category B's items. We want to re-use the same ViewModel's implementation.

Right now, here is no way to inject two different Args objects in a Fragment, no?

Sorry, my question might not suitable for this thread, and it might not your original design.

[gpeal]

@bearprada You can make your fragment args contain as much information as you want because it's just a parcelable class. We recommend using a kotlin data class with @Parcelize. You can store the information you need for both view models there.

[gpeal]

@Mishkun @bearprada @hellohuanlin @BenSchwab Is #148 sufficient to resolve this?

[BenSchwab]

@gpeal I don't think so. I think the rationale here is that if your state needs to reference a property that is stored in something other than args (state in a dagger component for example), you can't put in it your state without making it artificially nullable. Even with fragmentFactory, the state will be created before the dagger component is easily accessible.

My thought of a pretty easy fix is to introduce an optional fun initialState(activity/fragment): S inside ViewModel Factories. Internally, we would take the result of this function, and then apply state restoration, before passing to createViewModel.

[gpeal]

@BenSchwab That could work actually!

[BenSchwab]

Played around a bunch, and couldn't find a clean fully backwards compatible solution. This is what I am leaning towards now:

interface MvRxVMFactory<S: MvRxState> {


    fun create(viewModelContext: ViewModelContext): BaseMvRxViewModel<S>? = null

    fun initialState(viewModelContext: ViewModelContext): S? = null

}

sealed class ViewModelContext {
    abstract val args: Any?
}

class ActivityViewModelContext(val activity: Activity, override val args: Any?) : ViewModelContext()

class FragmentViewModelContext(val activity: Activity, val fragment: Fragment, override val args: Any?) : ViewModelContext()

This approach has a few advantages:

1. One factory for both activity scoped and fragment scoped VMs. Currently, if you use a FragmentViewModelFactory but scope it to an activity, you will crash. The use of a sealed class makes acknowledging this possibility explicit.
2. Args are weakly typed in this model (which I think is fine, because inherently they are loosely typed). This keeps the model consistent with the constructor based state creation, which could have many constructors with different arg types.
3. Decouples args from state -- you can access args just for use in ViewModel
4. Allows DI of state objects by giving access to activity/fragment.
5. Nullable return types for state creation and view model creation allow clean fallbacks to default factories.

[elihart]

@BenSchwab that looks pretty good to me - the benefits you listed all seem great.

One thing - are you sure weakly typed args are a good idea? I think in most cases there will only be one type, and if somebody has a case for multiple args types they can use Any explicitly. I'm concerned that not providing strong typing would lead to more boilerplate, and making it harder to catch the error case where the wrong args type is passed.

[BenSchwab]

Good point @elihart. I like the idea of preferring typed args, but allowing Any as an escape. Only downside I see is it forces one more generic into the factory interface.