Skip to content

Commit

Permalink
Send widget tag with property change (#2355)
Browse files Browse the repository at this point in the history
This will enable a future change to perform property value deserialization on the Zipline thread. Previously only the ID of the widget was available, and we can only resolve that to a widget tag on the main thread.
  • Loading branch information
JakeWharton authored Oct 2, 2024
1 parent 6b014bb commit 8b5f374
Show file tree
Hide file tree
Showing 18 changed files with 123 additions and 89 deletions.
6 changes: 3 additions & 3 deletions redwood-protocol-guest/api/redwood-protocol-guest.api
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ public final class app/cash/redwood/protocol/guest/DefaultGuestProtocolAdapter :
public fun appendCreate-kyz2zXs (II)V
public fun appendModifierChange-z3jyS0k (ILapp/cash/redwood/Modifier;)V
public fun appendMove-HpxY78w (IIIII)V
public fun appendPropertyChange-DxQz5cw (IILkotlinx/serialization/KSerializer;Ljava/lang/Object;)V
public fun appendPropertyChange-M7EZMwg (III)V
public fun appendPropertyChange-e3iP1vo (IIZ)V
public fun appendPropertyChange-13Ob0Yo (IIILkotlinx/serialization/KSerializer;Ljava/lang/Object;)V
public fun appendPropertyChange-ITsWdOQ (IIIZ)V
public fun appendPropertyChange-hzhmVHk (IIII)V
public fun appendRemove-HpxY78w (IIIILjava/util/List;)V
public fun emitChanges ()V
public fun getJson ()Lkotlinx/serialization/json/Json;
Expand Down
6 changes: 3 additions & 3 deletions redwood-protocol-guest/api/redwood-protocol-guest.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ final class app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter : app.ca
final val widgetSystem // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.widgetSystem|{}widgetSystem[0]
final fun <get-widgetSystem>(): app.cash.redwood.widget/WidgetSystem<kotlin/Unit> // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.widgetSystem.<get-widgetSystem>|<get-widgetSystem>(){}[0]

final fun <#A1: kotlin/Any?> appendPropertyChange(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/PropertyTag, kotlinx.serialization/KSerializer<#A1>, #A1) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendPropertyChange|appendPropertyChange(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.PropertyTag;kotlinx.serialization.KSerializer<0:0>;0:0){0§<kotlin.Any?>}[0]
final fun <#A1: kotlin/Any?> appendPropertyChange(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/WidgetTag, app.cash.redwood.protocol/PropertyTag, kotlinx.serialization/KSerializer<#A1>, #A1) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendPropertyChange|appendPropertyChange(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.WidgetTag;app.cash.redwood.protocol.PropertyTag;kotlinx.serialization.KSerializer<0:0>;0:0){0§<kotlin.Any?>}[0]
final fun appendAdd(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/ChildrenTag, kotlin/Int, app.cash.redwood.protocol.guest/ProtocolWidget) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendAdd|appendAdd(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.ChildrenTag;kotlin.Int;app.cash.redwood.protocol.guest.ProtocolWidget){}[0]
final fun appendCreate(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/WidgetTag) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendCreate|appendCreate(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.WidgetTag){}[0]
final fun appendModifierChange(app.cash.redwood.protocol/Id, app.cash.redwood/Modifier) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendModifierChange|appendModifierChange(app.cash.redwood.protocol.Id;app.cash.redwood.Modifier){}[0]
final fun appendMove(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/ChildrenTag, kotlin/Int, kotlin/Int, kotlin/Int) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendMove|appendMove(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.ChildrenTag;kotlin.Int;kotlin.Int;kotlin.Int){}[0]
final fun appendPropertyChange(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/PropertyTag, kotlin/Boolean) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendPropertyChange|appendPropertyChange(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.PropertyTag;kotlin.Boolean){}[0]
final fun appendPropertyChange(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/PropertyTag, kotlin/UInt) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendPropertyChange|appendPropertyChange(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.PropertyTag;kotlin.UInt){}[0]
final fun appendPropertyChange(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/WidgetTag, app.cash.redwood.protocol/PropertyTag, kotlin/Boolean) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendPropertyChange|appendPropertyChange(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.WidgetTag;app.cash.redwood.protocol.PropertyTag;kotlin.Boolean){}[0]
final fun appendPropertyChange(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/WidgetTag, app.cash.redwood.protocol/PropertyTag, kotlin/UInt) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendPropertyChange|appendPropertyChange(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.WidgetTag;app.cash.redwood.protocol.PropertyTag;kotlin.UInt){}[0]
final fun appendRemove(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/ChildrenTag, kotlin/Int, kotlin/Int, kotlin.collections/List<app.cash.redwood.protocol/Id>) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendRemove|appendRemove(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.ChildrenTag;kotlin.Int;kotlin.Int;kotlin.collections.List<app.cash.redwood.protocol.Id>){}[0]
final fun emitChanges() // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.emitChanges|emitChanges(){}[0]
final fun initChangesSink(app.cash.redwood.protocol/ChangesSink) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.initChangesSink|initChangesSink(app.cash.redwood.protocol.ChangesSink){}[0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,28 +86,31 @@ public class DefaultGuestProtocolAdapter(

public override fun <T> appendPropertyChange(
id: Id,
tag: PropertyTag,
widgetTag: WidgetTag,
propertyTag: PropertyTag,
serializer: KSerializer<T>,
value: T,
) {
changes.add(PropertyChange(id, tag, json.encodeToJsonElement(serializer, value)))
changes.add(PropertyChange(id, widgetTag, propertyTag, json.encodeToJsonElement(serializer, value)))
}

public override fun appendPropertyChange(
id: Id,
tag: PropertyTag,
widgetTag: WidgetTag,
propertyTag: PropertyTag,
value: Boolean,
) {
changes.add(PropertyChange(id, tag, JsonPrimitive(value)))
changes.add(PropertyChange(id, widgetTag, propertyTag, JsonPrimitive(value)))
}

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

override fun appendModifierChange(id: Id, value: Modifier) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,17 @@ public abstract class GuestProtocolAdapter(
@RedwoodCodegenApi
public abstract fun <T> appendPropertyChange(
id: Id,
tag: PropertyTag,
widgetTag: WidgetTag,
propertyTag: PropertyTag,
serializer: KSerializer<T>,
value: T,
)

@RedwoodCodegenApi
public abstract fun appendPropertyChange(
id: Id,
tag: PropertyTag,
widgetTag: WidgetTag,
propertyTag: PropertyTag,
value: Boolean,
)

Expand All @@ -102,7 +104,8 @@ public abstract class GuestProtocolAdapter(
@RedwoodCodegenApi
public abstract fun appendPropertyChange(
id: Id,
tag: PropertyTag,
widgetTag: WidgetTag,
propertyTag: PropertyTag,
value: UInt,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class GuestProtocolAdapterTest {

val expected = listOf(
Create(Id(1), WidgetTag(5)),
PropertyChange(Id(1), PropertyTag(2), JsonPrimitive("PT10S")),
PropertyChange(Id(1), WidgetTag(5), PropertyTag(2), JsonPrimitive("PT10S")),
)
assertThat(guestAdapter.takeChanges()).isEqualTo(expected)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class ProtocolTest {
// Text
Create(Id(2), WidgetTag(3)),
// text
PropertyChange(Id(2), PropertyTag(1), JsonPrimitive("hey")),
PropertyChange(Id(2), WidgetTag(3), PropertyTag(1), JsonPrimitive("hey")),
ModifierChange(Id(2)),
ChildrenChange.Add(Id(1), ChildrenTag(1), Id(2), 0),
// Row
Expand All @@ -129,7 +129,7 @@ class ProtocolTest {
// Text
Create(Id(4), WidgetTag(3)),
// text
PropertyChange(Id(4), PropertyTag(1), JsonPrimitive("hello")),
PropertyChange(Id(4), WidgetTag(3), PropertyTag(1), JsonPrimitive("hello")),
ModifierChange(Id(4)),
ChildrenChange.Add(Id(3), ChildrenTag(1), Id(4), 0),
ChildrenChange.Add(Id(1), ChildrenTag(1), Id(3), 1),
Expand All @@ -151,29 +151,29 @@ class ProtocolTest {
// Button
Create(Id(1), WidgetTag(4)),
// text
PropertyChange(Id(1), PropertyTag(1), JsonPrimitive("hi")),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(1), JsonPrimitive("hi")),
// onClick
PropertyChange(Id(1), PropertyTag(2), JsonPrimitive(false)),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(2), JsonPrimitive(false)),
// color
PropertyChange(Id(1), PropertyTag(3), JsonPrimitive(0u)),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(3), JsonPrimitive(0u)),
ModifierChange(Id(1)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(1), 0),
// Button
Create(Id(2), WidgetTag(4)),
// text
PropertyChange(Id(2), PropertyTag(1), JsonPrimitive("hi")),
PropertyChange(Id(2), WidgetTag(4), PropertyTag(1), JsonPrimitive("hi")),
// onClick
PropertyChange(Id(2), PropertyTag(2), JsonPrimitive(true)),
PropertyChange(Id(2), WidgetTag(4), PropertyTag(2), JsonPrimitive(true)),
// color
PropertyChange(Id(2), PropertyTag(3), JsonPrimitive(0u)),
PropertyChange(Id(2), WidgetTag(4), PropertyTag(3), JsonPrimitive(0u)),
ModifierChange(Id(2)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(2), 1),
// Button2
Create(Id(3), WidgetTag(7)),
// text
PropertyChange(Id(3), PropertyTag(1), JsonPrimitive("hi")),
PropertyChange(Id(3), WidgetTag(7), PropertyTag(1), JsonPrimitive("hi")),
// onClick
PropertyChange(Id(3), PropertyTag(2), JsonPrimitive(true)),
PropertyChange(Id(3), WidgetTag(7), PropertyTag(2), JsonPrimitive(true)),
ModifierChange(Id(3)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(3), 2),
),
Expand Down Expand Up @@ -214,11 +214,11 @@ class ProtocolTest {
// Button
Create(Id(1), WidgetTag(4)),
// text
PropertyChange(Id(1), PropertyTag(1), JsonPrimitive("state: 0")),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(1), JsonPrimitive("state: 0")),
// onClick
PropertyChange(Id(1), PropertyTag(2), JsonPrimitive(true)),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(2), JsonPrimitive(true)),
// color
PropertyChange(Id(1), PropertyTag(3), JsonPrimitive(0u)),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(3), JsonPrimitive(0u)),
ModifierChange(Id(1)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(1), 0),
),
Expand All @@ -230,7 +230,7 @@ class ProtocolTest {
assertThat(composition.awaitSnapshot()).isEqualTo(
listOf(
// text
PropertyChange(Id(1), PropertyTag(1), JsonPrimitive("state: 1")),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(1), JsonPrimitive("state: 1")),
),
)

Expand All @@ -240,9 +240,9 @@ class ProtocolTest {
assertThat(composition.awaitSnapshot()).isEqualTo(
listOf(
// text
PropertyChange(Id(1), PropertyTag(1), JsonPrimitive("state: 2")),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(1), JsonPrimitive("state: 2")),
// text
PropertyChange(Id(1), PropertyTag(2), JsonPrimitive(false)),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(2), JsonPrimitive(false)),
),
)

Expand All @@ -252,7 +252,7 @@ class ProtocolTest {
assertThat(composition.awaitSnapshot()).isEqualTo(
listOf(
// text
PropertyChange(Id(1), PropertyTag(1), JsonPrimitive("state: 3")),
PropertyChange(Id(1), WidgetTag(4), PropertyTag(1), JsonPrimitive("state: 3")),
),
)
}
Expand Down Expand Up @@ -283,9 +283,9 @@ class ProtocolTest {
// Button2
Create(Id(1), WidgetTag(7)),
// text
PropertyChange(Id(1), PropertyTag(1), JsonPrimitive("state: 0")),
PropertyChange(Id(1), WidgetTag(7), PropertyTag(1), JsonPrimitive("state: 0")),
// onClick
PropertyChange(Id(1), PropertyTag(2), JsonPrimitive(true)),
PropertyChange(Id(1), WidgetTag(7), PropertyTag(2), JsonPrimitive(true)),
ModifierChange(Id(1)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(1), 0),
),
Expand All @@ -297,7 +297,7 @@ class ProtocolTest {
assertThat(composition.awaitSnapshot()).isEqualTo(
listOf(
// text
PropertyChange(Id(1), PropertyTag(1), JsonPrimitive("state: 1")),
PropertyChange(Id(1), WidgetTag(7), PropertyTag(1), JsonPrimitive("state: 1")),
),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ class HostProtocolAdapterTest {
// Set Button's required color property.
PropertyChange(
id = Id(1),
tag = PropertyTag(3),
widgetTag = WidgetTag(4),
propertyTag = PropertyTag(3),
value = JsonPrimitive(0),
),
Add(
Expand Down Expand Up @@ -155,8 +156,9 @@ class HostProtocolAdapterTest {
val updateButtonText = listOf(
PropertyChange(
id = Id(1),
widgetTag = WidgetTag(4),
// text
tag = PropertyTag(1),
propertyTag = PropertyTag(1),
value = JsonPrimitive("hello"),
),
)
Expand Down Expand Up @@ -230,7 +232,7 @@ class HostProtocolAdapterTest {
ModifierChange(Id(2)),
// Text
Create(Id(3), WidgetTag(3)),
PropertyChange(Id(3), PropertyTag(1), JsonPrimitive("hello")),
PropertyChange(Id(3), WidgetTag(3), PropertyTag(1), JsonPrimitive("hello")),
ModifierChange(Id(3)),
Add(Id(2), ChildrenTag(1), Id(3), 0),
Add(Id(1), ChildrenTag(1), Id(2), 0),
Expand All @@ -241,7 +243,7 @@ class HostProtocolAdapterTest {
// Validate we're tracking ID=3.
host.sendChanges(
listOf(
PropertyChange(Id(3), PropertyTag(1), JsonPrimitive("hey")),
PropertyChange(Id(3), WidgetTag(3), PropertyTag(1), JsonPrimitive("hey")),
),
)

Expand All @@ -255,7 +257,7 @@ class HostProtocolAdapterTest {
assertFailure {
host.sendChanges(
listOf(
PropertyChange(Id(3), PropertyTag(1), JsonPrimitive("sup")),
PropertyChange(Id(3), WidgetTag(3), PropertyTag(1), JsonPrimitive("sup")),
),
)
}.isInstanceOf<IllegalStateException>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ class ProtocolFactoryTest {
val textInput = factory.widget(WidgetTag(5))!!.createNode(Id(1))

val throwingEventSink = UiEventSink { error(it) }
textInput.apply(PropertyChange(Id(1), PropertyTag(2), JsonPrimitive("PT10S")), throwingEventSink)
textInput.apply(PropertyChange(Id(1), WidgetTag(5), PropertyTag(2), JsonPrimitive("PT10S")), throwingEventSink)

assertThat((textInput.widget.value as TextInputValue).customType).isEqualTo(10.seconds)
}
Expand All @@ -272,7 +272,7 @@ class ProtocolFactoryTest {
)
val button = factory.widget(WidgetTag(4))!!.createNode(Id(1))

val change = PropertyChange(Id(1), PropertyTag(345432))
val change = PropertyChange(Id(1), WidgetTag(4), PropertyTag(345432))
val eventSink = UiEventSink { throw UnsupportedOperationException() }
val t = assertFailsWith<IllegalArgumentException> {
button.apply(change, eventSink)
Expand All @@ -292,7 +292,7 @@ class ProtocolFactoryTest {
)
val button = factory.widget(WidgetTag(4))!!.createNode(Id(1))

button.apply(PropertyChange(Id(1), PropertyTag(345432))) { throw UnsupportedOperationException() }
button.apply(PropertyChange(Id(1), WidgetTag(4), PropertyTag(345432))) { throw UnsupportedOperationException() }

assertThat(handler.events.single()).isEqualTo("Unknown property 345432 for 4")
}
Expand All @@ -314,7 +314,7 @@ class ProtocolFactoryTest {
val textInput = factory.widget(WidgetTag(5))!!.createNode(Id(1))

val eventSink = RecordingUiEventSink()
textInput.apply(PropertyChange(Id(1), PropertyTag(4), JsonPrimitive(true)), eventSink)
textInput.apply(PropertyChange(Id(1), WidgetTag(5), PropertyTag(4), JsonPrimitive(true)), eventSink)

(textInput.widget.value as TextInputValue).onChangeCustomType!!.invoke(10.seconds)

Expand Down
9 changes: 5 additions & 4 deletions redwood-protocol/api/redwood-protocol.api
Original file line number Diff line number Diff line change
Expand Up @@ -326,11 +326,12 @@ public final class app/cash/redwood/protocol/ModifierTag$Companion {

public final class app/cash/redwood/protocol/PropertyChange : app/cash/redwood/protocol/ValueChange {
public static final field Companion Lapp/cash/redwood/protocol/PropertyChange$Companion;
public synthetic fun <init> (IILkotlinx/serialization/json/JsonElement;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (IIILkotlinx/serialization/json/JsonElement;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
public fun getId-0HhLjSo ()I
public final fun getTag-VGAaYLs ()I
public final fun getPropertyTag-VGAaYLs ()I
public final fun getValue ()Lkotlinx/serialization/json/JsonElement;
public final fun getWidgetTag-BlhN7y0 ()I
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}
Expand All @@ -347,8 +348,8 @@ public synthetic class app/cash/redwood/protocol/PropertyChange$$serializer : ko
}

public final class app/cash/redwood/protocol/PropertyChange$Companion {
public final fun invoke-e3iP1vo (IILkotlinx/serialization/json/JsonElement;)Lapp/cash/redwood/protocol/PropertyChange;
public static synthetic fun invoke-e3iP1vo$default (Lapp/cash/redwood/protocol/PropertyChange$Companion;IILkotlinx/serialization/json/JsonElement;ILjava/lang/Object;)Lapp/cash/redwood/protocol/PropertyChange;
public final fun invoke-ITsWdOQ (IIILkotlinx/serialization/json/JsonElement;)Lapp/cash/redwood/protocol/PropertyChange;
public static synthetic fun invoke-ITsWdOQ$default (Lapp/cash/redwood/protocol/PropertyChange$Companion;IIILkotlinx/serialization/json/JsonElement;ILjava/lang/Object;)Lapp/cash/redwood/protocol/PropertyChange;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

Expand Down
Loading

0 comments on commit 8b5f374

Please sign in to comment.