Move bindCompose lambda parameters into WorkflowRenderingScope

[zach-klippenstein]

Right now, the function passed to bindCompose takes two parameters: rendering and ViewEnvironment. A lot of view factories don't actually need the ViewEnvironment though, so end up looking like bindCompose<Foo> { rendering, _ ->. Always needing to specify that extra underscore is an unfortunate inconvenience.

There are three ways to solve this:

1. Add an overload that takes a function with a single rendering parameter. Eg bindCompose<Foo> { rendering ->
2. Move both parameters into a new WorkflowRenderingScope type. Eg

class WorkflowRenderingScope<R>(
  val rendering: R,
  val viewEnvironment: ViewEnvironment
)
bindCompose<Foo> { /* this: WorkflowRenderingScope<Foo> */

3. Mix of 1 and 2: only 1 function, but always put the view env in the scope. Leave the rendering as a parameter.

1 is more consistent with the best practice adopted by the core workflow library of avoiding passing data arguments (ie not types there solely to hang methods off of) as lambda receivers because they're not discoverable enough. 2 also mostly aligns with this, since the view env is kind of a secondary/auxiliary argument - not the main object of the function and often ignored. However, 3 is more consistent with Compose APIs, namely CanvasScope, DrawScope, ContentDrawScope. Some of these APIs used to be more like 2, but recently were refactored to be entirely receiver-based.

As this library sits right in between Workflow and Compose, which style of API should we be consistent with?

This question also applies to ComposeWorkflow.render and Workflow.Companion.composed. In the core workflow library, the abstract methods use option 1 (explicit params, no receiver) and the convenience builders are more like 2 (one param is in the receiver position, but no separate scope type).

[zach-klippenstein]

I think I am leaning towards option 3 for bindCompose.

I like 3 better than 2 since it allows the caller to call rendering whatever they want, and it may be useful for readability to give it a more specific name. And for really simple cases, it also allows just using it. I like 2 better than 1 because there's only a single function (no overloads) and it lets us move the showRendering extension to the scope type, which feels a lot more consistent with similar Compose APIs.

Edit: I actually meant 3, not 2. That's what I get for trying to type my thoughts coherently at 3 in the morning.

So just to be extra clear:

bindCompose<Foo> { Text(it.toString()) }

bindCompose<Bar> { bar ->
  val borderColor = viewEnvironment[HighlightColor]!!
  showRendering(
    bar.child, 
    Modifier.drawBorder(1.dp, borderColor)
  )
}

Where showRendering is now defined on WorkflowRenderingScope.

[zach-klippenstein]

For ComposeWorkflow, I think props should stay as a parameter in both cases (abstract method & lambda builder), but the sink and view env should move to a ComposeRenderContext type (to mirror RenderContext). This context type could extend the WorkflowRenderingScope type from bindCompose to automatically get any extensions defined there.

The lambda convenience builder should take the CRC as a receiver. The abstract render method, I am not sure if it should be a receiver or a parameter. We've gotten specific feedback for StatefulWorkflow/StatelessWorkflow classes that people prefer everything to be params, no receiver. However, it's more common for Compose APIs to take this stuff as a receiver even in abstract methods, so it might be good to be more consistent with that.

So for example for the convenience builder:

object Clicked
Workflow.composed<Foo, Clicked> { foo ->
  Button(
    color = viewEnvironment[ButtonColor]!!
    onClick = { sendOutput(Clicked) }
  ) {
    showRendering (foo.text)
  }
}