Do not store the WidgetTag + base impl of visitIds (#2314)

* Do not store the WidgetTag + base impl of visitIds Storing the WidgetTag is a waste of space. We rarely use it, and even then it's only an int so we can just codegen returning it directly. Do not generate implementations of visitIds for nodes with no children. Have the base class invoke the lambda with the current node ID by default. Only nodes for widigets with children will override and do codegen. * API dump, Spotless
cashapp · Sep 23, 2024 · 2a17540 · 2a17540
1 parent 63cde06
commit 2a17540
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 32 deletions.
diff --git a/redwood-protocol-host/api/redwood-protocol-host.api b/redwood-protocol-host/api/redwood-protocol-host.api
@@ -34,17 +34,17 @@ public final class app/cash/redwood/protocol/host/ProtocolMismatchHandler$Compan
 }
 
 public abstract class app/cash/redwood/protocol/host/ProtocolNode {
-	public synthetic fun <init> (IILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public synthetic fun <init> (ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public abstract fun apply (Lapp/cash/redwood/protocol/PropertyChange;Lapp/cash/redwood/protocol/host/UiEventSink;)V
 	public abstract fun children-dBpC-2Y (I)Lapp/cash/redwood/protocol/host/ProtocolChildren;
 	public abstract fun detach ()V
 	public final fun getId-0HhLjSo ()I
 	public abstract fun getWidget ()Lapp/cash/redwood/widget/Widget;
-	public final fun getWidgetTag-BlhN7y0 ()I
+	public abstract fun getWidgetTag-BlhN7y0 ()I
 	public final fun setId-ou3jOuA (I)V
 	public abstract fun toString ()Ljava/lang/String;
 	public final fun updateModifier (Lapp/cash/redwood/Modifier;)V
-	public abstract fun visitIds (Lkotlin/jvm/functions/Function1;)V
+	public fun visitIds (Lkotlin/jvm/functions/Function1;)V
 }
 
 public final class app/cash/redwood/protocol/host/UiEvent {

diff --git a/redwood-protocol-host/api/redwood-protocol-host.klib.api b/redwood-protocol-host/api/redwood-protocol-host.klib.api
@@ -34,12 +34,12 @@ sealed interface <#A: kotlin/Any> app.cash.redwood.protocol.host/ProtocolFactory
 }
 
 abstract class <#A: kotlin/Any> app.cash.redwood.protocol.host/ProtocolNode { // app.cash.redwood.protocol.host/ProtocolNode|null[0]
-    constructor <init>(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/WidgetTag) // app.cash.redwood.protocol.host/ProtocolNode.<init>|<init>(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.WidgetTag){}[0]
+    constructor <init>(app.cash.redwood.protocol/Id) // app.cash.redwood.protocol.host/ProtocolNode.<init>|<init>(app.cash.redwood.protocol.Id){}[0]
 
     abstract val widget // app.cash.redwood.protocol.host/ProtocolNode.widget|{}widget[0]
         abstract fun <get-widget>(): app.cash.redwood.widget/Widget<#A> // app.cash.redwood.protocol.host/ProtocolNode.widget.<get-widget>|<get-widget>(){}[0]
-    final val widgetTag // app.cash.redwood.protocol.host/ProtocolNode.widgetTag|{}widgetTag[0]
-        final fun <get-widgetTag>(): app.cash.redwood.protocol/WidgetTag // app.cash.redwood.protocol.host/ProtocolNode.widgetTag.<get-widgetTag>|<get-widgetTag>(){}[0]
+    abstract val widgetTag // app.cash.redwood.protocol.host/ProtocolNode.widgetTag|{}widgetTag[0]
+        abstract fun <get-widgetTag>(): app.cash.redwood.protocol/WidgetTag // app.cash.redwood.protocol.host/ProtocolNode.widgetTag.<get-widgetTag>|<get-widgetTag>(){}[0]
 
     final var id // app.cash.redwood.protocol.host/ProtocolNode.id|{}id[0]
         final fun <get-id>(): app.cash.redwood.protocol/Id // app.cash.redwood.protocol.host/ProtocolNode.id.<get-id>|<get-id>(){}[0]
@@ -49,8 +49,8 @@ abstract class <#A: kotlin/Any> app.cash.redwood.protocol.host/ProtocolNode { //
     abstract fun children(app.cash.redwood.protocol/ChildrenTag): app.cash.redwood.protocol.host/ProtocolChildren<#A>? // app.cash.redwood.protocol.host/ProtocolNode.children|children(app.cash.redwood.protocol.ChildrenTag){}[0]
     abstract fun detach() // app.cash.redwood.protocol.host/ProtocolNode.detach|detach(){}[0]
     abstract fun toString(): kotlin/String // app.cash.redwood.protocol.host/ProtocolNode.toString|toString(){}[0]
-    abstract fun visitIds(kotlin/Function1<app.cash.redwood.protocol/Id, kotlin/Unit>) // app.cash.redwood.protocol.host/ProtocolNode.visitIds|visitIds(kotlin.Function1<app.cash.redwood.protocol.Id,kotlin.Unit>){}[0]
     final fun updateModifier(app.cash.redwood/Modifier) // app.cash.redwood.protocol.host/ProtocolNode.updateModifier|updateModifier(app.cash.redwood.Modifier){}[0]
+    open fun visitIds(kotlin/Function1<app.cash.redwood.protocol/Id, kotlin/Unit>) // app.cash.redwood.protocol.host/ProtocolNode.visitIds|visitIds(kotlin.Function1<app.cash.redwood.protocol.Id,kotlin.Unit>){}[0]
 }
 
 final class <#A: kotlin/Any> app.cash.redwood.protocol.host/HostProtocolAdapter : app.cash.redwood.protocol/ChangesSink { // app.cash.redwood.protocol.host/HostProtocolAdapter|null[0]

diff --git a/...protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt b/...protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt
@@ -387,8 +387,10 @@ public class HostProtocolAdapter<W : Any>(
 @OptIn(RedwoodCodegenApi::class)
 private class RootProtocolNode<W : Any>(
   children: Widget.Children<W>,
-) : ProtocolNode<W>(Id.Root, UnknownWidgetTag),
+) : ProtocolNode<W>(Id.Root),
   Widget<W> {
+  override val widgetTag: WidgetTag get() = UnknownWidgetTag
+
   private val children = ProtocolChildren(children)
 
   override fun apply(change: PropertyChange, eventSink: UiEventSink) {

diff --git a/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/ProtocolNode.kt b/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/ProtocolNode.kt
@@ -34,8 +34,9 @@ import kotlin.math.min
 public abstract class ProtocolNode<W : Any>(
   /** Updated in place when a node is reused due to pooling. */
   public var id: Id,
-  public val widgetTag: WidgetTag,
 ) {
+  public abstract val widgetTag: WidgetTag
+
   public abstract val widget: Widget<W>
 
   /** The index of [widget] within its parent [container]. */
@@ -65,7 +66,9 @@ public abstract class ProtocolNode<W : Any>(
   public abstract fun children(tag: ChildrenTag): ProtocolChildren<W>?
 
   /** Recursively visit IDs in this widget's tree, starting with this widget's [id]. */
-  public abstract fun visitIds(block: (Id) -> Unit)
+  public open fun visitIds(block: (Id) -> Unit) {
+    block(id)
+  }
 
   /**
    * Detach all child widgets recursively, then clear direct references to them.

diff --git a/...otocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/ChildrenNodeIndexTest.kt b/...otocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/ChildrenNodeIndexTest.kt
@@ -126,7 +126,9 @@ class ChildrenNodeIndexTest {
 }
 
 @OptIn(RedwoodCodegenApi::class)
-private class WidgetNode(override val widget: StringWidget) : ProtocolNode<String>(Id(1), WidgetTag(1)) {
+private class WidgetNode(override val widget: StringWidget) : ProtocolNode<String>(Id(1)) {
+  override val widgetTag: WidgetTag get() = WidgetTag(1)
+
   override fun apply(change: PropertyChange, eventSink: UiEventSink) {
     throw UnsupportedOperationException()
   }
@@ -135,9 +137,6 @@ private class WidgetNode(override val widget: StringWidget) : ProtocolNode<Strin
     throw UnsupportedOperationException()
   }
 
-  override fun visitIds(block: (Id) -> Unit) {
-  }
-
   override fun detach() {
   }
 

diff --git a/...ooling-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/protocolHostGeneration.kt b/...ooling-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/protocolHostGeneration.kt
@@ -253,7 +253,9 @@ internal class ProtocolButton<W : Any>(
   widget: Button<W>,
   private val json: Json,
   private val mismatchHandler: ProtocolMismatchHandler,
-) : ProtocolNode<W>(id, WidgetTag(4)) {
+) : ProtocolNode<W>(id) {
+  override val widgetTag: WidgetTag get() = WidgetTag(4)
+
   private var _widget: Button<W>? = widget
   override val widget: Widget<W> get() = _widget ?: error("detached")
 
@@ -319,7 +321,15 @@ internal fun generateProtocolNode(
             .build(),
         )
         .addSuperclassConstructorParameter("id")
-        .addSuperclassConstructorParameter("%T(%L)", WidgetTag, widget.tag)
+        .addProperty(
+          PropertySpec.builder("widgetTag", WidgetTag, OVERRIDE)
+            .getter(
+              FunSpec.getterBuilder()
+                .addStatement("return %T(%L)", WidgetTag, widget.tag)
+                .build(),
+            )
+            .build(),
+        )
         .addProperty(
           PropertySpec.builder("_widget", widgetType.copy(nullable = true), PRIVATE)
             .mutable(true)
@@ -487,20 +497,24 @@ internal fun generateProtocolNode(
             }
             .build(),
         )
-        .addFunction(
-          FunSpec.builder("visitIds")
-            .addModifiers(OVERRIDE)
-            .addParameter("block", LambdaTypeName.get(null, Id, returnType = UNIT))
-            .addStatement("block(id)")
-            .apply {
-              for (trait in widget.traits) {
-                if (trait is ProtocolChildren) {
-                  addStatement("%N.visitIds(block)", trait.name)
+        .apply {
+          if (childrens.isNotEmpty()) {
+            addFunction(
+              FunSpec.builder("visitIds")
+                .addModifiers(OVERRIDE)
+                .addParameter("block", LambdaTypeName.get(null, Id, returnType = UNIT))
+                .addStatement("block(id)")
+                .apply {
+                  for (trait in widget.traits) {
+                    if (trait is ProtocolChildren) {
+                      addStatement("%N.visitIds(block)", trait.name)
+                    }
+                  }
                 }
-              }
-            }
-            .build(),
-        )
+                .build(),
+            )
+          }
+        }
         .addFunction(
           FunSpec.builder("detach")
             .addModifiers(OVERRIDE)
@@ -525,7 +539,7 @@ internal fun generateProtocolNode(
             .addStatement("""append("(id=")""")
             .addStatement("append(id.value)")
             .addStatement("""append(", tag=")""")
-            .addStatement("append(widgetTag.value)")
+            .addStatement("append(%L)", widget.tag)
             .addStatement("append(')')")
             .endControlFlow()
             .build(),

diff --git a/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeProtocolNode.kt b/redwood-treehouse-host/src/commonTest/kotlin/app/cash/redwood/treehouse/FakeProtocolNode.kt
@@ -33,8 +33,8 @@ import kotlinx.serialization.json.JsonPrimitive
 @RedwoodCodegenApi
 internal class FakeProtocolNode(
   id: Id,
-  tag: WidgetTag,
-) : ProtocolNode<FakeWidget>(id, tag) {
+  override val widgetTag: WidgetTag,
+) : ProtocolNode<FakeWidget>(id) {
   override val widget = FakeWidget()
 
   override fun apply(change: PropertyChange, eventSink: UiEventSink) {