JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
16.19k stars 1.17k forks source link

LazyColumn not refreshed with particular model #487

Closed mister11 closed 3 years ago

mister11 commented 3 years ago

Environment info

Version: 0.3.1 OS: Arch Linux IDE: IntelliJ Java version: 11 Kotlin version: 1.4.30

Problem explanation

I have a somewhat strange issue when it comes to rendering updates when using LazyColumn and I'm not able to find any errors on my side so it might be something strange happening under-the-hood.

The code sample is a bit longer, but simple - it has a hardcoded list of Coin objects that are set as items (indexed) of a LazyColumn. Every 2 seconds, I simulate an update of that list (the original code was using RSocket, so this is for simplicity). When an update comes, there's no update of the LazyColumn view until I start scrolling.

Now, here's the strange part. I wanted to create a sample project for a bug report and I copied the whole thing to a new project and just replaced it with a smaller model called User. When I ran the app with it, everything worked, so I started experimenting.

I took the original code that's using Coin and not updating the list and mapped each item to User leaving everything else exactly the same and this worked - the list is refreshed on every update. The code sample below is a version with Coin that doesn't work. To make it work, uncomment line 78 that artificially maps it to the User model and change type from Coin to User in line 98.

Code example

import androidx.compose.desktop.Window
import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.itemsIndexed
import androidx.compose.material.Divider
import androidx.compose.material.Text
import androidx.compose.material.TopAppBar
import androidx.compose.runtime.Composable
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.dp
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import java.text.NumberFormat
import java.util.UUID
import kotlin.random.Random

val currencyFormatter = NumberFormat.getCurrencyInstance()

private val coinList = listOf(
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2),
    Coin(Random.nextLong().toString(), "Name 1", 100.0, "", 0.2, 0.2)
)

@ExperimentalFoundationApi
fun main() = Window {
    Column {
        TopAppBar(title = { Text(text = "CryptoCare") })
        ListView()
    }
}

@ExperimentalFoundationApi
@Composable
fun ListView() {
    val coinsState = remember { mutableStateOf(coinList) }
    val coroutineScope = rememberCoroutineScope()
    coroutineScope.launch {
        generateSequence { simulateMoneyUpdates() }
            .asFlow()
            .onEach { delay(2000) }
            .collect { payload ->
                coinsState.value = payload
            }
    }
    Box {
        LazyColumn(
            modifier = Modifier.fillMaxWidth()
        ) {
            itemsIndexed(items = coinsState.value
//                .map { User(UUID.nameUUIDFromBytes(it.id.toByteArray()), it.name, it.price) }
            ) { index, item ->
                println("$index, $item")
                CoinView(index, item)
                Divider()
            }
        }
    }

}

private fun simulateMoneyUpdates() = coinList.map {
    if (Random.nextDouble() > 0.2) {
        it.copy(price = Random.nextDouble(0.0, 10_000.0))
    } else {
        it
    }
}

@Composable
fun CoinView(index: Int, coin: Coin) {
    println("In")
    Row(
        modifier = Modifier.padding(10.dp)
    ) {
        Text((index + 1).toString(), modifier = Modifier.weight(.2f), textAlign = TextAlign.Center)
        Text(coin.name, modifier = Modifier.weight(1f), textAlign = TextAlign.Center)
        Text(currencyFormatter.format(coin.price), modifier = Modifier.weight(1f), textAlign = TextAlign.Center)
    }
}

data class User(
    val id: UUID,
    val name: String,
    val price: Double
)

data class Coin(
    val id: String,
    val name: String,
    val price: Double,
    val iconUrl: String,
    val change: Double,
    val marketCap: Double
)
olonho commented 3 years ago

Consider using mutableStateListOf().

alexandrepiveteau commented 3 years ago

Are you sure that you can start a coroutine like this ?

val coroutineScope = rememberCoroutineScope()
    coroutineScope.launch {
        generateSequence { simulateMoneyUpdates() }
            .asFlow()
            .onEach { delay(2000) }
            .collect { payload ->
                coinsState.value = payload
            }
    }

To me, it looks like you should use a LaunchedEffect(true) { ... }, with your generateSequence { ... }.asFlow().collect { ... } call inside to avoid incorrect behaviour on recomposition.

mister11 commented 3 years ago

Consider using mutableStateListOf().

Changing code to:

val coinsState = remember { mutableStateListOf<Coin>() }

// ...

generateSequence { simulateMoneyUpdates() }
            .asFlow()
            .onEach { delay(2000) }
            .collect { payload ->
                coinsState.clear()
                coinsState.addAll(payload)
            }

shows the same behavior.

mister11 commented 3 years ago

Are you sure that you can start a coroutine like this ?

val coroutineScope = rememberCoroutineScope()
    coroutineScope.launch {
        generateSequence { simulateMoneyUpdates() }
            .asFlow()
            .onEach { delay(2000) }
            .collect { payload ->
                coinsState.value = payload
            }
    }

To me, it looks like you should use a LaunchedEffect(true) { ... }, with your generateSequence { ... }.asFlow().collect { ... } call inside to avoid incorrect behaviour on recomposition.

I honestly didn't know about it. I'm still new to Compose. But, changing to code to use LaunchedEffect:

LaunchedEffect(true) {
        generateSequence { simulateMoneyUpdates() }
            .asFlow()
            .onEach { delay(2000) }
            .collect { payload ->
                coinsState.clear()
                coinsState.addAll(payload)
            }
    }

shows the same behavior.

akurasov commented 3 years ago

@mister11 I run your code on 1.0.0-alpha1-rc1 and it worked fine (data refreshed without scrolling). Do you still experience this issue?

mister11 commented 3 years ago

@akurasov I can confirm it works with alpha-2 (latest) release. Thank you for the fix and for letting me know :)

okushnikov commented 2 months ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.