https://kotlinlang.org logo
Join Slack
Powered by
Hmm, this seems like a Compose bug. Or am I doing ...
# compose
a
Hmm, this seems like a Compose bug. Or am I doing something wrong? Code in thread.
🤯 1
Copy code
class Data(
    val index: Int,
    val dataFlow: StateFlow<String>
)

private val LocalData = compositionLocalOf<Data>{ error("Missing local data") }


@Composable
fun DataView(){
    val data = LocalData.current
    val text by data.dataFlow.collectAsState()

    println("Displaying $text for index ${data.index}")
    Text(text)
}

fun main(){
    singleWindowApplication(
        state = WindowState(
            width = 800.dp,
            height = 800.dp,
        )
    ) {
        Column {
            var data: Data by remember { mutableStateOf(Data(1, MutableStateFlow("Hello"))) }
            CompositionLocalProvider(LocalData provides data){
                DataView()
            }
            Button(
                onClick = {
                    data = Data(2, MutableStateFlow("World"))
                }
            ){
                Text("Click me")
            }
        }
    }
}
This prints
Copy code
Displaying Hello for index 1
Displaying Hello for index 2
Displaying World for index 2
when the button is clicked.
The question is, of course, how can it print that 2nd line?
as there is never a 
Data
object in existance that has index 2 and a 
dataFlow
whose value is “Hello”
Ok, here’s a clearer case, without the `CompositionLocal`:
Copy code
class Data(
    val index: Int,
    val dataFlow: StateFlow<String>
)

@Composable
fun DataView(data: Data){
    val text by data.dataFlow.collectAsState()

    println("Displaying $text for index ${data.index}")
    Text(text)
}

fun main(){
    singleWindowApplication(
        state = WindowState(
            width = 800.dp,
            height = 800.dp,
        )
    ) {
        Column {
            var data by remember { mutableStateOf(Data(1, MutableStateFlow("Hello"))) }
            DataView(data)
            Button(
                onClick = {
                    data = Data(2, MutableStateFlow("World"))
                }
            ){
                Text("Click me")
            }
        }
    }
}
I think it’s a bug in 
produceState
Copy code
@Composable
fun <T> produceState(
    initialValue: T,
    key1: Any?,
    key2: Any?,
    @BuilderInference producer: suspend ProduceStateScope<T>.() -> Unit
): State<T> {
    val result = remember { mutableStateOf(initialValue) }
    LaunchedEffect(key1, key2) {
        ProduceStateScopeImpl(result, coroutineContext).producer()
    }
    return result
}
When it’s called with a new key, it will return 
result
whose (old) 
value
can be read, and only afterwards 
LaunchedEffect
will update it.
What do you think of this workaround?
Copy code
@Composable
fun <T> StateFlow<T>.fixedCollectAsState(
    context: CoroutineContext = EmptyCoroutineContext
): State<T> = fixedCollectAsState(value, context)


@Composable
fun <T : R, R> Flow<T>.fixedCollectAsState(
    initial: R,
    context: CoroutineContext = EmptyCoroutineContext
): State<R> = fixedProduceStateForCollect(initial, this, context) {
    if (context == EmptyCoroutineContext) {
        collect { value = it }
    } else withContext(context) {
        collect { value = it }
    }
}


@OptIn(ExperimentalTypeInference::class)
@Composable
fun <T> fixedProduceStateForCollect(
    initialValue: T,
    key1: Any?,
    key2: Any?,
    @BuilderInference producer: suspend MutableState<T>.() -> Unit
): State<T> {
    val result = remember(key1, key2) { mutableStateOf(initialValue) }
    LaunchedEffect(key1, key2){
        result.producer()
    }
    return result
}
I just replaced all 
collectAsState
uses in my app with 
fixedCollectAsState
and that fixed another problem. I have a root object with many properties exposed as 
StateFlow
and many UI elements taking the root object as an argument and each collects and displays a different property. When the root object changes, what would happen is I would see the UI update in parts. For a split second, you could see some of the UI elements displaying the properties of the old object and some displaying the properties of the new object. I thought this was just how Compose worked with Flows. But after replacing 
collectAsState
with 
fixedCollectAsState
, this issue is completely gone. The UI updates all at once to the new object!
a
This is how 
produceState()
is designed. A workaround is to use 
val text by key(data) { data.dataFlow.collectAsState() }
if you can’t change 
Data
class to use mutable states.
a
That seems very counter-intuitive.
Also, it doesn’t make sense. It does switch to the new flow, it just first gives you the last value of the previous flow.
a
It does not "give" you the last value, it's just the first value of the new flow hasn't been collected. Collecting a new flow will be delayed by a frame and that's why you need to provide the initial value yourself.
a
I am providing it when I'm using a new stateflow. But it returns a State with the value of the old flow.
a
The state "collectAsState()" returns is always the same state. The last value of the previous is collected into the state before, and the new value of the current flow will be collected in the next frame, so the current value of the state is the former. Is this clear enough?
a
It's perfectly clear, it's just wrong. When I call stateFlow.collectAsState() I expect the returned State to only ever hold values from that StateFlow. Not from anywhere else.
👍 1
Does it look right to you that
Copy code
MutableStateFlow(n).collectAsState().value
can be not equal to 
n
?
a
No, and it won't happen. Passing different flow objects to 
collectAsState()
doesn't look right to me in the first place probably because I'm used to the compose world as it's actually a common cause of error.
a
Copy code
@Composable
fun DataView(n: Int){
    val value = MutableStateFlow(n).collectAsState().value
    if (value != n)
        throw IllegalStateException()
    Text(n.toString())
}

