Skip to content

Commit

Permalink
Update re-attached widget removes to use detach
Browse files Browse the repository at this point in the history
This ensures that the host-side maintains their instances to correctly support re-attach.
  • Loading branch information
JakeWharton committed Dec 13, 2024
1 parent 50d2de9 commit 1920eef
Show file tree
Hide file tree
Showing 14 changed files with 347 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Fixed:
- Add support for the Height modifier in `ComposeUiBox`.
- Add support for the Width modifier in `ComposeUiBox`.
- Call `DisposableEffect` when a screen is unbound. We were only calling these when the effect was removed from the composition.
- Support `movableContentOf` in Treehouse (and generally in the Redwood protocol). Note: this requires the host be running version 0.17.0 or newer.


## [0.16.0] - 2024-11-19
Expand Down
3 changes: 2 additions & 1 deletion redwood-protocol-guest/api/redwood-protocol-guest.api
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public final class app/cash/redwood/protocol/guest/DefaultGuestProtocolAdapter :
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-hx81-Ec (IIII)V
public fun appendRemove-ARs5Qwk (IIILapp/cash/redwood/protocol/guest/ProtocolWidget;)V
public fun emitChanges ()V
public fun getJson ()Lkotlinx/serialization/json/Json;
public fun getRoot ()Lapp/cash/redwood/widget/Widget$Children;
Expand All @@ -24,6 +24,7 @@ public abstract class app/cash/redwood/protocol/guest/GuestProtocolAdapter : app
public static final field $stable I
public synthetic fun <init> (Ljava/lang/String;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public abstract fun emitChanges ()V
protected final fun getHostVersion-7jYel6c ()Ljava/lang/String;
public abstract fun getRoot ()Lapp/cash/redwood/widget/Widget$Children;
public abstract fun getWidgetSystem ()Lapp/cash/redwood/widget/WidgetSystem;
public abstract fun initChangesSink (Lapp/cash/redwood/protocol/ChangesSink;)V
Expand Down
4 changes: 3 additions & 1 deletion redwood-protocol-guest/api/redwood-protocol-guest.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ abstract class app.cash.redwood.protocol.guest/GuestProtocolAdapter : app.cash.r
abstract fun <get-root>(): app.cash.redwood.widget/Widget.Children<kotlin/Unit> // app.cash.redwood.protocol.guest/GuestProtocolAdapter.root.<get-root>|<get-root>(){}[0]
abstract val widgetSystem // app.cash.redwood.protocol.guest/GuestProtocolAdapter.widgetSystem|{}widgetSystem[0]
abstract fun <get-widgetSystem>(): app.cash.redwood.widget/WidgetSystem<kotlin/Unit> // app.cash.redwood.protocol.guest/GuestProtocolAdapter.widgetSystem.<get-widgetSystem>|<get-widgetSystem>(){}[0]
final val hostVersion // app.cash.redwood.protocol.guest/GuestProtocolAdapter.hostVersion|{}hostVersion[0]
final fun <get-hostVersion>(): app.cash.redwood.protocol/RedwoodVersion // app.cash.redwood.protocol.guest/GuestProtocolAdapter.hostVersion.<get-hostVersion>|<get-hostVersion>(){}[0]

abstract fun emitChanges() // app.cash.redwood.protocol.guest/GuestProtocolAdapter.emitChanges|emitChanges(){}[0]
abstract fun initChangesSink(app.cash.redwood.protocol/ChangesSink) // app.cash.redwood.protocol.guest/GuestProtocolAdapter.initChangesSink|initChangesSink(app.cash.redwood.protocol.ChangesSink){}[0]
Expand All @@ -49,7 +51,7 @@ final class app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter : app.ca
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/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, 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;app.cash.redwood.protocol.Id){}[0]
final fun appendRemove(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/ChildrenTag, kotlin/Int, app.cash.redwood.protocol.guest/ProtocolWidget) // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.appendRemove|appendRemove(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.ChildrenTag;kotlin.Int;app.cash.redwood.protocol.guest.ProtocolWidget){}[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]
final fun nextId(): app.cash.redwood.protocol/Id // app.cash.redwood.protocol.guest/DefaultGuestProtocolAdapter.nextId|nextId(){}[0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import app.cash.redwood.protocol.PropertyChange
import app.cash.redwood.protocol.PropertyTag
import app.cash.redwood.protocol.RedwoodVersion
import app.cash.redwood.protocol.WidgetTag
import app.cash.redwood.protocol.guest.ProtocolWidget.Companion.INVALID_INDEX
import app.cash.redwood.widget.Widget
import app.cash.redwood.widget.WidgetSystem
import kotlinx.serialization.ExperimentalSerializationApi
Expand Down Expand Up @@ -136,9 +137,18 @@ public class DefaultGuestProtocolAdapter(
index: Int,
child: ProtocolWidget,
) {
val replaced = widgets.put(child.id.value, child)
check(replaced == null) {
"Attempted to add widget with ID ${child.id} but one already exists"
val childId = child.id.value
if (child.removeIndex != INVALID_INDEX) {
check(hostSupportsRemoveDetach) { "Host v$hostVersion does not support widget re-attach" }
check(childId in widgets) { "Attempted to re-attach unknown widget with ID $childId" }
removed.remove(childId)

// Update the remove change to indicate it's only a detach.
val remove = changes[child.removeIndex] as ChildrenChange.Remove
changes[child.removeIndex] = ChildrenChange.Remove(remove.id, remove.tag, remove.index, true)
} else {
val replaced = widgets.put(childId, child)
check(replaced == null) { "Attempted to add widget with existing ID $childId" }
}
changes.add(ChildrenChange.Add(id, tag, child.id, index))
}
Expand All @@ -157,9 +167,10 @@ public class DefaultGuestProtocolAdapter(
id: Id,
tag: ChildrenTag,
index: Int,
childId: Id,
child: ProtocolWidget,
) {
removed += childId.value
removed += child.id.value
child.removeIndex = changes.size
changes.add(ChildrenChange.Remove(id, tag, index, detach = false))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,19 @@ import kotlinx.serialization.json.Json
* This interface is for generated code use only.
*/
public abstract class GuestProtocolAdapter(
hostVersion: RedwoodVersion,
protected val hostVersion: RedwoodVersion,
) : EventSink {
@RedwoodCodegenApi
public abstract val json: Json

/**
* Whether the host supports the detach parameter on
* [app.cash.redwood.protocol.ChildrenChange.Remove]. This also implies that the `count`
* parameter does not need to be sent.
* [app.cash.redwood.protocol.ChildrenChange.Remove].
*
* TODO Remove when minimum-supported host version is 1.17 or higher.
* TODO Remove when minimum-supported host version is 0.17 or higher.
*/
@RedwoodCodegenApi
protected val hostSupportsRemoveDetachAndNoCount: Boolean = hostVersion >= RedwoodVersion("0.17.0-SNAPSHOT")
protected val hostSupportsRemoveDetach: Boolean = hostVersion >= RedwoodVersion("0.17.0-SNAPSHOT")

/**
* The provider of factories of widgets which record property changes and whose children changes
Expand Down Expand Up @@ -143,6 +142,6 @@ public abstract class GuestProtocolAdapter(
id: Id,
tag: ChildrenTag,
index: Int,
childId: Id,
child: ProtocolWidget,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ public interface ProtocolWidget : Widget<Unit> {
public val id: Id
public val tag: WidgetTag

/**
* Used to detect when a removed widget is re-added later in the frame, such as when
* `movableContentOf` is used. When a widget is added with this value greater than or equal to 0,
* the original 'remove' operation should be changed to indicate that this node
* (and its entire subtree) should be retained host-side, as a future 'add' of this node
* will occur without associated 'create' events for the subtree.
*/
public var removeIndex: Int

override val value: Unit get() = Unit

public fun sendEvent(event: Event)
Expand Down Expand Up @@ -73,4 +82,9 @@ public interface ProtocolWidget : Widget<Unit> {
children: ProtocolWidgetChildren,
)
}

@RedwoodCodegenApi // https://github.com/Kotlin/binary-compatibility-validator/issues/91
public companion object {
public const val INVALID_INDEX: Int = -1
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public class ProtocolWidgetChildren(

override fun remove(index: Int, count: Int) {
for (i in index + count - 1 downTo index) {
val widget = widgets[i]
guestAdapter.appendRemove(id, tag, index, widget.id)
guestAdapter.appendRemove(id, tag, index, widgets[i])
}
_widgets.remove(index, count)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,14 @@ package app.cash.redwood.protocol.guest

import androidx.compose.runtime.BroadcastFrameClock
import androidx.compose.runtime.getValue
import androidx.compose.runtime.movableContentOf
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import app.cash.redwood.compose.WidgetVersion
import app.cash.redwood.layout.compose.Box
import app.cash.redwood.layout.compose.Column
import app.cash.redwood.layout.compose.Row
import app.cash.redwood.protocol.Change
import app.cash.redwood.protocol.ChildrenChange
import app.cash.redwood.protocol.ChildrenTag
Expand All @@ -31,13 +36,15 @@ import app.cash.redwood.protocol.ModifierChange
import app.cash.redwood.protocol.PropertyChange
import app.cash.redwood.protocol.PropertyTag
import app.cash.redwood.protocol.RedwoodVersion
import app.cash.redwood.protocol.ValueChange
import app.cash.redwood.protocol.WidgetTag
import app.cash.redwood.testing.TestRedwoodComposition
import app.cash.redwood.ui.Cancellable
import app.cash.redwood.ui.OnBackPressedCallback
import app.cash.redwood.ui.OnBackPressedDispatcher
import app.cash.redwood.ui.UiConfiguration
import assertk.assertThat
import assertk.assertions.containsExactly
import assertk.assertions.isEqualTo
import com.example.redwood.testapp.compose.Button
import com.example.redwood.testapp.compose.Button2
Expand Down Expand Up @@ -288,6 +295,171 @@ class ProtocolTest {
)
}

@Test fun movableContentSameRecomposition() = runTest {
val (composition) = testProtocolComposition()

var state by mutableIntStateOf(0)
composition.setContent {
val oneTwoThree = remember {
movableContentOf {
Box {
Text("one")
Text("two")
Text("three")
}
}
}
when (state) {
0 -> Row { oneTwoThree() }
1 -> Column { oneTwoThree() }
}
}

assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
// Row to Root
Create(Id(1), WidgetTag(1000001)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(1), 0),
// Box
Create(Id(2), WidgetTag(1000004)),
// Text("one") to Box
Create(Id(3), WidgetTag(3)),
ChildrenChange.Add(Id(2), ChildrenTag(1), Id(3), 0),
// Text("two") to Box
Create(Id(4), WidgetTag(3)),
ChildrenChange.Add(Id(2), ChildrenTag(1), Id(4), 1),
// Text("three") to Box
Create(Id(5), WidgetTag(3)),
ChildrenChange.Add(Id(2), ChildrenTag(1), Id(5), 2),
// Box to Row
ChildrenChange.Add(Id(1), ChildrenTag(1), Id(2), 0),
)

state = 1
assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
// Box from Row
ChildrenChange.Remove(Id(1), ChildrenTag(1), 0, detach = true),
// Row from Root
ChildrenChange.Remove(Id.Root, ChildrenTag.Root, 0, detach = false),
// Column to Root
Create(Id(6), WidgetTag(1000002)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(6), 0),
// Box to Column
ChildrenChange.Add(Id(6), ChildrenTag(1), Id(2), 0),
)
}

@Test fun multipleMovableContentButOnlyOneReused() = runTest {
val (composition) = testProtocolComposition()

var state by mutableIntStateOf(0)
composition.setContent {
val one = remember { movableContentOf { Text("one") } }
val two = remember { movableContentOf { Text("two") } }
val three = remember { movableContentOf { Text("three") } }
when (state) {
0 -> {
one()
two()
three()
}
1 -> {
two()
}
}
}

assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
Create(Id(1), WidgetTag(3)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(1), 0),
Create(Id(2), WidgetTag(3)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(2), 1),
Create(Id(3), WidgetTag(3)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(3), 2),
)

state = 1
assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
ChildrenChange.Remove(Id.Root, ChildrenTag.Root, 0, detach = false),
ChildrenChange.Remove(Id.Root, ChildrenTag.Root, 0, detach = true),
ChildrenChange.Remove(Id.Root, ChildrenTag.Root, 0, detach = false),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(2), 0),
)
}

