Hi everyone. I was wondering what a good solution ...

# compose

[Simon Stahl]

Hi everyone. 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?

[Ian Lake]

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?

[Ian Lake]

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)

[Ian Lake]

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)

[Ian Lake]

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 = "")
}

[Ian Lake]

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

[Ian Lake]

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 Stahl]

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

[Simon Stahl]

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]

remember {} only goes inside of a Composable. Yeah.

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.

[Colton Idle]

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)

[Colton Idle]

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.

[Colton Idle]

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 Stahl]

right right, 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

[Simon Stahl]

i can split the methods up into one that provides the flow and one that provides the state. that way the VM is still unit testable

[Simon Stahl]

fun getMyValueFlow(): FLow<Int> = flowOf(1, 2, 3)

@Composable
fun getMyValueState() = 
    remember { getMyValueFlow() }.collectAsState(initial = 0)

[Colton Idle]

that is why i would like to move the remember and collectAsState to the VM.

Remember is not needed in a VM. Remember is a thing thats only used in COmposables. So there is no need to wrap things in remember. Just leave off the remember {} and everything should work. The reason remember{} is needed is so that if a composable is recomposed 1000 times in 1 second, that whatever you put in the remember { } isn't called 1000 times. A VM doesn't just "recompose" and so you don't need any concept of remembering.

CollectAsState should not be called in a VM. All examples from Android docs have it in the composable right before you draw your UI.

I have 3 compose only apps, with probably over 80 screens across all three apps. Not one of them uses a @Composable function in a ViewModel. Now... I am definitely not a compose expert, but what you sound like you want to do seems incorrect to me (although you have good intentions where you want to alleviate the need for even a composable to have to call collectAsState)

[Ian Lake]

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.

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

[Simon Stahl]

got it. @Colton Idle and @Ian Lake thank you both for the input. I will rethink our current solution.