haroldadmin / Vector

Kotlin Coroutines based MVI architecture library for Android
https://haroldadmin.github.io/Vector/
Apache License 2.0
193 stars 9 forks source link

Treat `withState` blocks as side effects #29

Closed haroldadmin closed 4 years ago

haroldadmin commented 4 years ago

Describe the bug Accessing the current state in a VectorViewModel is done through withState blocks. These blocks are processed just like state reducer setState blocks. However, withState blocks are generally used to perform some operations using some information contained in the current state. These operations might end up taking a long time to run, and therefore block the StateProcessor from being able to process new state updates while this operation completes.

To Reproduce An example of how this behaviour can cause problems can be seen in LaunchesViewModel of MoonShot:

private fun getAllLaunches() = withState { state ->
  launchesUseCase
    .getLaunches(state.filter, limit = fetchLimit, offset = state.offset)
    .collect { launchesResource ->
      setState {
        copy(
          launchesRes = launchesResource,
          launches = launches.append(launchesRes()),
          hasMoreToFetch = launchesResource().hasAtLeastSize(fetchLimit)
        )
      }
    }
 }

This method uses the withState block to fetch some information from the current state, and then starts collecting a flow. The problem here is that the withState block does not finish execution until the flow stops emitting. This means that any setState updates called before the last one within the flow collector are not processed until the flow completes its emissions.

Expected behavior Instead of processing withState blocks in a blocking manner within the StateProcessor, send them off to be processed in a different coroutine.

So instead of processing it as soon as it received, launch a separate coroutine to process them.

while (isActive) {
            select<Unit> {
                setStateChannel.onReceive { action ->
                    val newState = action.invoke(stateHolder.state)
                    stateHolder.stateObservable.offer(newState)
                }
                getStateChannel.onReceive { action ->
                    // Replace this: action.invoke(stateHolder.state)
                    // with this: launch { action.invoke(stateHolder.state) }
                }
            }
        }

Additional context The context of execution of this coroutine would be the state store context, which might cause some problems.