cashapp / molecule

Build a StateFlow stream using Jetpack Compose
https://cashapp.github.io/molecule/docs/1.x/
Apache License 2.0
1.89k stars 81 forks source link

Add recipes/samples module showing common patterns and gotchas #33

Open JakeWharton opened 3 years ago

JakeWharton commented 2 years ago

E.g.,

val stuff by foo.bar().collectAsState()

will invoke foo.bar() on every recomposition which probably returns a new instance which means the old flow will be canceled and the new one collected. Needs hoisted to enclosing class/file or wrapped in a produce state call.

JakeWharton commented 2 years ago

Non-main thread and/or non-AndroidUiDispatcher usage.

DSteve595 commented 2 years ago

I have a WIP lint detector for collectAsState() which suggests remembering the source Flow. Should I try to finish that and bring it in?

Edit: Structurally this is a bit difficult. It's simple to detect the issue when the collectAsState is called in the same composable that retrieved the flow:

@Composable fun createCoolUiState(repo: Repository): CoolUiState {
  val data by repo.getDataFlow().collectAsState() // report the issue here!
  return CoolUiState(title = data.title)
}

but it doesn't make as much sense when the Flow is passed to another composable:

@Composable fun createCoolUiState(repo: Repository): CoolUiState {
  val dataFlow = repo.getDataFlow()
  return CoolUiState(title = title(dataFlow))
}

@Composable fun title(dataFlow: Flow<Data>): String {
  val data by dataFlow.collectAsState()
  // it doesn't make much sense to report the issue here,
  //  since `remember`ing the Flow here won't help anything
  return CoolUiState(title = data.title)
}

In practice, I've been hoisting the collectAsState to happen in the root composable, because often many inner UI states need the same data. But I don't think it's obvious that this is a good practice. I was thinking of making sure all declarations/reads of Flows are remembered:

@Composable fun createCoolUiState(repo: Repository): CoolUiState {
  val dataFlow = repo.getDataFlow() // report the issue here!
  return CoolUiState(title = title(dataFlow))
}

@Composable fun title(dataFlow: Flow<Data>): String {
  val data by dataFlow.collectAsState()
  return CoolUiState(title = data.title)
}

but that seems potentially heavy-handed.

PaulWoitaschek commented 1 year ago

I wonder how to generally manage more complex tasks involving nullability and the general problem that it's quite a ceremony to bridge a suspend function to a compose state.

To evluate this as a general architecture pattern, I've rewritten some code to compose, while trying to maintain the original logics:

Original Flow

  fun diaryProFlow(): StateFlow<DiaryProCard?> = userRepo.flow()
    .flatMapLatest { user ->
      if (user?.isPro == true) return@flatMapLatest flowOf(null)
      val purchaseItem = getPurchaseItem() ?: return@flatMapLatest flowOf(null)
      offerCardViewState(purchaseItem)
    }
    .map<PurchaseOfferCardViewState?, DiaryProCard?> { cardViewState ->
      when {
        cardViewState == null || cardViewState.countdown <= Duration.ZERO -> null
        else -> DiaryProCard.Offer(cardViewState)
      }
    }
    .stateIn(scope, SharingStarted.Eagerly, DiaryProCard.Loading)

Logics

  1. It starts with Loading
  2. If a user is pro, the flow emits null
  3. If there are no purchase items, the flow emits null
  4. Then it switches to an offer countdown and emits offers
  5. Once the offer countdown reaches 0, emit null again.

Compose

  sealed interface Task<out T> {
    object Loading : Task<Nothing>
    data class Content<T>(val content: T) : Task<T>
  }

  @Composable
  fun diaryProCardComposable(): DiaryProCard? {
    val user = remember { userRepo.flow() }.collectAsState(initial = null).value
      ?: return DiaryProCard.Loading
    if (user.isPro) {
      return null
    }

    val purchaseItemLoadingState = produceState<Task<PurchaseItemBundle?>>(
      initialValue = Task.Loading,
      producer = {
        value = Task.Content(getPurchaseItem())
      },
    ).value

    val purchaseItem = if (purchaseItemLoadingState is Task.Content) {
      purchaseItemLoadingState.content
    } else {
      return DiaryProCard.Loading
    }

    purchaseItem ?: return null

    val offerTask = remember(purchaseItem) {
      offerCardViewState(purchaseItem).map {
        Task.Content(it)
      }
    }.collectAsState(initial = Task.Loading).value

    val cardViewState = if (offerTask is Task.Content) {
      offerTask.content
    } else {
      return DiaryProCard.Loading
    }

    return if (cardViewState == null || cardViewState.countdown <= Duration.ZERO) {
      null
    } else {
      DiaryProCard.Offer(cardViewState)
    }
  }

Thoughts

I wonder, at which point this will scale and what would happen to the code base if we would apply compose absolutely everywhere.

It would probably introduce lots of helpers, around the Task and produce state.

The sample in the readme alread ignores lots of complexity:

@Composable
fun ProfilePresenter(
  userFlow: Flow<User>,
  balanceFlow: Flow<Long>,
): ProfileModel {
  val user by userFlow.collectAsState(null)
  val balance by balanceFlow.collectAsState(0L)

  return if (user == null) {
    Loading
  } else {
    Data(user.name, balance)
  }
}

What if the Flow<User>would be nullable? And it also has a bug because it will initially emit 0 as a balance, which is probably wrong. Also it uses delegations, so the smart cast to the not nullable user wouldn't be possible.

So to fix it, the balance would also need to be mapped to some form of a Generic LoadingState. Or have fallbacks with an initial null value. But that only works as long as the original type is not nullable as well. Doing the null default is also a little error prone, because when you make the data source nullable later, the code will still compile but contain a bug.

So I wonder: Which patterns have already emerged here? What's the larger picture if this is applied to a larger codebase?