Repository: cashapp/redwood
Pull request #533 (2022-11-21 20:17:04)

Eliminate specialized `ProtocolRedwoodComposition` type
We can accept the `DiffSink` in the factory function now that the `EventSink` is created before the function can be invoked. That means the function now launches the composition rather than needing an explicit start call.

Refs #14. Getting close!


Patch:
diff --git a/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolRedwoodComposition.kt b/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolRedwoodComposition.kt
index c1c29dccb..d4ed4ea46 100644
--- a/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolRedwoodComposition.kt
+++ b/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolRedwoodComposition.kt
@@ -21,7 +21,6 @@ import androidx.compose.runtime.InternalComposeApi
 import androidx.compose.runtime.MonotonicFrameClock
 import androidx.compose.runtime.Recomposer
 import androidx.compose.runtime.currentComposer
-import androidx.compose.runtime.snapshots.ObserverHandle
 import androidx.compose.runtime.snapshots.Snapshot
 import app.cash.redwood.compose.LocalWidgetVersion
 import app.cash.redwood.compose.RedwoodComposition
@@ -33,10 +32,6 @@ import kotlinx.coroutines.CoroutineStart.UNDISPATCHED
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.launch
 
-public interface ProtocolRedwoodComposition : RedwoodComposition {
-  public fun start(diffSink: DiffSink)
-}
-
 /**
  * @param scope A [CoroutineScope] whose [coroutineContext][kotlin.coroutines.CoroutineContext]
  * must have a [MonotonicFrameClock] key which is being ticked.
@@ -44,59 +39,44 @@ public interface ProtocolRedwoodComposition : RedwoodComposition {
 public fun ProtocolRedwoodComposition(
   scope: CoroutineScope,
   factory: DiffProducingWidget.Factory,
+  diffSink: DiffSink,
   widgetVersion: UInt,
-): ProtocolRedwoodComposition {
-  return DiffProducingRedwoodComposition(scope, factory, widgetVersion)
+): RedwoodComposition {
+  val bridge = factory.bridge
+  val root = DiffProducingWidgetChildren(Id.Root, ChildrenDiff.RootChildrenTag, bridge)
+  val applier = ProtocolApplier(factory, root) {
+    bridge.createDiffOrNull()?.let(diffSink::sendDiff)
+  }
+  return DiffProducingRedwoodComposition(scope, applier, widgetVersion)
 }
 
 private class DiffProducingRedwoodComposition(
   private val scope: CoroutineScope,
-  private val factory: DiffProducingWidget.Factory,
+  applier: ProtocolApplier,
   private val widgetVersion: UInt,
-) : ProtocolRedwoodComposition {
+) : RedwoodComposition {
   private val recomposer = Recomposer(scope.coroutineContext)
+  private val composition = Composition(applier, recomposer)
 
-  private lateinit var applier: ProtocolApplier
-  private lateinit var composition: Composition
-
-  private lateinit var snapshotHandle: ObserverHandle
   private var snapshotJob: Job? = null
-  private lateinit var recomposeJob: Job
-
-  override fun start(diffSink: DiffSink) {
-    check(!this::applier.isInitialized) { "display already initialized" }
-
-    val bridge = factory.bridge
-    val root = DiffProducingWidgetChildren(Id.Root, ChildrenDiff.RootChildrenTag, bridge)
-    applier = ProtocolApplier(factory, root) {
-      bridge.createDiffOrNull()?.let(diffSink::sendDiff)
-    }
-    composition = Composition(applier, recomposer)
-
+  private val snapshotHandle = Snapshot.registerGlobalWriteObserver {
     // Set up a trigger to apply changes on the next frame if a global write was observed.
-    // TODO where should this live?
-    var applyScheduled = false
-    snapshotHandle = Snapshot.registerGlobalWriteObserver {
-      if (!applyScheduled) {
-        applyScheduled = true
-        snapshotJob = scope.launch {
-          applyScheduled = false
-          Snapshot.sendApplyNotifications()
-        }
+    if (snapshotJob == null) {
+      snapshotJob = scope.launch {
+        snapshotJob = null
+        Snapshot.sendApplyNotifications()
       }
     }
+  }
 
-    // These launch undispatched so that they reach their first suspension points before returning
-    // control to the caller.
-    recomposeJob = scope.launch(start = UNDISPATCHED) {
-      recomposer.runRecomposeAndApplyChanges()
-    }
+  // These launch undispatched so that they reach their first suspension points before returning
+  // control to the caller.
+  private val recomposeJob = scope.launch(start = UNDISPATCHED) {
+    recomposer.runRecomposeAndApplyChanges()
   }
 
   @OptIn(InternalComposeApi::class) // See internal function comment below.
   override fun setContent(content: @Composable () -> Unit) {
-    check(this::applier.isInitialized) { "display not initialized" }
-
     // TODO using CompositionLocalProvider fails to link in release mode with:
     //  inlinable function call in a function with debug info must have a !dbg location
     //    %16 = call i32 @"kfun:kotlin.Array#<get-size>(){}kotlin.Int"(%struct.ObjHeader* %15)
diff --git a/redwood-treehouse/src/jsMain/kotlin/app/cash/redwood/treehouse/treehouseCompose.kt b/redwood-treehouse/src/jsMain/kotlin/app/cash/redwood/treehouse/treehouseCompose.kt
index 50b82149f..127db509a 100644
--- a/redwood-treehouse/src/jsMain/kotlin/app/cash/redwood/treehouse/treehouseCompose.kt
+++ b/redwood-treehouse/src/jsMain/kotlin/app/cash/redwood/treehouse/treehouseCompose.kt
@@ -19,6 +19,7 @@ import androidx.compose.runtime.BroadcastFrameClock
 import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
+import app.cash.redwood.compose.RedwoodComposition
 import app.cash.redwood.protocol.EventSink
 import app.cash.redwood.protocol.compose.DiffProducingWidget
 import app.cash.redwood.protocol.compose.ProtocolRedwoodComposition
@@ -36,29 +37,30 @@ public fun TreehouseUi.asZiplineTreehouseUi(
   factory: DiffProducingWidget.Factory,
   widgetVersion: UInt,
 ): ZiplineTreehouseUi {
-  val bridge = factory.bridge
-  val composition = ProtocolRedwoodComposition(
-    scope = scope + frameClock,
-    factory = factory,
-    widgetVersion = widgetVersion,
-  )
-  return RedwoodZiplineTreehouseUi(composition, this, bridge)
+  return RedwoodZiplineTreehouseUi(factory, widgetVersion, this)
 }
 
 private class RedwoodZiplineTreehouseUi(
-  private val composition: ProtocolRedwoodComposition,
+  private val factory: DiffProducingWidget.Factory,
+  private val widgetVersion: UInt,
   private val treehouseUi: TreehouseUi,
-  private val eventSink: EventSink,
-) : ZiplineTreehouseUi, EventSink by eventSink {
-  private var diffSinkToClose: DiffSinkService? = null
+) : ZiplineTreehouseUi, EventSink by factory.bridge {
+  private lateinit var diffSinkToClose: DiffSinkService
+  private lateinit var composition: RedwoodComposition
 
   override fun start(
     diffSink: DiffSinkService,
     hostConfigurations: FlowWithInitialValue<HostConfiguration>,
   ) {
-    check(diffSinkToClose == null)
+    check(::diffSinkToClose.isInitialized)
     diffSinkToClose = diffSink
-    composition.start(diffSink)
+
+    val composition = ProtocolRedwoodComposition(
+      scope = scope + frameClock,
+      factory = factory,
+      widgetVersion = widgetVersion,
+      diffSink = diffSink,
+    )
 
     val (initialHostConfiguration, hostConfigurationFlow) = hostConfigurations
     composition.setContent {
@@ -71,7 +73,7 @@ private class RedwoodZiplineTreehouseUi(
 
   override fun close() {
     composition.cancel()
-    diffSinkToClose?.close()
+    diffSinkToClose.close()
   }
 }
 
diff --git a/samples/counter/android-composeui/src/main/java/example/android/MainActivity.kt b/samples/counter/android-composeui/src/main/java/example/android/MainActivity.kt
index a87cda0e3..205a6bfab 100644
--- a/samples/counter/android-composeui/src/main/java/example/android/MainActivity.kt
+++ b/samples/counter/android-composeui/src/main/java/example/android/MainActivity.kt
@@ -52,11 +52,6 @@ class MainActivity : AppCompatActivity() {
     val composeChildren = ComposeWidgetChildren()
 
     val composeBridge = ComposeProtocolBridge()
-    val composition = ProtocolRedwoodComposition(
-      scope = scope,
-      factory = DiffProducingSunspotWidgetFactory(composeBridge),
-      widgetVersion = 1U,
-    )
 
     val factory = DiffConsumingSunspotWidgetFactory(AndroidSunspotWidgetFactory())
     val widgetBridge = ProtocolBridge(
@@ -65,7 +60,12 @@ class MainActivity : AppCompatActivity() {
       eventSink = composeBridge,
     )
 
-    composition.start(widgetBridge)
+    val composition = ProtocolRedwoodComposition(
+      scope = scope,
+      factory = DiffProducingSunspotWidgetFactory(composeBridge),
+      diffSink = widgetBridge,
+      widgetVersion = 1U,
+    )
     composition.setContent(content)
 
     return composeChildren
diff --git a/samples/counter/android-views/src/main/java/example/android/MainActivity.kt b/samples/counter/android-views/src/main/java/example/android/MainActivity.kt
index 3d0c76e23..2cb5670ec 100644
--- a/samples/counter/android-views/src/main/java/example/android/MainActivity.kt
+++ b/samples/counter/android-views/src/main/java/example/android/MainActivity.kt
@@ -39,11 +39,6 @@ class MainActivity : AppCompatActivity() {
     super.onCreate(savedInstanceState)
 
     val composeBridge = ComposeProtocolBridge()
-    val composition = ProtocolRedwoodComposition(
-      scope = scope,
-      factory = DiffProducingSunspotWidgetFactory(composeBridge),
-      widgetVersion = 1U,
-    )
 
     val root = FrameLayout(this).apply {
       layoutParams = LayoutParams(MATCH_PARENT, MATCH_PARENT)
@@ -57,8 +52,12 @@ class MainActivity : AppCompatActivity() {
       eventSink = composeBridge,
     )
 
-    composition.start(widgetBridge)
-
+    val composition = ProtocolRedwoodComposition(
+      scope = scope,
+      factory = DiffProducingSunspotWidgetFactory(composeBridge),
+      diffSink = widgetBridge,
+      widgetVersion = 1U,
+    )
     composition.setContent {
       Counter()
     }
diff --git a/samples/counter/browser/src/main/kotlin/example/browser/main.kt b/samples/counter/browser/src/main/kotlin/example/browser/main.kt
index 9e4c0f987..afd55d42d 100644
--- a/samples/counter/browser/src/main/kotlin/example/browser/main.kt
+++ b/samples/counter/browser/src/main/kotlin/example/browser/main.kt
@@ -31,14 +31,7 @@ import kotlinx.coroutines.plus
 import org.w3c.dom.HTMLElement
 
 fun main() {
-  @OptIn(DelicateCoroutinesApi::class)
-  val scope = GlobalScope + WindowAnimationFrameClock
   val composeBridge = ComposeProtocolBridge()
-  val composition = ProtocolRedwoodComposition(
-    scope = scope,
-    factory = DiffProducingSunspotWidgetFactory(composeBridge),
-    widgetVersion = 1U,
-  )
 
   val content = document.getElementById("content")!! as HTMLElement
   val factory = DiffConsumingSunspotWidgetFactory(HtmlSunspotNodeFactory(document))
@@ -48,8 +41,14 @@ fun main() {
     eventSink = composeBridge,
   )
 
-  composition.start(widgetBridge)
-
+  @OptIn(DelicateCoroutinesApi::class)
+  val scope = GlobalScope + WindowAnimationFrameClock
+  val composition = ProtocolRedwoodComposition(
+    scope = scope,
+    factory = DiffProducingSunspotWidgetFactory(composeBridge),
+    diffSink = widgetBridge,
+    widgetVersion = 1U,
+  )
   composition.setContent {
     Counter()
   }
diff --git a/samples/counter/ios/shared/src/commonMain/kotlin/example/ios/CounterViewControllerDelegate.kt b/samples/counter/ios/shared/src/commonMain/kotlin/example/ios/CounterViewControllerDelegate.kt
index f9c024209..32f73ff4a 100644
--- a/samples/counter/ios/shared/src/commonMain/kotlin/example/ios/CounterViewControllerDelegate.kt
+++ b/samples/counter/ios/shared/src/commonMain/kotlin/example/ios/CounterViewControllerDelegate.kt
@@ -38,11 +38,6 @@ class CounterViewControllerDelegate(
 
   init {
     val composeBridge = ComposeProtocolBridge()
-    val composition = ProtocolRedwoodComposition(
-      scope = scope,
-      factory = DiffProducingSunspotWidgetFactory(composeBridge),
-      widgetVersion = 1U,
-    )
 
     val children = UIViewChildren(
       parent = root,
@@ -55,8 +50,12 @@ class CounterViewControllerDelegate(
       eventSink = composeBridge,
     )
 
-    composition.start(widgetBridge)
-
+    val composition = ProtocolRedwoodComposition(
+      scope = scope,
+      factory = DiffProducingSunspotWidgetFactory(composeBridge),
+      diffSink = widgetBridge,
+      widgetVersion = 1U,
+    )
     composition.setContent {
       Counter()
     }


Test patch:
diff --git a/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/ProtocolTest.kt b/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/ProtocolTest.kt
index 8df75b511..5de2dd34a 100644
--- a/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/ProtocolTest.kt
+++ b/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/ProtocolTest.kt
@@ -48,9 +48,9 @@ class ProtocolTest {
     val composition = ProtocolRedwoodComposition(
       scope = this + clock,
       factory = DiffProducingExampleSchemaWidgetFactory(ProtocolBridge()),
+      diffSink = ::error,
       widgetVersion = 22U,
     )
-    composition.start { }
 
     var actualDisplayVersion = 0U
     composition.setContent {
@@ -63,13 +63,13 @@ class ProtocolTest {
 
   @Test fun childrenInheritIdFromSyntheticParent() = runTest {
     val clock = BroadcastFrameClock()
+    val diffs = ArrayDeque<Diff>()
     val composition = ProtocolRedwoodComposition(
       scope = this + clock,
       factory = DiffProducingExampleSchemaWidgetFactory(ProtocolBridge()),
+      diffSink = { diff -> diffs += diff },
       widgetVersion = 1U,
     )
-    val diffs = ArrayDeque<Diff>()
-    composition.start { diff -> diffs += diff }
 
     composition.setContent {
       Row {
@@ -110,13 +110,13 @@ class ProtocolTest {
     val clock = BroadcastFrameClock()
     var state by mutableStateOf(0)
     val bridge = ProtocolBridge()
+    val diffs = ArrayDeque<Diff>()
     val composition = ProtocolRedwoodComposition(
       scope = this + clock,
       factory = DiffProducingExampleSchemaWidgetFactory(bridge),
+      diffSink = { diff -> diffs += diff },
       widgetVersion = 1U,
     )
-    val diffs = ArrayDeque<Diff>()
-    composition.start { diff -> diffs += diff }
 
     composition.setContent {
       Button(