Skip to content

Commit

Permalink
Work around serialization bug for UInt
Browse files Browse the repository at this point in the history
  • Loading branch information
swankjesse committed Jun 14, 2024
1 parent 222e2c4 commit d2f5ae5
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import app.cash.redwood.protocol.RedwoodVersion
import app.cash.redwood.protocol.WidgetTag
import app.cash.redwood.widget.Widget
import app.cash.redwood.widget.WidgetSystem
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.KSerializer
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonPrimitive
Expand Down Expand Up @@ -101,6 +102,15 @@ public class DefaultProtocolBridge(
changes.add(PropertyChange(id, tag, JsonPrimitive(value)))
}

@OptIn(ExperimentalSerializationApi::class)
override fun appendPropertyChange(
id: Id,
tag: PropertyTag,
value: UInt,
) {
changes.add(PropertyChange(id, tag, JsonPrimitive(value)))
}

public override fun appendModifierChange(
id: Id,
elements: List<ModifierElement>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,20 @@ public interface ProtocolBridge : EventSink {
value: Boolean,
)

/**
* There's a bug in kotlinx.serialization where decodeFromDynamic() is broken for UInt values
* larger than MAX_INT. For example, 4294967295 is incorrectly encoded as -1. We work around that
* here by special casing that type.
*
* https://github.com/Kotlin/kotlinx.serialization/issues/2713
*/
@RedwoodCodegenApi
public fun appendPropertyChange(
id: Id,
tag: PropertyTag,
value: UInt,
)

@RedwoodCodegenApi
public fun appendModifierChange(
id: Id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import com.squareup.kotlinpoet.STRING
import com.squareup.kotlinpoet.TypeName
import com.squareup.kotlinpoet.TypeSpec
import com.squareup.kotlinpoet.UNIT
import com.squareup.kotlinpoet.U_INT
import com.squareup.kotlinpoet.joinToCode

private val protocolViewType = UNIT
Expand Down Expand Up @@ -278,21 +279,32 @@ internal fun generateProtocolWidget(
when (trait) {
is ProtocolProperty -> {
val traitTypeName = trait.type.asTypeName()
val serializerId = serializerIds.computeIfAbsent(traitTypeName) {
nextSerializerId++
}

addFunction(
FunSpec.builder(trait.name)
.addModifiers(OVERRIDE)
.addParameter(trait.name, traitTypeName)
.addStatement(
"this.bridge.appendPropertyChange(this.id, %T(%L), serializer_%L, %N)",
Protocol.PropertyTag,
trait.tag,
serializerId,
trait.name,
)
.apply {
// Work around https://github.com/Kotlin/kotlinx.serialization/issues/2713.
if (traitTypeName == U_INT) {
addStatement(
"this.bridge.appendPropertyChange(this.id, %T(%L), %N)",
Protocol.PropertyTag,
trait.tag,
trait.name,
)
} else {
val serializerId = serializerIds.computeIfAbsent(traitTypeName) {
nextSerializerId++
}
addStatement(
"this.bridge.appendPropertyChange(this.id, %T(%L), serializer_%L, %N)",
Protocol.PropertyTag,
trait.tag,
serializerId,
trait.name,
)
}
}
.build(),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ internal class FastProtocolBridge(
changes.push(js("""["property",{"id":id,"tag":tag,"value":value}]"""))
}

override fun appendPropertyChange(id: Id, tag: PropertyTag, value: UInt) {
val id = id
val tag = tag
val value = value.toDouble()
changes.push(js("""["property",{"id":id,"tag":tag,"value":value}]"""))
}

override fun appendModifierChange(
id: Id,
elements: List<ModifierElement>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ class FastProtocolBridgeTest {
}
}

/** Test our special case for https://github.com/Kotlin/kotlinx.serialization/issues/2713 */
@Test fun consistentWithDefaultProtocolBridgeForUint() {
assertChangesEqual { root, widgetSystem ->
val button = widgetSystem.TestSchema.Button()
button.color(0xffeeddccu)
}
}

private fun assertChangesEqual(
block: (Widget.Children<Unit>, TestSchemaWidgetSystem<Unit>) -> Unit,
) {
Expand All @@ -72,8 +80,10 @@ class FastProtocolBridgeTest {
contextual(UInt::class, UInt.serializer())
}
}
assertThat(collectChangesFromFastProtocolBridge(json, block))
.isEqualTo(collectChangesFromDefaultProtocolBridge(json, block))

val fastUpdates = collectChangesFromFastProtocolBridge(json, block)
val defaultUpdates = collectChangesFromDefaultProtocolBridge(json, block)
assertThat(fastUpdates).isEqualTo(defaultUpdates)
}

private fun collectChangesFromDefaultProtocolBridge(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public data class Text(
public data class Button(
@Property(1) val text: String?,
@Property(2) val onClick: (() -> Unit)?,
@Property(3) val color: UInt,
)

/** Like [Button] but with a required lambda. */
Expand Down

0 comments on commit d2f5ae5

Please sign in to comment.