square / workflow-kotlin

A Swift and Kotlin library for making composable state machines, and UIs driven by those state machines.
https://square.github.io/workflow
Apache License 2.0
1.02k stars 101 forks source link

Feature Request: Sugar for running simple workers and actions #75

Open digitalbuddha opened 4 years ago

digitalbuddha commented 4 years ago

Hi folks, I wrote an extension function run which tries to reduce some of the boilerplate associated with running workers. I find myself writing a bit more boilerplate that the minimum when creating a worker that changes state. I'd like the ability to pass a lambda to runningWorker that is an Action.Updater while still receiving the result of my worker.

As an example:

context.runningWorker(
  Worker.from {calendarStore.get(Unit)}) { action { State.ShowingCalendars(it) } 
}

Ideally I wanted to collapse the following to something that asks for a suspended function to execute and a state to show with the response

//does not compile
context.run({calendarStore.get(Unit)}, { State.ShowingCalendars(it) })

I got pretty close but cannot get a working solution that allows me to pass an updater to my run function without having to also pass an action:

context.run({calendarStore.get(Unit)},{ action { State.ShowingCalendars(it) } })

From what I gather (apologies this is all new to me) the issue is with action being unable to pass a param to updater the only reason the above example compiles is that it is being captured from the "handler" without ever getting passed between the action and the action.updater. For context here is my current solution

inline fun <reified T,StateT, reified OutputT : Any> RenderContext<StateT, OutputT>.run(
        noinline block: suspend () -> T,
        noinline handler: (T) -> WorkflowAction<StateT, OutputT>
    ) {
        runningWorker(Worker.from { block.invoke() }, handler =  handler)
    }

I'm coming from mvrx which has a similar api as I describe above where a single execute function both acts as a mutator and an action. It would be helpful to have the same here.

//mvrx psuedocode
observable.execute{ result:Result
  return NewState(this as State++)
}
rjrjr commented 4 years ago

My concern here is that it's going to be an even bigger temptation to read from the state argument to render instead of the nextState of the WorkflowAction. Your handler lambda really should be receiving a StateT as well as T.

Given that, still want this?

zach-klippenstein commented 4 years ago

I think my concern is more around obscuring how various calls to the proposed run function would be deduped (even after adding a key parameter). Hiding the fact that there is a Worker here makes it even more magical that two run calls with the same type are considered equivalent, but with a different return type are considered separate. One of the most confusing bits of the API is how workers are deduped, even when they're explicit. I'm not sure if this would make that significantly more confusing, or just marginally confusing relative to how much it already is.

zach-klippenstein commented 4 years ago

Also, to clarify the discussion going forward, this proposes two separate and unrelated things:

  1. Inlines Worker.from (which is what my concern above is about)
  2. Inlines action (which is what Ray's concern is about, I think). We've also had other proposals about doing the latter (cc @steveinflow)
digitalbuddha commented 4 years ago

@rjrjr Yup! The handler should be receiving both the result of the worker as well as the state. I am mostly looking for a way to make the common case of get data from a service and make a new state based on the result a bit shorter to write.