airbnb / mavericks

Mavericks: Android on Autopilot
https://airbnb.io/mavericks/
Apache License 2.0
5.83k stars 500 forks source link

Reducer passed to setState method is called twice #268

Closed pchmielowski closed 5 years ago

pchmielowski commented 5 years ago

Here is a simple test case, which shows that reducer passed as an argument to setState method is invoked twice, even if the caller (method foo()) is invoked just once.

class InvokeOnceTest {

    @Test
    fun `foo invoked`() {
        val model = TestViewModel(TestState())

        model.foo()
        Thread.sleep(100)

        assertThat(invocations, `is`(1))
    }
}

var invocations = 0

data class TestState(val value: Unit = Unit) : MvRxState

class TestViewModel(initialState: TestState) : BaseMvRxViewModel<TestState>(initialState, true) {

    fun foo() = setState {
        invocations++
        this
    }
}

The result:

java.lang.AssertionError: 
Expected: is <1>
     but: was <2>

Is it a desired behavior or a bug? This behavior causes some problems for me, because in my real application I have a ViewModel with the following method:

    fun onLoginClick() = setState {
        if (login.isEmpty()) copy(status = Status.Error(LoginError.EmptyLogin))
        else {
            performLogin(login)
            copy(status = Status.Pending)
        }
    }

The problem is that performLogin method is invoked twice, which leads to two server requests.

haroldadmin commented 5 years ago

You're passing debugMode = true to your ViewModel. In Debug mode, MvRx runs your state reducers twice to make sure that your state is immutable. Since you modify state as well as perform a side effect (invocations++) in the state reducer, you get the side effect performed twice.

Please refer to this page for more information on this: Debug Checks

haroldadmin commented 5 years ago

Your onLoginClick method should look something like this:

fun onLoginClick() = withState { state ->
  if (state.login.isEmpty()) {
    setState { copy(status = Status.Error(LoginError.EmptyLogin)) }
  } else {
    viewModel.performLogin()
    setState {
      copy(status = Status.Pending)
    } 
  }  
pchmielowski commented 5 years ago

Ok, now I understand! Thank you for the explanation and correct code example ;)