From 0dd3844787aea26618ead9d8d9b7be33a39e2674 Mon Sep 17 00:00:00 2001 From: Jake Wharton Date: Tue, 10 Dec 2024 12:06:37 -0500 Subject: [PATCH] Implement host-side protocol widget reuse This does not yet support sending the detach flag from the guest. --- .../protocol/host/HostProtocolAdapter.kt | 11 +-- .../protocol/host/HostProtocolAdapterTest.kt | 71 ++++++++++++++++++- .../app/cash/redwood/protocol/protocol.kt | 9 ++- 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt b/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt index 0a0af0f43d..f14b7a05f8 100644 --- a/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt +++ b/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt @@ -36,7 +36,6 @@ import app.cash.redwood.protocol.RedwoodVersion import app.cash.redwood.protocol.WidgetTag import app.cash.redwood.widget.ChangeListener import app.cash.redwood.widget.Widget -import kotlin.collections.mutableMapOf import kotlin.native.ObjCName /** @@ -107,10 +106,12 @@ public class HostProtocolAdapter( } is Remove -> { - for (childIndex in change.index until change.index + change.count) { - val child = children.nodes[childIndex] - child.visitIds(removeNodeById) - poolOrDetach(child) + if (!change.detach) { + for (childIndex in change.index until change.index + change.count) { + val child = children.nodes[childIndex] + child.visitIds(removeNodeById) + poolOrDetach(child) + } } children.remove(change.index, change.count) } diff --git a/redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapterTest.kt b/redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapterTest.kt index ab0d7e6f29..4148476f39 100644 --- a/redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapterTest.kt +++ b/redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapterTest.kt @@ -29,6 +29,7 @@ import app.cash.redwood.protocol.PropertyChange import app.cash.redwood.protocol.PropertyTag import app.cash.redwood.protocol.WidgetTag import app.cash.redwood.protocol.guest.guestRedwoodVersion +import app.cash.redwood.testing.WidgetValue import app.cash.redwood.widget.MutableListChildren import assertk.assertFailure import assertk.assertThat @@ -37,7 +38,9 @@ import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf import assertk.assertions.message import com.example.redwood.testapp.protocol.host.TestSchemaProtocolFactory +import com.example.redwood.testapp.testing.TestRowValue import com.example.redwood.testapp.testing.TestSchemaTestingWidgetFactory +import com.example.redwood.testapp.testing.TextValue import com.example.redwood.testapp.widget.TestSchemaWidgetSystem import kotlin.test.Test import kotlin.test.assertFailsWith @@ -204,9 +207,10 @@ class HostProtocolAdapterTest { } @Test fun entireSubtreeRemoved() { + val container = MutableListChildren() val host = HostProtocolAdapter( guestVersion = guestRedwoodVersion, - container = MutableListChildren(), + container = container, factory = TestSchemaProtocolFactory( widgetSystem = TestSchemaWidgetSystem( TestSchema = TestSchemaTestingWidgetFactory(), @@ -263,4 +267,69 @@ class HostProtocolAdapterTest { .message() .isEqualTo("Unknown widget ID 3") } + + @Test fun detachAndAdd() { + val container = MutableListChildren() + val host = HostProtocolAdapter( + guestVersion = guestRedwoodVersion, + container = container, + factory = TestSchemaProtocolFactory( + widgetSystem = TestSchemaWidgetSystem( + TestSchema = TestSchemaTestingWidgetFactory(), + RedwoodLayout = RedwoodLayoutTestingWidgetFactory(), + RedwoodLazyLayout = RedwoodLazyLayoutTestingWidgetFactory(), + ), + ), + eventSink = ::error, + leakDetector = LeakDetector.none(), + ) + + // TestRow { + // TestRow { + // Text("hello") + host.sendChanges( + listOf( + // TestRow + Create(Id(1), WidgetTag(1)), + ModifierChange(Id(1)), + // TestRow + Create(Id(2), WidgetTag(1)), + ModifierChange(Id(2)), + // Text + Create(Id(3), WidgetTag(3)), + 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), + Add(Id.Root, ChildrenTag.Root, Id(1), 0), + ), + ) + + host.sendChanges( + listOf( + // Detach inner TestRow. + Remove(Id(1), ChildrenTag(1), 0, 1, detach = true), + // Remove outer TestRow. + Remove(Id.Root, ChildrenTag.Root, 0, 1), + // New outer TestRow. + Create(Id(4), WidgetTag(1)), + ModifierChange(Id(4)), + Add(Id.Root, ChildrenTag.Root, Id(4), 0), + // Re-attach inner TestRow. + Add(Id(4), ChildrenTag(1), Id(2), 0), + ), + ) + + assertThat(container.single().value).isEqualTo( + TestRowValue( + children = listOf( + TestRowValue( + children = listOf( + TextValue(text = "hello"), + ), + ), + ), + ), + ) + } } diff --git a/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt b/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt index 4a64fc113a..5e646d5c8c 100644 --- a/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt +++ b/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt @@ -248,6 +248,12 @@ public sealed interface ChildrenChange : Change { private val _tag: Int, public val index: Int, public val count: Int, + /** + * When true, the associated nodes should only be detached from the tree with the expectation + * that a future [Add] will re-attach them. Otherwise, nodes should be detached and fully + * removed for garbage collection. + */ + public val detach: Boolean = false, ) : ChildrenChange { override val id: Id get() = Id(_id) override val tag: ChildrenTag get() = ChildrenTag(_tag) @@ -258,7 +264,8 @@ public sealed interface ChildrenChange : Change { tag: ChildrenTag, index: Int, count: Int, - ): Remove = Remove(id.value, tag.value, index, count) + detach: Boolean = false, + ): Remove = Remove(id.value, tag.value, index, count, detach) } } }