bindShowRendering no longer calls showRendering.

workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/BackStackContainerTest.kt

@@ -21,7 +21,7 @@ import org.junit.runner.RunWith
 
 @OptIn(WorkflowUiExperimentalApi::class)
 @RunWith(AndroidJUnit4::class)
-class BackStackContainerTest {
+internal class BackStackContainerTest {
 
   @Rule @JvmField val scenarioRule = ActivityScenarioRule(BackStackTestActivity::class.java)
   private val scenario get() = scenarioRule.scenario

workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/BackStackTestActivity.kt

@@ -25,7 +25,7 @@ import com.squareup.workflow1.ui.showRendering
  * [onRetainNonConfigurationInstance].
  */
 @OptIn(WorkflowUiExperimentalApi::class)
-class BackStackTestActivity : Activity() {
+internal class BackStackTestActivity : Activity() {
 
   /**
    * A simple string holder that creates [ViewStateTestView]s with their ID and tag derived from
@@ -35,7 +35,7 @@ class BackStackTestActivity : Activity() {
    * @param onViewCreated An optional function that will be called by the view factory after the
    * view is created but before [bindShowRendering].
    */
-  class TestRendering(
+  internal class TestRendering(
     val name: String,
     val onViewCreated: (ViewStateTestView) -> Unit = {},
     val onShowRendering: (ViewStateTestView) -> Unit = {},
@@ -96,6 +96,7 @@ class BackStackTestActivity : Activity() {
     check(backstackContainer == null)
     backstackContainer =
       NoTransitionBackStackContainer.buildView(backstack!!, viewEnvironment, this)
+    backstackContainer!!.showRendering(backstack!!, viewEnvironment)
     setContentView(backstackContainer)
   }
 

workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/NoTransitionBackStackContainer.kt

@@ -16,7 +16,7 @@ import com.squareup.workflow1.ui.bindShowRendering
  * actual backstack logic. Views are just swapped instantly.
  */
 @OptIn(WorkflowUiExperimentalApi::class)
-class NoTransitionBackStackContainer(context: Context) : BackStackContainer(context) {
+internal class NoTransitionBackStackContainer(context: Context) : BackStackContainer(context) {
 
   override fun performTransition(oldViewMaybe: View?, newView: View, popped: Boolean) {
     oldViewMaybe?.let(::removeView)

workflow-ui/backstack-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt

@@ -66,7 +66,12 @@ public open class BackStackContainer @JvmOverloads constructor(
           return
         }
 
-    val newView = environment[ViewRegistry].buildView(named.top, environment, this)
+    val newView = environment[ViewRegistry].buildView(
+      initialRendering = named.top,
+      initialViewEnvironment = environment,
+      contextForNewView = this.context,
+      container = this
+    )
     viewStateCache.update(named.backStack, oldViewMaybe, newView)
 
     val popped = currentRendering?.backStack?.any { compatible(it, named.top) } == true

workflow-ui/core-android/api/core-android.api

@@ -127,9 +127,9 @@ public final class com/squareup/workflow1/ui/ViewRegistry$Companion : com/square
 public final class com/squareup/workflow1/ui/ViewRegistryKt {
 	public static final fun ViewRegistry ()Lcom/squareup/workflow1/ui/ViewRegistry;
 	public static final fun ViewRegistry ([Lcom/squareup/workflow1/ui/ViewFactory;)Lcom/squareup/workflow1/ui/ViewRegistry;
-	public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Landroid/view/View;
-	public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/view/ViewGroup;)Landroid/view/View;
-	public static synthetic fun buildView$default (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;ILjava/lang/Object;)Landroid/view/View;
+	public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lkotlin/jvm/functions/Function1;)Landroid/view/View;
+	public static synthetic fun buildView$default (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Landroid/view/View;
+	public static final fun getFactoryForRendering (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;)Lcom/squareup/workflow1/ui/ViewFactory;
 	public static final fun plus (Lcom/squareup/workflow1/ui/ViewRegistry;Lcom/squareup/workflow1/ui/ViewFactory;)Lcom/squareup/workflow1/ui/ViewRegistry;
 	public static final fun plus (Lcom/squareup/workflow1/ui/ViewRegistry;Lcom/squareup/workflow1/ui/ViewRegistry;)Lcom/squareup/workflow1/ui/ViewRegistry;
 }

workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/DecorativeViewFactoryTest.kt

@@ -8,7 +8,7 @@ import com.google.common.truth.Truth.assertThat
 import org.junit.Test
 
 @OptIn(WorkflowUiExperimentalApi::class)
-class DecorativeViewFactoryTest {
+internal class DecorativeViewFactoryTest {
 
   private val instrumentation = InstrumentationRegistry.getInstrumentation()
 
@@ -28,11 +28,19 @@ class DecorativeViewFactoryTest {
         }
       }
     }
+
+    val envString = object : ViewEnvironmentKey<String>(String::class) {
+      override val default: String get() = "Not set"
+    }
+
     val outerViewFactory = DecorativeViewFactory(
       type = OuterRendering::class,
-      map = { outer -> outer.wrapped },
-      initView = { outerRendering, _ ->
-        events += "initView $outerRendering"
+      map = { outer, env ->
+        val enhancedEnv = env + (envString to "Updated environment")
+        Pair(outer.wrapped, enhancedEnv)
+      },
+      initView = { outerRendering, view ->
+        events += "initView $outerRendering ${view.environment!![envString]}"
       }
     )
     val viewRegistry = ViewRegistry(innerViewFactory)
@@ -44,11 +52,9 @@ class DecorativeViewFactoryTest {
       instrumentation.context
     )
 
-    // Note that showRendering is called twice. Technically this behavior is not incorrect, although
-    // it's not necessary. Fix coming soon.
     assertThat(events).containsExactly(
-      "inner showRendering InnerRendering(innerData=inner)",
-      "initView OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner))",
+      "initView OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " +
+        "Updated environment",
       "inner showRendering InnerRendering(innerData=inner)"
     )
   }
@@ -86,10 +92,7 @@ class DecorativeViewFactoryTest {
       instrumentation.context
     )
 
-    // Note that showRendering is called twice. Technically this behavior is not incorrect, although
-    // it's not necessary. Fix coming soon.
     assertThat(events).containsExactly(
-      "inner showRendering InnerRendering(innerData=inner)",
       "doShowRendering OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner))",
       "inner showRendering InnerRendering(innerData=inner)"
     )

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/DecorativeViewFactory.kt

@@ -137,10 +137,10 @@ public class DecorativeViewFactory<OuterT : Any, InnerT : Any>(
       innerShowRendering(map(outerRendering), viewEnvironment)
     }
   ) : this(
-      type,
-      map = { outer, viewEnvironment -> Pair(map(outer), viewEnvironment) },
-      initView = initView,
-      doShowRendering = doShowRendering
+    type,
+    map = { outer, viewEnvironment -> Pair(map(outer), viewEnvironment) },
+    initView = initView,
+    doShowRendering = doShowRendering
   )
 
   override fun buildView(
@@ -152,18 +152,27 @@ public class DecorativeViewFactory<OuterT : Any, InnerT : Any>(
     val (innerInitialRendering, processedInitialEnv) = map(initialRendering, initialViewEnvironment)
 
     return processedInitialEnv[ViewRegistry]
-        .buildView(
-            innerInitialRendering,
-            processedInitialEnv,
-            contextForNewView,
-            container
-        )
-        .also { view ->
-          val innerShowRendering: ViewShowRendering<InnerT> = view.getShowRendering()!!
-          initView(initialRendering, view)
-          view.bindShowRendering(initialRendering, processedInitialEnv) { rendering, env ->
-            doShowRendering(view, innerShowRendering, rendering, env)
-          }
-        }
+      .buildView(
+        innerInitialRendering,
+        processedInitialEnv,
+        contextForNewView,
+        container,
+        // Don't call showRendering yet, we need to wrap the function first.
+        initializeView = { }
+      )
+      .also { view ->
+        val innerShowRendering: ViewShowRendering<InnerT> = view.getShowRendering()!!
+        view.bindShowRendering(
+          initialRendering,
+          processedInitialEnv
+        ) { rendering, env -> doShowRendering(view, innerShowRendering, rendering, env) }
+
+        // Call this after we fuss with the bindings, to ensure it can pull the updated
+        // ViewEnvironment.
+        initView(initialRendering, view)
+
+        // Our showRendering wrapper is in place, now call it.
+        view.showRendering(initialRendering, processedInitialEnv)
+      }
   }
 }

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunner.kt

@@ -6,7 +6,6 @@ import android.view.View
 import android.view.ViewGroup
 import androidx.annotation.LayoutRes
 import androidx.viewbinding.ViewBinding
-import com.squareup.workflow1.ui.LayoutRunner.Companion.bind
 
 @WorkflowUiExperimentalApi
 public typealias ViewBindingInflater<BindingT> = (LayoutInflater, ViewGroup?, Boolean) -> BindingT
@@ -93,16 +92,10 @@ public fun interface LayoutRunner<RenderingT : Any> {
      * Creates a [ViewFactory] that inflates [layoutId] to "show" renderings of type [RenderingT],
      * with a no-op [LayoutRunner]. Handy for showing static views, e.g. when prototyping.
      */
+    @Suppress("unused")
     public inline fun <reified RenderingT : Any> bindNoRunner(
       @LayoutRes layoutId: Int
-    ): ViewFactory<RenderingT> = bind(layoutId) {
-      object : LayoutRunner<RenderingT> {
-        override fun showRendering(
-          rendering: RenderingT,
-          viewEnvironment: ViewEnvironment
-        ) = Unit
-      }
-    }
+    ): ViewFactory<RenderingT> = bind(layoutId) { LayoutRunner { _, _ -> } }
   }
 }
 

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunnerViewFactory.kt

@@ -25,13 +25,12 @@ internal class LayoutRunnerViewFactory<RenderingT : Any>(
     container: ViewGroup?
   ): View {
     return contextForNewView.viewBindingLayoutInflater(container)
-        .inflate(layoutId, container, false)
-        .apply {
-          bindShowRendering(
-              initialRendering,
-              initialViewEnvironment,
-              runnerConstructor.invoke(this)::showRendering
-          )
+      .inflate(layoutId, container, false)
+      .also { view ->
+        val runner = runnerConstructor(view)
+        view.bindShowRendering(initialRendering, initialViewEnvironment) { rendering, environment ->
+          runner.showRendering(rendering, environment)
         }
+      }
   }
 }

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewBindingViewFactory.kt

@@ -20,12 +20,14 @@ internal class ViewBindingViewFactory<BindingT : ViewBinding, RenderingT : Any>(
     container: ViewGroup?
   ): View =
     bindingInflater(contextForNewView.viewBindingLayoutInflater(container), container, false)
-        .also { binding ->
-          binding.root.bindShowRendering(
-              initialRendering,
-              initialViewEnvironment,
-              runnerConstructor.invoke(binding)::showRendering
-          )
+      .also { binding ->
+        val runner = runnerConstructor(binding)
+        binding.root.bindShowRendering(
+          initialRendering,
+          initialViewEnvironment
+        ) { rendering, environment ->
+          runner.showRendering(rendering, environment)
         }
-        .root
+      }
+      .root
 }

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewRegistry.kt

@@ -98,55 +98,45 @@ public fun ViewRegistry(vararg bindings: ViewFactory<*>): ViewRegistry =
 public fun ViewRegistry(): ViewRegistry = TypedViewRegistry()
 
 /**
- * It is usually more convenient to use [WorkflowViewStub] than to call this method directly.
+ * It is usually more convenient to use [WorkflowViewStub] or [DecorativeViewFactory]
+ * than to call this method directly.
  *
- * Creates a [View] to display [initialRendering], which can be updated via calls
- * to [View.showRendering]. Prefers entries found via [ViewRegistry.getFactoryFor].
- * If that returns null, falls back to the factory provided by the rendering's
- * implementation of [AndroidViewRendering.viewFactory], if there is one.
+ * Returns the [ViewFactory] that builds [View] instances suitable to display the given [rendering],
+ * via subsequent calls to [View.showRendering].
  *
- * @throws IllegalArgumentException if no factory can be find for type [RenderingT]
+ * Prefers factories found via [ViewRegistry.getFactoryFor]. If that returns null, falls
+ * back to the factory provided by the rendering's implementation of
+ * [AndroidViewRendering.viewFactory], if there is one. Note that this means that a
+ * compile time [AndroidViewRendering.viewFactory] binding can be overridden at runtime.
  *
- * @throws IllegalStateException if the matching [ViewFactory] fails to call
- * [View.bindShowRendering] when constructing the view
+ * @throws IllegalArgumentException if no factory can be find for type [RenderingT]
  */
 @WorkflowUiExperimentalApi
-public fun <RenderingT : Any> ViewRegistry.buildView(
-  initialRendering: RenderingT,
-  initialViewEnvironment: ViewEnvironment,
-  contextForNewView: Context,
-  container: ViewGroup? = null
-): View {
+public fun <RenderingT : Any>
+  ViewRegistry.getFactoryForRendering(rendering: RenderingT): ViewFactory<RenderingT> {
   @Suppress("UNCHECKED_CAST")
-  val factory: ViewFactory<RenderingT> = getFactoryFor(initialRendering::class)
-    ?: (initialRendering as? AndroidViewRendering<*>)?.viewFactory as? ViewFactory<RenderingT>
+  return getFactoryFor(rendering::class)
+    ?: (rendering as? AndroidViewRendering<*>)?.viewFactory as? ViewFactory<RenderingT>
     ?: throw IllegalArgumentException(
       "A ${ViewFactory::class.qualifiedName} should have been registered to display " +
-        "${initialRendering::class.qualifiedName} instances, or that class should implement " +
-        "${AndroidViewRendering::class.simpleName}<${initialRendering::class.simpleName}>."
+        "${rendering::class.qualifiedName} instances, or that class should implement " +
+        "${AndroidViewRendering::class.simpleName}<${rendering::class.simpleName}>."
     )
-
-  return factory.buildView(
-    initialRendering,
-    initialViewEnvironment,
-    contextForNewView,
-    container
-  )
-    .apply {
-      check(this.getRendering<Any>() != null) {
-        "View.bindShowRendering should have been called for $this, typically by the " +
-          "${ViewFactory::class.java.name} that created it."
-      }
-    }
 }
 
 /**
- * It is usually more convenient to use [WorkflowViewStub] than to call this method directly.
+ * It is usually more convenient to use [WorkflowViewStub] or [DecorativeViewFactory]
+ * than to call this method directly.
+ *
+ * Finds a [ViewFactory] to create a [View] to display [initialRendering]. The new view
+ * can be updated via calls to [View.showRendering] -- that is, it is guaranteed that
+ * [bindShowRendering] has been called on this view.
  *
- * Creates a [View] to display [initialRendering], which can be updated via calls
- * to [View.showRendering].
+ * @param initializeView Optional function invoked immediately after the [View] is
+ * created (that is, immediately after the call to [ViewFactory.buildView]). Defaults
+ * to a call to [View.showRendering].
  *
- * @throws IllegalArgumentException if no binding can be find for type [RenderingT]
+ * @throws IllegalArgumentException if no factory can be find for type [RenderingT]
  *
  * @throws IllegalStateException if the matching [ViewFactory] fails to call
  * [View.bindShowRendering] when constructing the view
@@ -155,8 +145,23 @@ public fun <RenderingT : Any> ViewRegistry.buildView(
 public fun <RenderingT : Any> ViewRegistry.buildView(
   initialRendering: RenderingT,
   initialViewEnvironment: ViewEnvironment,
-  container: ViewGroup
-): View = buildView(initialRendering, initialViewEnvironment, container.context, container)
+  contextForNewView: Context,
+  container: ViewGroup? = null,
+  initializeView: View.() -> Unit = {
+    showRendering(getRendering<RenderingT>()!!, environment!!)
+  }
+): View {
+  return getFactoryForRendering(initialRendering).buildView(
+    initialRendering, initialViewEnvironment, contextForNewView, container
+  ).also { view ->
+    checkNotNull(view.showRenderingTag) {
+      "View.bindShowRendering should have been called for $view, typically by the " +
+        "${ViewFactory::class.java.name} that created it."
+    }
+    @Suppress("UNCHECKED_CAST")
+    initializeView.invoke(view)
+  }
+}
 
 @WorkflowUiExperimentalApi
 public operator fun ViewRegistry.plus(binding: ViewFactory<*>): ViewRegistry =

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewShowRendering.kt

@@ -25,13 +25,13 @@ public data class ShowRenderingTag<out RenderingT : Any>(
 )
 
 /**
- * It is usually more convenient to use [WorkflowViewStub] than to call this method directly.
- *
  * Establishes [showRendering] as the implementation of [View.showRendering]
- * for the receiver, possibly replacing the existing one. Immediately invokes [showRendering]
- * to display [initialRendering].
+ * for the receiver, possibly replacing the existing one. Likewise sets / updates
+ * the values returned by [View.getRendering] and [View.environment].
  *
  * Intended for use by implementations of [ViewFactory.buildView].
+ *
+ * @see DecorativeViewFactory
  */
 @WorkflowUiExperimentalApi
 public fun <RenderingT : Any> View.bindShowRendering(
@@ -43,7 +43,6 @@ public fun <RenderingT : Any> View.bindShowRendering(
     R.id.view_show_rendering_function,
     ShowRenderingTag(initialRendering, initialViewEnvironment, showRendering)
   )
-  showRendering.invoke(initialRendering, initialViewEnvironment)
 }
 
 /**
@@ -81,7 +80,10 @@ public fun <RenderingT : Any> View.showRendering(
           "Consider using ${WorkflowViewStub::class.java.simpleName} to display arbitrary types."
       }
 
+      // Update the tag's rendering and viewEnvironment.
       bindShowRendering(rendering, viewEnvironment, tag.showRendering)
+      // And do the actual showRendering work.
+      tag.showRendering.invoke(rendering, viewEnvironment)
     }
     ?: error(
       "Expected $this to have a showRendering function to show $rendering. " +