-
Notifications
You must be signed in to change notification settings - Fork 112
*BREAKING* Introduces BaseRenderContext.remember and stable eventHandlers.
#1267
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
Conversation
d4cdaa3 to
a137249
Compare
workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/RealRenderContext.kt
Outdated
Show resolved
Hide resolved
95498e2 to
fec7178
Compare
...dungeon/timemachine/src/test/java/com/squareup/sample/timemachine/TimeMachineWorkflowTest.kt
Outdated
Show resolved
Hide resolved
workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/RealRenderContext.kt
Outdated
Show resolved
Hide resolved
workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt
Show resolved
Hide resolved
workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/WorkflowInterceptor.kt
Show resolved
Hide resolved
fc02943 to
1f63015
Compare
77672ac to
7348024
Compare
workflow-core/src/commonMain/kotlin/com/squareup/workflow1/BaseRenderContext.kt
Show resolved
Hide resolved
5528d91 to
bc7a48d
Compare
BaseRenderContext.remember and stable eventHandlers.
6452f7f to
0b76577
Compare
| /** | ||
| * Convenience alias for working with [WorkflowAction.Updater]. | ||
| */ | ||
| public typealias Updater<P, S, O> = WorkflowAction<P, S, O>.Updater |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
| * [props][PropsT] received from its parent, and it may render child workflows that do have | ||
| * their own internal state. | ||
| */ | ||
| public inline fun <PropsT, OutputT, RenderingT> Workflow.Companion.stateless( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to this change, but did we end up discouraging the use of these functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. They're not super widely used but they're very handy when they're handy.
workflow-core/src/commonMain/kotlin/com/squareup/workflow1/BaseRenderContext.kt
Show resolved
Hide resolved
| private fun checkNotFrozen(reason: () -> String = { "" }) = check(!frozen) { | ||
| "RenderContext cannot be used after render method returns" + | ||
| "${reason().takeUnless { it.isBlank() }?.let { " ($it)" }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
…andlers`. Workflow makes it very convenient to render view models with anonymous lambdas as their event handler functions. Compose hates that. To address that mismatch without forcing everyone to retrofit their apps to use event objects instead of lambdas (it's a little late for that!) we introduce support for stable event handlers: anonymous lambdas whose identity looks the same to Compose across updates. In order to do this we're breaking the existing `eventHandler` and `safeEventHandler` functions a bit. - We introduce a new optional `remember: Boolean? = null` parameter. Set that true to get the new stability. If you leave it to the default `null` we look for a new `STABLE_EVENT_HANDLER : RuntimeConfigOption` to decide what to do. If you set that config option on an existing app and make no other changes, all of your existing `eventHandler` functions will be stable. - When `remember` is true, the existing `name` parameter becomes weight bearing. It's no longer just a logging aid, it's part of a key identifying your stable lambda. The other parts of the key are its return type, and the types of any of its parameters. Duplicating a key within a particular `render()` call is a runtime error, similar to the rules for `renderChild`, `runningWorker`, and `runningSideEffect`. - To make it easier to find fix and prevent those new runtime errors `testRender` and `WorkflowTestParams` now accept optional `RuntimeConfig` parameters, and throw appropriate errors if `STABLE_EVENT_HANDLER` is set. `testRender` also now honors `JvmTestRuntimeConfigTools.getTestRuntimeConfig()`. - Most of the `eventHandler` functions have also been changed to `inline` -- necessary so that we can reify their parameter types for the key scheme described above All of this is built on a new `BaseRenderContext.remember` primitive, which provides a light weight mechanism to save a bit of state across a workflow session without having to find room for it in `StateT`. `BaseRenderContext` also now provides `val runtimeConfig: RuntimeConfig` in support of all of the above.
0b76577 to
60f7ea5
Compare
...samples/src/main/java/com/squareup/sample/compose/inlinerendering/InlineRenderingWorkflow.kt
Show resolved
Hide resolved
| .using(SizeTransform(clip = false)) | ||
| } | ||
| }, | ||
| label = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this fixed a compiler warning? Will check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, seems to serve no purpose, will revert.
| * [props][PropsT] received from its parent, and it may render child workflows that do have | ||
| * their own internal state. | ||
| */ | ||
| public inline fun <PropsT, OutputT, RenderingT> Workflow.Companion.stateless( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. They're not super widely used but they're very handy when they're handy.
Workflow makes it very convenient to render view models with anonymous lambdas as their event handler functions. Compose hates that.
To address that mismatch without forcing everyone to retrofit their apps to use event objects instead of lambdas (it's a little late for that!) we introduce support for stable event handlers: anonymous lambdas whose identity looks the same to Compose across updates.
In order to do this we're breaking the existing
eventHandlerandsafeEventHandlerfunctions a bit.We introduce a new optional
remember: Boolean? = nullparameter. Set that true to get the new stability. If you leave it to the defaultnullwe look for a newSTABLE_EVENT_HANDLER : RuntimeConfigOptionto decide what to do. If you set that config option on an existing app and make no other changes, all of your existingeventHandlerfunctions will be stable.When
rememberis true, the existingnameparameter becomes weight bearing. It's no longer just a logging aid, it's part of a key identifying your stable lambda. The other parts of the key are its return type, and the types of any of its parameters. Duplicating a key within a particularrender()call is a runtime error, similar to the rules forrenderChild,runningWorker, andrunningSideEffect.To make it easier to find fix and prevent those new runtime errors,
testRenderandWorkflowTestParamsnow accept optionalRuntimeConfigparameters, and throw appropriate errors ifSTABLE_EVENT_HANDLERis set.testRenderalso now honorsJvmTestRuntimeConfigTools.getTestRuntimeConfig().Most of the
eventHandlerfunctions have also been changed toinline-- necessary so that we can reify their parameter types for the key scheme described aboveAll of this is built on a new
BaseRenderContext.rememberprimitive, which provides a light weight mechanism to save a bit of state across a workflow session without having to find room for it inStateT.BaseRenderContextalso now providesval runtimeConfig: RuntimeConfigin support of all of the above.