airbnb / mavericks

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

How should I execute a request only if the Async object is not a Loading instance? #135

Closed alexbchr closed 5 years ago

alexbchr commented 5 years ago

In the Wiki, you mention that you could do something like that to trigger a request only if it's not in the loading state:

Unlike in a MvRxView/Fragment, this block is not run synchronously. All pending setState updates are run first so that your state is guaranteed to be up to date. MvRx will always run all pending setState updates before every single withState block so you could have code like this (which we don't recommend)

fun fetchSomething() = withState { state ->
    if (state.foo is Loading) return@withState
    MyRequest().execute { copy(foo = it) }
}

However, a similar code is used in the code sample in the DadJokeIndexViewModel:

fun fetchNextPage() = withState { state ->
        if (state.request is Loading) return@withState

        dadJokeService
                .search(page = state.jokes.size / JOKES_PER_PAGE + 1, limit = JOKES_PER_PAGE)
                .execute { copy(request = it, jokes = jokes + (it()?.results ?: emptyList())) }

    }

However, since you don't recommend the approach, it would be interesting to know what is the recommended approach to implement this. The wiki and code sample should be updated accordingly too.

gpeal commented 5 years ago

@alexbchr I'm confused. What is the contradiction?

alexbchr commented 5 years ago

It's not really a contradiction, it's more the fact that no recommended usage is given, you just show the non-recommended usage in the documentation and sample app. It would be interesting to have the recommended usage too.

elihart commented 5 years ago

I feel like something is wrong in the wiki, because I think we do recommend this pattern. Something may have not been updated correctly when we made improvements to how the view model processes withState calls before setState.

@gpeal is it correct that the wiki says (which we don't recommend)? I feel like we could just remove that

gpeal commented 5 years ago

@alexbchr @elihart Yeah, that seems like an out of date doc. I remove it.