fun main(){
    singleWindowApplication(
        state = WindowState(
            width = 800.dp,
            height = 800.dp,
        )
    ) {
        Column {
            var value by remember { mutableStateOf(0) }
            DataView(value)
            Button(
                onClick = {
                    value = 1
                }
            ){
                Text("Click me")
            }
        }
    }
}
This throws 
IllegalStateException
when you press the button.
a
As I explained before this is expected and is how 
collectAsState()
is designed. Doesn't change my argument.
a
Can you point me to the documentation that even hints that this can happen?
a
No I can't and I didn't say anything indicating the doc is perfect. There are many things in compose that are not clearly documented.
I'm not gonna argue with you. If you feel it's not right, file an issue and gook luck.
a
There's an existing issue, linked above. I added my comments to it.
Especially given how it's a one-line fix to make it intuitive, and avoid an extra recomposition with an old value, even if the current implementation is intentional, I would think it's worth changing it.
a
Found the related discussion and the issue (for 
produceState()
, though).
a
Ok, so at the very least people way more experienced than me do agree with my point of view.
Do you think this is the same issue, or a different one? https://issuetracker.google.com/issues/232007227
a
Looks like a different issue. Might be related but this definitely looks like a bug.
However for correctness and better performance you should use this anyway:
Copy code
for (index in (0 until people.size)) {
    key(people[index]) {
        val person = people[index].collectAsState().value
        PersonUi(person)
        if (person != null){
            val car = person.car.collectAsState().value
            Text("    ${car.model}")
        }
    }
}
a
Copy code
@Composable
fun <T : R, R> Flow<T>.fixedCollectAsState(
    initialValue: R,
    context: CoroutineContext = EmptyCoroutineContext
): State<R>{
    val result = remember { mutableStateOf(initialValue) }
    key(this){
        result.value = initialValue
    }
    LaunchedEffect(this, context){
        if (context == EmptyCoroutineContext) {
            collect { result.value = it }
        } else withContext(context) {
            collect { result.value = it }
        }
    }

    return result
}
This keeps returning the same 
State
and avoids the problem of updating the value too late on a switch. But it causes an extra recomposition for some reason. So my original example would print
Copy code
Displaying Hello for index 1
Displaying World for index 2
Displaying World for index 2
a
Don’t do this. The 
key(this) { … }
part in your code doesn’t make any sense. Just use 
key(flow) { flow.collectAsState() }
.
a
It makes sure to update the value before returning when the flow changes.
a
I know what you want to do. I’m saying that the code is wrong. It may work, but just accidentally.
result.value = initialValue
will run when any of the parameters change, including 
initialValue
and 
context
.
a
Shouldn’t it run only when 
this
changes?
a
No. You can try yourself.
You are probably mixing 
key
and 
remember
here. They are different.
a
Yes, I was thinking of it as a 
remember
but without the actual remembering
Then I don’t understand what it does.
Copy code
@Composable
inline fun <T> key(
    @Suppress("UNUSED_PARAMETER")
    vararg keys: Any?,
    block: @Composable () -> T
) = block()
Why is it called when something that isn’t its key changes? Because it’s 
inline
?
Or because it takes a vararg?
i.e. an Array
But then I don’t understand its point.
a
Unlike 
remember
, 
key
execute the body every time, just like without the 
key
call. It just dismiss all the internal states when any of the keys passed to 
key
changes.
So
Copy code
key(this) {
    result.value = initialValue
}
is mostly the same as
Copy code
result.value = initialValue
, just that when 
this
changes, the former will run again.
a
What causes it to dismiss all the internal states?
a
It’s the feature of 
key
(
key
is designed to do this).
a
I mean how does it do that?
a
It’s done by the compose compiler and runtime. If you are asking because of the source, that’s actually just part of it.
a
I can't find any decent documentation on it, and I'm not sure i understand what it does or when to use it.
l
key itself is an intrinsic, meaning that even though the source code of it is just 
block
, your code never actually calls it, and when the compiler sees calls to it it transforms it into a different call. I believe the documentation used to say that, but at some point it may have gotten removed. It is more or less transformed into a call to 
startMovableGroup()
with the appropriate 
dataKey
passed in. We could/should update the actual source code to: 1. mention the fact that it is an intrinsic 2. maybe make the source code (which never actually gets used) do something resembling what the intrinsic does. or just throw an error saying “this is an intrinsic and shouldnt ever get run”.
👍 1
n
Another ticket for probably the same issue: https://issuetracker.google.com/issues/222385306
a
I think I’m starting to understand what 
key
does and what it’s for. I have UI that displays something like a spreadsheet, and in each cell there is an 
AnimatedContent
such that when the value of the cell changes to +1 or -1, there’s an animation of the number going up/down. This is so when you press +/- on a cell, it does a nice animation. Now, what happened is that by pure chance, I had two spreadsheets, one with 
4
in a cell and the other with 
5
in the same cell. So when I would switch between the two spreadsheets, the animation would run. Solved by wrapping the entire rendering of the spreadsheet in 
key(spreadsheet){}
which clears the gap buffer and removes the state remembered by 
AnimatedContent
, preventing the animation from running on different spreadsheets.
22 Views
Open in Slack
PreviousNext
Powered by