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

Compose Web: wrong instance remembered when several remember calls in one scope #2535

Closed
bgcl opened this issue Dec 5, 2022 · 7 comments
Closed
Assignees

Comments

@bgcl
Copy link

bgcl commented Dec 5, 2022

Clicking the foo button the second and subsequent times causes the code for the bar button to be executed.

fun main() {
    renderComposableInBody {
        val words = remember { mutableStateListOf<String>() }
        words.map { P { Text(it) }}
        Button(attrs = { onClick { words.add("foo") } }) { Text("foo") }
        Button(attrs = { onClick { words.add("bar") } }) { Text("bar") }
    }
}

Following is the build.gradle:

plugins {
    kotlin("multiplatform") version "1.7.20"
    id("org.jetbrains.compose") version "latest.release"
    kotlin("plugin.serialization") version "latest.release"
}

repositories {
    mavenCentral()
    maven("https://maven.pkg.jetbrains.space/public/p/compose/dev")
    google()
}

kotlin {
    js(IR) {
        browser()
        binaries.executable()
    }
    sourceSets {
        val jsMain by getting {
            dependencies {
                implementation(compose.web.core)
                implementation(compose.runtime)
                implementation("io.ktor:ktor-client-core:latest.release")
                implementation("io.ktor:ktor-client-content-negotiation:latest.release")
                implementation("io.ktor:ktor-client-js:latest.release")
                implementation("io.ktor:ktor-serialization-kotlinx-json:latest.release")
            }
        }
    }
}
@eymar eymar added the web label Dec 6, 2022
@eymar
Copy link
Collaborator

eymar commented Dec 6, 2022

Reproduced with compose version 1.2.1 and kotlin 1.7.20.

Works as expected with compose version 1.2.0 and kotlin 1.7.10.

@bgcl
Copy link
Author

bgcl commented Dec 6, 2022

Thank you @eymar for the quick diagnosis and a workaround that helped unblock me! I can't wait to complete my 100% Kotlin (client+server) app and show it off to everybody :)

PS: My specific case also worked fine using 1.2.1 and 1.7.10, but I'll be more conservative and stick to 1.2.0 on 1.7.10.

@eymar eymar changed the title Bug in Jetpack Compose Web Compose Web: wrong instance remembered when several remember calls in one scope Dec 6, 2022
@eymar
Copy link
Collaborator

eymar commented Dec 6, 2022

@bgcl Did you encounter that issue in some more complex use cases?

It looks like even with 1.2.1 (and kotlin 1.7.20) there is a weird workaround:

@Composable
fun Content() {
        val words = remember { mutableStateListOf<String>() }
        words.map { P { Text(it) }}
        Button(attrs = { onClick { words.add("foo") } }) { Text("foo") }
        Button(attrs = { onClick { words.add("bar") } }) { Text("bar") }
}

fun main() {
    renderComposableInBody {
        Content()
    }
}

// or:

fun main() {
    val content = @Composable {
        val words = remember { mutableStateListOf<String>() }
        words.map { P { Text(it) }}
        Button(attrs = { onClick { words.add("foo") } }) { Text("foo") }
        Button(attrs = { onClick { words.add("bar") } }) { Text("bar") }
    }

    renderComposableInBody {
        content()
    }
}

So I'm wondering if you had other cases/snippets where the issue takes place.

@igordmn
Copy link
Collaborator

igordmn commented Dec 6, 2022

as expected with compose version 1.2.0 and kotlin 1.7.10.

It probably should also work with Compose 1.2.1 and Kotlin 1.7.10, because Compose Multiplatform supports multiple Kotlin's

@bgcl
Copy link
Author

bgcl commented Dec 6, 2022

@bgcl Did you encounter that issue in some more complex use cases?

So I'm wondering if you had other cases/snippets where the issue takes place.

@eymar I applied your workaround to my more complex use case and it does not work with k1.7.20. I'm posting the full code below although you wouldn't know how to run it live. But essentially, refactoring the whole thing into a separate composable still doesn't work with k1.7.20 but it works with k1.7.10 -- regardless of compose 1.2.0/1.2.1. (not factoring it out also does not work with k1.7.20 but works with k1.7.10 -- regardless of compose 1.2.0/1.2.1)

import androidx.compose.runtime.*
import androidx.compose.runtime.snapshots.SnapshotStateList
import io.ktor.client.*
import io.ktor.client.call.*
import io.ktor.client.plugins.contentnegotiation.*
import io.ktor.client.request.*
import io.ktor.serialization.kotlinx.json.*
import kotlinx.browser.window
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.MainScope
import kotlinx.coroutines.launch
import kotlinx.serialization.Serializable
import kotlinx.serialization.Transient
import kotlinx.serialization.json.Json
import org.jetbrains.compose.web.css.Color.gray
import org.jetbrains.compose.web.css.Color.green
import org.jetbrains.compose.web.css.Color.red
import org.jetbrains.compose.web.css.Color.yellow
import org.jetbrains.compose.web.css.backgroundColor
import org.jetbrains.compose.web.dom.*
import org.jetbrains.compose.web.renderComposableInBody

