spotify / mobius

A functional reactive framework for managing state evolution and side-effects.
https://spotify.github.io/mobius/
Apache License 2.0
1.23k stars 98 forks source link

mobius-android and jetpack compose (type-erasure and neighboring loops) #159

Closed 5TFN closed 1 year ago

5TFN commented 1 year ago

I'm succesfully using Mobius in an Android Kotlin project with MobiusLoopViewModel and Fragments. For a new project, I am trying to get Mobius running with Kotlin, Jetpack Compose and MobiusLoopViewModel. The loop is placed within the Viewmodel. Instead of using a Fragment as a host for the ViewModel, a Composable is used. I created two loops in two Composables, let's call them LoopOne and LoopTwo. LoopOne is inititialized first, followed by LoopTwo.

Model updates are to be received in the Composable like this:

val modelState: LoopTwoModel by viewModel.models.observeAsState(initialModel)

On the first model update, a class cast exception is thrown in the Composable at that position: "LoopOneModel cannot be cast to LoopTwoModel"

It seems that the combination of Compose and Mobius leads to some strange type mix-up. When I move LoopTwo to an Activity it all works as expected, but in a Composable it does not.

I added mobius-android as a local library for better insights. In the MobiusLoopViewModel class, when observing the loop (Line 100) the wrong LoopOne type is received instead of LoopTwo.

Line 100:

loop.observe(this::onModelChanged);

This wrong type is then posted as LiveData:

Line 230:

private void onModelChanged(M model) {
    modelData.postValue(model);
}

This is probably caused by Compose and Java interoperabilty issues. But I'd still like to share my results and ask for suggestions here.

anawara commented 1 year ago

Hey there!

Could you tell us a bit more about how you're setting things up. MobiusLoopViewModel does not currently support more than 1 loop at a time. If you have two loops, then I'd expect you're either

  1. Having a view model instance per loop
  2. Flatting both mobius domains into a single loop that then gets run in a single view model

How are you setting up your viewmodel to run both loops?

Also, the code example you're providing for how you observe the model updates in the composable does not show the type of view model you have. I would expect that viewModel is of type MobiusLoopViewModel<LoopTwoModel, LoopTwoEvent, LoopTwoEffect, LoopTwoViewEffect> in this case. Can you confirm that it is?

5TFN commented 1 year ago

I am using one ViewModel instance per loop.

After further investigation, I figured out that Compose is not the issue here.
Using Compose rather leads to a setup where two or multiple ViewModels nested in Composables are actually "side by side" in one Activity.

I created an example just using an Activity with two loops placed inside. It causes the same exception (line 57, see comment in next block).

FATAL EXCEPTION: main
                            Process: sample.mobiustwoloops, PID: 10770
                            java.lang.ClassCastException: sample.mobiustwoloops.featureOne.domain.LoopOneModel cannot be cast to sample.mobiustwoloops.featureTwo.domain.LoopTwoModel
                                at sample.mobiustwoloops.MainActivity$onCreate$$inlined$setObserver$6.invoke(MainActivity.kt:57)
                                at sample.mobiustwoloops.MainActivity$onCreate$$inlined$setObserver$6.invoke(MainActivity.kt:57)
                                at sample.mobiustwoloops.MainActivityKt$sam$i$androidx_lifecycle_Observer$0.onChanged(Unknown Source:2)
                                at androidx.lifecycle.LiveData.considerNotify(LiveData.java:133)
                                at androidx.lifecycle.LiveData.dispatchingValue(LiveData.java:151)
                                at androidx.lifecycle.LiveData.setValue(LiveData.java:309)
                                at androidx.lifecycle.MutableLiveData.setValue(MutableLiveData.java:50)
                                at androidx.lifecycle.LiveData$1.run(LiveData.java:93)
                                at android.os.Handler.handleCallback(Handler.java:938)
                                at android.os.Handler.dispatchMessage(Handler.java:99)
                                at android.os.Looper.loop(Looper.java:223)
                                at android.app.ActivityThread.main(ActivityThread.java:7656)
                                at java.lang.reflect.Method.invoke(Native Method)
                                at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
                                at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)```

Activity

typealias LoopOneViewModel = MobiusLoopViewModel<
        LoopOneModel,
        LoopOneEvent,
        LoopOneEffect,
        LoopOneViewEffect>

typealias LoopTwoViewModel = MobiusLoopViewModel<
        LoopTwoModel,
        LoopTwoEvent,
        LoopTwoEffect,
        LoopTwoViewEffect>

class MainActivity : AppCompatActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        val viewModelOne: LoopOneViewModel = getViewModel(LoopOneViewModelFactory(this))
        val viewModelTwo: LoopTwoViewModel = getViewModel(LoopTwoViewModelFactory(this))

        viewModelOne.setObserver(lifecycleOwner = this, models = { /*TODO*/ }, viewEffects = { /*TODO*/ })
        viewModelTwo.setObserver(lifecycleOwner = this, models = { /*TODO*/ }, viewEffects = { /*TODO*/ })

        setContentView(R.layout.activity_main)
    }
}

inline fun <reified T : ViewModel> AppCompatActivity.getViewModel(factory: ViewModelProvider.Factory) =
    ViewModelProvider(this, factory)[T::class.java]

inline fun <reified M, E, F, V> MobiusLoopViewModel<M, E, F, V>.setObserver(
    lifecycleOwner: LifecycleOwner,
    crossinline models: (M) -> Unit = { },
    crossinline viewEffects: (V) -> Unit = { }
) {
    this.viewEffects.setObserver(lifecycleOwner,
        { foreground -> viewEffects(foreground) },
        { background -> background.forEach { effect -> viewEffects(effect) } })

    this.models.observe(lifecycleOwner) { model -> models(model) } // line 57, error is thrown here 
} 

The ViewModelFactories for LoopOne and LoopTwo look like the following and are each placed within their own featureOne /featureTwo package

package sample.mobiustwoloops.featureOne

class LoopOneViewModelFactory(
    private val context: Context,
    private val initialModel: LoopOneModel = LoopOneModel()
): ViewModelProvider.Factory, DefaultLifecycleObserver {

    @Suppress("UNCHECKED_CAST")
    override fun <VM : ViewModel> create(modelClass: Class<VM>): VM =
            MobiusLoopViewModel.create(
                provideLoopFactory(context),
                initialModel,
                Init(::init)) as VM

    private fun provideLoopFactory(context: Context): Function<
            Consumer<LoopOneViewEffect>,
            MobiusLoop.Factory<LoopOneModel, LoopOneEvent, LoopOneEffect>> =
            Function { viewEffects ->
                loopFactory(::update, createObservableEffectHandler(context, viewEffects))
                    .logger(AndroidLogger.tag("LoopOne")) }
}
package sample.mobiustwoloops.featureTwo

class LoopTwoViewModelFactory(
    private val context: Context,
    private val initialModel: LoopTwoModel = LoopTwoModel()
): ViewModelProvider.Factory, DefaultLifecycleObserver {

    @Suppress("UNCHECKED_CAST")
    override fun <VM : ViewModel> create(modelClass: Class<VM>): VM =
            MobiusLoopViewModel.create(
                provideLoopFactory(context),
                initialModel,
                Init(::init)) as VM

    private fun provideLoopFactory(context: Context): Function<
            Consumer<LoopTwoViewEffect>,
            MobiusLoop.Factory<LoopTwoModel, LoopTwoEvent, LoopTwoEffect>> =
            Function { viewEffects ->
                loopFactory(::update, createObservableEffectHandler(context, viewEffects))
                    .logger(AndroidLogger.tag("LoopTwo")) }
togi commented 1 year ago

It sounds like your problem actually is that you're using ViewModelProvider.get(modelClass), which by default uses the model class name as the key in the ViewModelStore, which would be the same for both of your MobiusLoopViewModel instances due to type-erasure in JVM. The smallest fix for this would probably be to change your code to use ViewModelProvider.get(key, modelClass) like this:

inline fun <reified T : ViewModel> AppCompatActivity.getViewModel(factory: ViewModelProvider.Factory, key: String) =
    ViewModelProvider(this, factory).get(key, T::class.java)

val viewModelOne: MobiusLoopViewModel<*, *, *, *> = getViewModel(LoopOneViewModelFactory(this), "one")
val viewModelTwo: MobiusLoopViewModel<*, *, *, *> = getViewModel(LoopTwoViewModelFactory(this), "two")

I think personally I would prefer to make my two loops be two separate subclasses of MobiusLoopViewModel ie. LoopOneViewModel and LoopTwoViewModel that each call the protected super-constructor rather than using MobiusLoopViewModel.create(...), which would also ensure the key is going to be different for both loops:

class LoopTwoViewModel(
    private val initialModel: LoopTwoModel = LoopTwoModel()
): MobiusLoopViewModel<LoopTwoModel, LoopTwoEvent, LoopTwoEffect, LoopTwoViewEffect>(
  ::provideLoopFactory,
  initialModel,
  Init(::init)
) {

    // ...

    private fun provideLoopFactory(viewEffects: Consumer<LoopTwoViewEffects>):
        MobiusLoop.Factory<LoopTwoModel, LoopTwoEvent, LoopTwoEffect> =
                loopFactory(::update, createEffectHandler(viewEffects))
                    .logger(AndroidLogger.tag("LoopTwo"))

Btw, on a side note, you should probably not use context as a dependency in your MobiusLoopViewModel, as that could cause you to leak an activity. It could be fine if it's actually an ApplicationContext you pass in, but I'd probably still use ViewEffects to avoid the Context dependency in the effect handler.

5TFN commented 1 year ago

Yes, that was the issue. Thank you, that was very helpful.

I edited the title, so it can be more helpful for others.