@Test fun movableContentMultipleRecompositions() = runTest {
val (composition) = testProtocolComposition()

var state by mutableIntStateOf(0)
composition.setContent {
val oneTwoThree = remember {
movableContentOf {
Box {
Text("one")
Text("two")
Text("three")
}
}
}
when (state) {
0 -> Row { oneTwoThree() }
1 -> Text("hey")
2 -> Column { oneTwoThree() }
}
}

assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
// Row to Root
Create(Id(1), WidgetTag(1000001)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(1), 0),
// Box
Create(Id(2), WidgetTag(1000004)),
// Text("one") to Box
Create(Id(3), WidgetTag(3)),
ChildrenChange.Add(Id(2), ChildrenTag(1), Id(3), 0),
// Text("two") to Box
Create(Id(4), WidgetTag(3)),
ChildrenChange.Add(Id(2), ChildrenTag(1), Id(4), 1),
// Text("three") to Box
Create(Id(5), WidgetTag(3)),
ChildrenChange.Add(Id(2), ChildrenTag(1), Id(5), 2),
// Box to Row
ChildrenChange.Add(Id(1), ChildrenTag(1), Id(2), 0),
)

state = 1
assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
// Box from Row
ChildrenChange.Remove(Id(1), ChildrenTag(1), 0, detach = false),
// Row from Root
ChildrenChange.Remove(Id.Root, ChildrenTag.Root, 0, detach = false),
// Text("hey") to Root
Create(Id(6), WidgetTag(3)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(6), 0),
)