@Serializable
data class Message (val id: Int, val message: String)

@Serializable
class CurrentGame {
  val k: Int = 0
  lateinit var game: Game
}

@Serializable
class CompletedGame {
  lateinit var game: Game
}

@Serializable
class Game {
  val k: String = ""
  val game_moves: Collection<GameMove> = listOf()
  @Transient val game_moves_x: Collection<GameMoveX> = listOf()
}

@Serializable
class GameMove (@Transient val game: Game = Game(), val guess: String, val reply: String) {
  val i: Int = 0
  val j: Int = 0
}

fun main() {
    val cs = MainScope()
    val hc = HttpClient() {
        install(ContentNegotiation) {
            json(Json {
                prettyPrint = true
                isLenient = true
            })
        }
    }
    var initialized = mutableStateOf(false)
    var completed = mutableStateOf(false)
    val moves = mutableStateListOf<List<Pair<Char, Char>>>()
    val words = mutableStateListOf<String>()
    val guess = mutableStateListOf<Char>()
    cs.launch { hc.post("http://localhost:8080/currentgame").let {
        println("currentgame: ${it.status}")
        val game = it.body<Game>()
        println("game: ${game.k}")
        if (game.game_moves.lastOrNull()?.reply == "GGGGG") {
            completed.value = true
        }
        moves.clear()
        words.clear()
        game.game_moves.map {
            moves.add(it.guess.zip(it.reply))
            words.add(it.guess)
        }
        guess.clear()
        initialized.value = true
    } }
    window.onkeydown = fun (e) {
        if (! initialized.value || completed.value) return
        if (e.altKey || e.ctrlKey || e.metaKey || e.shiftKey) return
        e.keyCode.let { kc ->
            if (listOf(8, 46).contains(kc)) {
                guess.removeLast()
                return
            }
            if (kc == 13) {
                if (guess.size < 5) return
                cs.launch { hc.post("http://localhost:8080/gamemove") { setBody(guess.joinToString("")) }.body<Game>().let {
                    guess.clear(); moves.clear(); words.clear()
                    it.game_moves.map { moves.add(it.guess.zip(it.reply)); words.add(it.guess) }
                    if (it.game_moves.lastOrNull()?.reply == "GGGGG") completed.value = true
                    println(guess.joinToString(""))
                    println(moves.joinToString())
                    println(words.joinToString())
                } }
            }
            if (guess.size == 5) return
            if ((65..90).contains(kc).not()) return
        }
        guess.add(e.key.get(0))
    }
    renderComposableInBody {
        g(cs, hc, initialized, completed, guess, moves, words)
    }
}

@Composable fun g(
    cs: CoroutineScope,
    hc: HttpClient,
    initialized: MutableState<Boolean>,
    completed: MutableState<Boolean>,
    guess: SnapshotStateList<Char>,
    moves: SnapshotStateList<List<Pair<Char, Char>>>,
    words: SnapshotStateList<String>
) {
    if (! initialized.value) {
        P { Text("initializing; please wait up to 10 seconds") }
        return
    }
    if (completed.value) {
        Button(attrs = { onClick { cs.launch { hc.post("http://localhost:8080/newgame").body<Game>().let {
            completed.value = false
            guess.clear()
            moves.clear()
            words.clear()
            it.game_moves.map {
                moves.add(it.guess.zip(it.reply))
                words.add(it.guess)
            }
        } } } }) { Text("new game") }
    }
    Table {
        Tbody {
            moves.map { Tr { it.forEach { val color = when (it.second) { 'X' -> gray; 'Y' -> yellow; 'G' -> green; else -> red }; Td (attrs = { style { backgroundColor(color) } }) { Text(it.first.toString()) } } } }
            Tr { guess.forEach { Td { Text(it.toString()) } } }
        }
    }
}

@eymar
Copy link
Collaborator

eymar commented Dec 7, 2022

Hey @bgcl

we published a new build of compose compiler plugin you may want to try with kotlin 1.7.20.

// in your build.gradle.kts
compose {
    kotlinCompilerPlugin.set("1.3.2.2-beta01") // it's a temporary hack. The new compiler version will likely be included in compose-multiplatform 1.2.2
}

The issue should be gone.

Note: 1.3.2.2-beta01 is available only with:

maven {
            url = uri("https://maven.pkg.jetbrains.space/public/p/compose/dev")
        }

@bgcl
Copy link
Author

bgcl commented Dec 9, 2022

@eymar that does seem to work! thank you for unblocking me so promptly on k1.7.20 :) (sorry for the delay in confirming)

@eymar eymar closed this as completed Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants