One Off the Slack: Where Do Flow Operators Go?

Simon Stahl asked:

I was wondering what a good solution for the Flow operator functions should not be invoked within composition warning is. Say I have following method in my VM:

@Composable
fun getValue() =
    flowOf(1, 2, 3)
        .map { it.toString() } // warning here
        .collectAsState(initial = "")

The naive solution is to just split the method up

fun getValueFlow() =
    flowOf(1, 2, 3)
        .map { it.toString() }

@Composable
fun getValue() =
    getValueFlow()
        .collectAsState(initial = "")

But that is not really a functional change and if there is an issue with flow operations, then we would still have it. What would be a real fix for this warning?

Google’s Ian Lake attempted to explain what is going on here:

Think about this way: if you recomposed every single frame (i.e., an animation is running 🙂 ), how much work are you redoing as part of composition?

Here, you’re doing two things as part of composition: creating a brand new Flow (the flowOf() part) and applying operators on it (the map)

Both are those are going to be recreated and restarted on every composition, causing the collectAsState() from the previous composition to be cancelled (as the underlying Flow changed out from underneath it since the last composition)

So you need a way to have your Flow be the exact same instance on every recomposition. That means you can either make that Flow a val in your VM:

private val valueFlow = flowOf(1, 2, 3).map {
  it.toString()
}

@Composable
fun getValue() = valueFlow.collectAsState(initial = "")

or remember your Flow so that that instance survives across recomposition:

@Composable
fun getValue() {
  val valueFlow by remember {
    flowOf(1, 2, 3).map {
      it.toString()
    }
  }
  return valueFlow.collectAsState(initial = "")
}

That second approach is shown in the flowWithLifecycle example here: https://medium.com/androiddevelopers/a-safer-way-to-collect-flows-from-android-uis-23080b1f8bda#27a9

Of course, in that case, it isn’t a static flowOf(1, 2, 3), but a separate val Flow that doesn’t change across recompositions - that’s generally the only responsibility of the VM layer (so a @Composable method in a VM is a bit weird to begin with that really only makes that layer harder to test in isolation)

Simon responded:

I didn’t notice that wrapping the flow with a remember{} removes the warning. That makes perfect sense of course.

And do I understand you right that a VM should not have any @Composble methods? So it would just provide the flow and remember{} as well as collectAsState() should be located in the composable method itself?

Colton Idle chimed in:

remember {} only goes inside of a Composable.

Ideally, your Composable are as “dumb” as possible. Think about it this way. They just take the current state that you pass into it, and they draw it. Hopefully not too much business logic or anything going on in there because you don’t really control the composable re-composing. Sometimes I like to just think of it like “Hey my composable could be redrawing like 1000 times a second. I don’t really know, and my composable shouldn’t care”

So yes, ideally all of your actually go, networking, database, other observables, go in your ViewModel. This stuff happening in your ViewModel should end up “updating” some state in your VM. And then your UI/Composable just draws the current state.

Another way is to pretend your UI doesn’t exist. Pretend you’re just writing an in-memory application, that only runs on the command line. Pretend that at any given point, the user of your command line app can request the output of the state of your app. If the user reads the state of the app, they should have a general sense of the current state.

That’s basically the VM. You do everything in there, logic, networking, etc only to end up updating the state variable you have in the VM (state = basically just a really simple model with as many properties as you need to get the point across of what state your app is in)

It’s kinda nice if you think about it that way, because then you can unit test your VM. Run methods on it, and assert that your state is as expected.

The last part of your app could basically be “okay im going to take this state. pass it into a @Composable which will draw my state accurately to the user”

Simon agreed… and disagreed:

i couldn’t agree more. and this is definitely how i have set it up. i want to make it as easy as possible for the compose view code though, that is why i would like to move the remember and collectAsState to the VM. So this is the real question right now, is that ok? because doing so requires me to annotate the VM methods with @Composable

Ian was not a fan of that particular aspect of the plan:

I think you’re just moving complexity into the wrong layer, not removing it

Read the original thread in the kotlinlang Slack workspace. Not a member? Join that Slack workspace here!