state = 2
assertThat(composition.awaitSnapshot().filter { it !is ValueChange }).containsExactly(
// Text from Root
ChildrenChange.Remove(Id.Root, ChildrenTag.Root, 0, detach = false),
// Column to Root
Create(Id(7), WidgetTag(1000002)),
ChildrenChange.Add(Id.Root, ChildrenTag.Root, Id(7), 0),
// Box
Create(Id(8), WidgetTag(1000004)),
// Text("one") to Box
Create(Id(9), WidgetTag(3)),
ChildrenChange.Add(Id(8), ChildrenTag(1), Id(9), 0),
// Text("two") to Box
Create(Id(10), WidgetTag(3)),
ChildrenChange.Add(Id(8), ChildrenTag(1), Id(10), 1),
// Text("three") to Box
Create(Id(11), WidgetTag(3)),
ChildrenChange.Add(Id(8), ChildrenTag(1), Id(11), 2),
// Box to Column
ChildrenChange.Add(Id(7), ChildrenTag(1), Id(8), 0),
)
}

private fun TestScope.testProtocolComposition(
hostVersion: RedwoodVersion = latestVersion,
): Pair<TestRedwoodComposition<List<Change>>, GuestProtocolAdapter> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public value class RedwoodVersion(public val value: String) : Comparable<Redwood
}
}

override fun toString(): String = value

/**
* Compare two versions to see which is newer.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ internal class ProtocolButton(
) : ProtocolWidget, Button<Unit> {
public override val id: Id = guestAdapter.nextId()
public override val tag: WidgetTag get() = WidgetTag(3)
public override var removeIndex: Int = InvalidRemoveIndex
private var onClick: (() -> Unit)? = null
Expand Down Expand Up @@ -274,6 +275,12 @@ internal fun generateProtocolWidget(
)
.build(),
)
.addProperty(
PropertySpec.builder("removeIndex", INT, OVERRIDE)
.mutable(true)
.initializer("%T.INVALID_INDEX", ProtocolGuest.ProtocolWidget)
.build(),
)
.apply {
var nextSerializerId = 0
val serializerIds = mutableMapOf<TypeName, Int>()
Expand Down
Loading

0 comments on commit 1920eef

Please sign in to comment.