Skip to content

Commit 1c428cf

Browse files
Add warnings about some common smells and gotchas to kdoc.
Fixes #260.
1 parent 645bfdf commit 1c428cf

File tree

2 files changed

+47
-7
lines changed

2 files changed

+47
-7
lines changed

workflow-core/src/main/java/com/squareup/workflow/StatefulWorkflow.kt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ import com.squareup.workflow.WorkflowAction.Updater
4343
* when they're no longer needed. [Props][PropsT] propagate down the tree, [outputs][OutputT] and
4444
* [renderings][RenderingT] propagate up the tree.
4545
*
46+
* ## Avoid capturing stale state
47+
*
48+
* Workflows may not perform side effects in their `render` methods, but may perform side effects by
49+
* running [Worker]s and getting events from [RenderingT]s via [WorkflowAction]s. A [WorkflowAction]
50+
* defines how to update the [StateT] and what [OutputT]s to emit. Actions get access to the current
51+
* workflow's state, and they must use that view of the state. If an action is defined inline, it is
52+
* incorrect to capture, or close over, the [StateT] passed to [render] in the action. Workflows are
53+
* executed synchronously, but external events may not be, so captured state may be stale when the
54+
* action is invoked.
55+
*
4656
* @param PropsT Typically a data class that is used to pass configuration information or bits of
4757
* state that the workflow can always get from its parent and needn't duplicate in its own state.
4858
* May be [Unit] if the workflow does not need any props data.

workflow-core/src/main/java/com/squareup/workflow/Workflow.kt

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,27 +38,57 @@ package com.squareup.workflow
3838
* extends `Workflow`, and keep the implementation (e.g. is your workflow state*ful* or
3939
* state*less* a private implementation detail.
4040
*
41+
* You should almost never implement [Workflow] directly, however. There are two abstract classes
42+
* that you should subclass instead: [StatefulWorkflow] and [StatelessWorkflow]. The differences
43+
* between them are described below, but both type have a `render` method that you implement to
44+
* generate [renderings][RenderingT] from your [props][PropsT] and interact with the runtime (e.g.
45+
* by changing state or emitting outputs).
46+
*
4147
* ### [Stateful Workflows][StatefulWorkflow]
4248
*
43-
* If your workflow needs to keep track of internal state, implement the [StatefulWorkflow]
44-
* interface. That interface has an additional type parameter, `StateT`, and allows you to specify
49+
* If your workflow needs to keep track of internal state, subclass [StatefulWorkflow]. It has an
50+
* additional type parameter, `StateT`, requires you to specify
4551
* [how to create the initial state][StatefulWorkflow.initialState] and how to
46-
* [snapshot][StatefulWorkflow.snapshotState]/restore your state.
52+
* [snapshot][StatefulWorkflow.snapshotState]/restore your state, and passes the current state to
53+
* the [StatefulWorkflow.render] method.
4754
*
4855
* ### [Stateless Workflows][StatelessWorkflow]
4956
*
50-
* If your workflow simply needs to delegate to other workflows, maybe transforming propss, outputs,
51-
* or renderings, extend [StatelessWorkflow], or just pass a lambda to the [stateless] function
52-
* below.
57+
* If your workflow does not have any state of its own and simply needs to delegate to other
58+
* workflows (e.g. transforming props, outputs, or renderings), subclass [StatelessWorkflow] and
59+
* implement its sole [StatelessWorkflow.render] method, or just pass a lambda to the [stateless]
60+
* function.
5361
*
54-
* ## Interacting with Events and Other Workflows
62+
* ## Interacting with events and other workflows
5563
*
5664
* All workflows are passed a [RenderContext] in their render methods. This context allows the
5765
* workflow to interact with the outside world by doing things like listening for events,
5866
* subscribing to streams of data, rendering child workflows, and performing cleanup when the
5967
* workflow is about to be torn down by its parent. See the documentation on [RenderContext] for
6068
* more information about what it can do.
6169
*
70+
* ## Things to avoid
71+
*
72+
* ### Mutable instance state
73+
*
74+
* Classes that implement [Workflow] should not contain mutable properties. Such properties are
75+
* instance-specific state and can introduce buggy behavior. Instead, subclass [StatefulWorkflow]
76+
* and move all your state to the workflow's `StateT` type. For example, setting a property will not
77+
* cause your workflow to be re-rendered – the runtime has no way of knowing that some data it
78+
* doesn't know about has changed. It can also break consumers of your workflows, who may be
79+
* expecting to be able to re-use the same instance of your workflow type in different places in the
80+
* workflow tree – this works if all the workflow's state is contained in its `StateT`, but not if
81+
* the instance has its own properties. _(Note that storing dependencies, which are effectively
82+
* static relative to the workflow instance, in properties is fine.)_
83+
*
84+
* ### Render side effects
85+
*
86+
* Workflows' `render` methods must not perform side effects or read mutable state. They can contain
87+
* logic, but the logic must be based on the [PropsT], the `StateT` if present, and the renderings
88+
* from other workflows. They must _declare_ what work to perform (via [Worker]s), and what data to
89+
* render (via [RenderingT]s and child workflows). For this reason, programming with workflows can
90+
* be considered declarative-style programming.
91+
*
6292
* @param PropsT Typically a data class that is used to pass configuration information or bits of
6393
* state that the workflow can always get from its parent and needn't duplicate in its own state.
6494
* May be [Unit] if the workflow does not need any props data.

0 commit comments

Comments
 (0)