airbnb / mavericks

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

[MvRx & Epoxy] State is outdated in OnClickListener #129

Closed qbait closed 5 years ago

qbait commented 5 years ago

I have my button declared like that

    override fun epoxyController() = simpleController(viewModel) { state ->
        ...
        button {
            id("button")
            Timber.d("state in button = %s", state) // here my state is updated and works as expected
            title(formatButtonText(state.picksCounter)) 
            onClick { _ ->
                Timber.d("state in onClick = %s", state) // here I have the old state and my alert shows old info
                showConfirmationAlert(state)
            }
        }

For some reason, my state in onClick isn't refreshed until I will scroll my RecyclerView. The state in the button is always up to date.

Do you have an idea of what could be an issue or shall I prepare a simplified sample project?

[EDIT] I solved it by adding val showConfirmation: Boolean to my state. My epoxy config looks now like that

        button {
            id("button")
            onClick { _ ->
                viewModel.showConfirmation(true)
            }
        }

        if(state.showConfirmation) {
            confirmSelectionsDialog = showConfirmationAlert(state)
            viewModel.showConfirmation(false)
        }

Let me know if it's a good practice what I did and I'm still curious why onClick has outdated state.

hellohuanlin commented 5 years ago

@qbait when you do onClick { _ -> state.doSomething }, you are implicitly capturing the state inside the lambda, and since state is a data class, it's making a copy. When epoxy rebuild the model, callbacks are not hashed, so it still has the old lambda capturing the old state.

Here's 2 solutions you can consider

  1. do not pass the state to showConfirmation. call withState inside showConfirmation to get the most up to date state
  2. Hash the callback. Something like
    class KeyedOnClickListener {
    val key: WhateverKeyThatHasAProperlyDefinedHashFunction
    val listener: OnClickListener
    func hashCode() {
        return key.hashCode
    }
    }

    ^^ (pseudocode above)

Not sure if option 2 is already supported in epoxy? @elihart @BenSchwab @gpeal

elihart commented 5 years ago

See https://github.com/airbnb/epoxy/wiki/DoNotHash