Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
JakeWharton opened this issue Oct 26, 2021 · 4 comments
Open

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

JakeWharton opened this issue Oct 26, 2021 · 4 comments

Comments

@JakeWharton
Copy link
Collaborator

No description provided.

@JakeWharton
Copy link
Collaborator Author

JakeWharton commented Nov 7, 2021

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
Copy link
Collaborator Author

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

@DSteve595
Copy link

DSteve595 commented Nov 8, 2021

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
Copy link

PaulWoitaschek commented Mar 27, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants