Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Kotlin Sink, makeActionSink, makeEventSink. #537

Merged
merged 1 commit into from Aug 21, 2019

Conversation

@rjrjr
Copy link
Collaborator

commented Aug 15, 2019

This PR introduces:

  • a Kotlin Sink class matching Swift's
  • RenderContext.makeActionSink matching Swift's makeSink(ofActionType)
  • RenderContext.makeEventSink matching Swift's makeSink(ofEventType)

and deprecates:

  • WorkflowAction.enterState
  • WorkflowAction.emitOutput
  • WorkflowAction.modifyState
  • RenderContext.onEvent
  • EventHandler

Kotlin apps are now able to make custom WorkflowAction class hierarchies in
the style common in Swift apps. This is made possible by the new
WorkflowAction.apply method, analagous to the same method in Swift.
Where the Swift version has an inout StateT function argument,
Kotlin's is called against a Mutator<StateT> receiver.

This allows us to implement action types like the following, where the null
returns indicate that no output is to be emitted.

  sealed class Action : WorkflowAction<Movement, Nothing> {
    class StartMoving(private val direction: Direction) : Action() {
      override fun Mutator<Movement>.apply(): Nothing? {
        state += direction
        return null
      }
    }

    class StopMoving(private val direction: Direction) : Action() {
      override fun Mutator<Movement>.apply(): Nothing? {
        state -= direction
        return null
      }
    }
  }

The convention is to declare these Action types inside workflows, instead of
Event type trees in rendering classes. And instead of creating EventHandler
instances to be used by renderings, we create Sink instances:

  override fun render(
    input: TakeTurnsInput,
    state: Turn,
    context: RenderContext<Turn, CompletedGame>
  ): GamePlayScreen {
    val sink = context.makeActionSink<Action>()

    return GamePlayScreen(
        playerInfo = input.playerInfo,
        gameState = state,
        onQuit = { sink.send(Quit) },
        onClick = { row, col -> sink.send(TakeSquare(row, col)) }
    )
  }

@rjrjr rjrjr force-pushed the ray/update-action branch from de4ec51 to e0e6b80 Aug 15, 2019

@rjrjr

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2019

@rjrjr rjrjr force-pushed the ray/update-action branch from 8306461 to 129605c Aug 16, 2019

@rjrjr rjrjr changed the title wip: SwiftyEvent wip: Swifty Actions for Kotlin Aug 17, 2019

@rjrjr rjrjr force-pushed the ray/update-action branch 2 times, most recently from d824cc9 to 59a3588 Aug 19, 2019

@rjrjr rjrjr changed the title wip: Swifty Actions for Kotlin Adds Kotlin Sink, makeActionSink, makeEventSink. Aug 20, 2019

@rjrjr rjrjr force-pushed the ray/update-action branch 2 times, most recently from c1734a0 to 25a83ef Aug 20, 2019

@@ -81,4 +75,14 @@ class HelloTerminalWorkflow : TerminalWorkflow,
}

override fun snapshotState(state: State): Snapshot = Snapshot.EMPTY

private fun onKeystroke(key: KeyStroke): HelloTerminalAction = WorkflowAction {

This comment has been minimized.

Copy link
@rjrjr

rjrjr Aug 20, 2019

Author Collaborator

This is the kind of place where I especially think WorkflowAction { carries its weight — these actions are all tied to worker calls, and so we never make a sink. An enum / sealed class would simply be boilerplate, and worse introduce an unused mechanism.

This comment has been minimized.

Copy link
@loganj

loganj Aug 20, 2019

Collaborator

+1 these are super clean

@rjrjr

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 20, 2019

The main driver here is that we want to stop interfering with the ability of Swift and Kotlin team-members to be useful to each other.

Without this change it just isn’t practical for Kotlin workflows to define their own own internal action / event types. I think this is the main reason that Kotlin devs are less like to write single-screen workflows, while that’s the norm in Swift. And our strong impression is that single screen is usually a win.

@rjrjr rjrjr added this to the kotlin v0.20.0 milestone Aug 20, 2019

@rjrjr rjrjr added this to In progress in Workflow (Kotlin) via automation Aug 20, 2019

@rjrjr rjrjr moved this from In progress to Needs review in Workflow (Kotlin) Aug 20, 2019

@rjrjr rjrjr force-pushed the ray/update-action branch from 25a83ef to 749dadb Aug 20, 2019

@rjrjr rjrjr force-pushed the ray/update-action branch from 749dadb to 6366fd2 Aug 20, 2019

@rjrjr rjrjr force-pushed the ray/update-action branch from 6366fd2 to d3488d0 Aug 20, 2019

@loganj
Copy link
Collaborator

left a comment

Reviewed mostly the samples. All the changes there look great!

@@ -81,4 +75,14 @@ class HelloTerminalWorkflow : TerminalWorkflow,
}

override fun snapshotState(state: State): Snapshot = Snapshot.EMPTY

private fun onKeystroke(key: KeyStroke): HelloTerminalAction = WorkflowAction {

This comment has been minimized.

Copy link
@loganj

loganj Aug 20, 2019

Collaborator

+1 these are super clean

@rjrjr rjrjr requested a review from zach-klippenstein Aug 21, 2019

Adds Kotlin Sink, makeActionSink, makeEventSink.
This PR introduces:

 * a Kotlin `Sink` class matching Swift's
 * `RenderContext.makeActionSink` matching Swift's `makeSink(ofActionType)`
 * `RenderContext.makeEventSink` matching Swift's `makeSink(ofEventType)`

and deprecates:

 * `WorkflowAction.enterState`
 * `WorkflowAction.emitOutput`
 * `WorkflowAction.modifyState`
 * `RenderContext.onEvent`
 * `EventHandler`

Kotlin apps are now able to make custom WorkflowAction class hierarchies in
the style common in Swift apps. This is made possible by the new
`WorkflowAction.apply` method, analagous to the same method in Swift.
Where the Swift version has an `inout StateT` function argument,
Kotlin's is called against a `Mutator<StateT>` receiver.

This allows us to implement action types like the following, where the null
returns indicate that no output is to be emitted.

```
  sealed class Action : WorkflowAction<Movement, Nothing> {
    class StartMoving(private val direction: Direction) : Action() {
      override fun Mutator<Movement>.apply(): Nothing? {
        state += direction
        return null
      }
    }

    class StopMoving(private val direction: Direction) : Action() {
      override fun Mutator<Movement>.apply(): Nothing? {
        state -= direction
        return null
      }
    }
  }
```

The convention is to declare these `Action` types inside workflows, instead of
`Event` type trees in rendering classes. And instead of creating `EventHandler`
instances to be used by renderings, we create `Sink` instances:

```
  override fun render(
    input: TakeTurnsInput,
    state: Turn,
    context: RenderContext<Turn, CompletedGame>
  ): GamePlayScreen {
    val sink = context.makeActionSink<Action>()

    return GamePlayScreen(
        playerInfo = input.playerInfo,
        gameState = state,
        onQuit = { sink.send(Quit) },
        onClick = { row, col -> sink.send(TakeSquare(row, col)) }
    )
  }
```

@rjrjr rjrjr force-pushed the ray/update-action branch from d3488d0 to 5f3cb7b Aug 21, 2019

@rjrjr

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 21, 2019

Update fixes signature of makeActionSink -- block return type should have been OutputT?, not OutputT. Added related unit test.

@rjrjr rjrjr merged commit 1fd793d into master Aug 21, 2019

3 checks passed

Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details
license/cla Contributor License Agreement is signed.
Details

Workflow (Kotlin) automation moved this from Needs review to Done Aug 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.