diff --git a/.github/workflows/kotlin.yml b/.github/workflows/kotlin.yml index a1cc582d47..bb54529b77 100644 --- a/.github/workflows/kotlin.yml +++ b/.github/workflows/kotlin.yml @@ -222,6 +222,27 @@ jobs : with : report_paths : '**/build/test-results/test/TEST-*.xml' + jvm-partial-runtime-test: + name: Partial Tree Rendering Only Runtime JVM Tests + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Check with Gradle + uses: ./.github/actions/gradle-task + with: + task: jvmTest --continue -Pworkflow.runtime=baseline-partial + restore-cache-key: main-build-artifacts + + # Report as GitHub Pull Request Check. + - name: Publish Test Report + uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4 + if: always() # always run even if the previous step fails + with: + report_paths: '**/build/test-results/test/TEST-*.xml' + jvm-conflate-stateChange-runtime-test : name : Render On State Change Only and Conflate Stale Runtime JVM Tests runs-on : ubuntu-latest @@ -243,6 +264,27 @@ jobs : with : report_paths : '**/build/test-results/test/TEST-*.xml' + jvm-conflate-partial-runtime-test: + name: Render On State Change Only and Conflate Stale Runtime JVM Tests + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Check with Gradle + uses: ./.github/actions/gradle-task + with: + task: jvmTest --continue -Pworkflow.runtime=conflate-partial + restore-cache-key: main-build-artifacts + + # Report as GitHub Pull Request Check. + - name: Publish Test Report + uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4 + if: always() # always run even if the previous step fails + with: + report_paths: '**/build/test-results/test/TEST-*.xml' + ios-tests : name : iOS Tests runs-on : macos-latest @@ -349,7 +391,7 @@ jobs : ### shardNum: [ 1, 2, 3 ] ### - runtime : [ conflate, baseline-stateChange, conflate-stateChange ] + runtime : [ conflate, baseline-stateChange, conflate-stateChange, baseline-partial, conflate-partial ] steps : - name: Checkout uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 @@ -378,8 +420,10 @@ jobs : - ios-tests - js-tests - jvm-conflate-runtime-test - - jvm-conflate-stateChange-runtime-test - jvm-stateChange-runtime-test + - jvm-partial-runtime-test + - jvm-conflate-stateChange-runtime-test + - jvm-conflate-partial-runtime-test - ktlint - performance-tests - runtime-instrumentation-tests diff --git a/workflow-config/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt b/workflow-config/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt index 93e69595e4..7df4791984 100644 --- a/workflow-config/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt +++ b/workflow-config/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt @@ -3,6 +3,7 @@ package com.squareup.workflow1.config import com.squareup.workflow1.RuntimeConfig import com.squareup.workflow1.RuntimeConfigOptions import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.WorkflowExperimentalRuntime @@ -27,6 +28,9 @@ public class AndroidRuntimeConfigTools { * Then, these can be combined (via '-') with: * "stateChange" : Only re-render when the state of some WorkflowNode has been changed by an * action cascade. + * "partial" : Which includes "stateChange" as well as partial tree rendering, which only + * re-renders each Workflow node if: 1) its state changed; or 2) one of its descendant's state + * changed. * * E.g., "baseline-stateChange" to turn on the stateChange option with the baseline runtime. * @@ -37,6 +41,12 @@ public class AndroidRuntimeConfigTools { "conflate" -> setOf(CONFLATE_STALE_RENDERINGS) "conflate-stateChange" -> setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES) "baseline-stateChange" -> setOf(RENDER_ONLY_WHEN_STATE_CHANGES) + "conflate-partial" -> setOf( + CONFLATE_STALE_RENDERINGS, + RENDER_ONLY_WHEN_STATE_CHANGES, + PARTIAL_TREE_RENDERING + ) + "baseline-partial" -> setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING) "", "baseline" -> RuntimeConfigOptions.RENDER_PER_ACTION else -> throw IllegalArgumentException("Unrecognized config \"${BuildConfig.WORKFLOW_RUNTIME}\"") diff --git a/workflow-config/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt b/workflow-config/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt index 98d3d8d2da..8d3358d352 100644 --- a/workflow-config/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt +++ b/workflow-config/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt @@ -3,6 +3,7 @@ package com.squareup.workflow1.config import com.squareup.workflow1.RuntimeConfig import com.squareup.workflow1.RuntimeConfigOptions import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.WorkflowExperimentalRuntime @@ -27,6 +28,9 @@ public class JvmTestRuntimeConfigTools { * Then, these can be combined (via '-') with: * "stateChange" : Only re-render when the state of some WorkflowNode has been changed by an * action cascade. + * "partial" : Which includes "stateChange" as well as partial tree rendering, which only + * re-renders each Workflow node if: 1) its state changed; or 2) one of its descendant's state + * changed. * * E.g., "baseline-stateChange" to turn on the stateChange option with the baseline runtime. * @@ -38,6 +42,12 @@ public class JvmTestRuntimeConfigTools { "conflate" -> setOf(CONFLATE_STALE_RENDERINGS) "conflate-stateChange" -> setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES) "baseline-stateChange" -> setOf(RENDER_ONLY_WHEN_STATE_CHANGES) + "conflate-partial" -> setOf( + CONFLATE_STALE_RENDERINGS, + RENDER_ONLY_WHEN_STATE_CHANGES, + PARTIAL_TREE_RENDERING + ) + "baseline-partial" -> setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING) "", "baseline" -> RuntimeConfigOptions.RENDER_PER_ACTION else -> throw IllegalArgumentException("Unrecognized config \"$runtimeConfig\"") diff --git a/workflow-core/api/workflow-core.api b/workflow-core/api/workflow-core.api index e7467b3a33..3ff8f29942 100644 --- a/workflow-core/api/workflow-core.api +++ b/workflow-core/api/workflow-core.api @@ -77,6 +77,26 @@ public abstract class com/squareup/workflow1/LifecycleWorker : com/squareup/work public final fun run ()Lkotlinx/coroutines/flow/Flow; } +public final class com/squareup/workflow1/NullableInitBox { + public static final synthetic fun box-impl (Ljava/lang/Object;)Lcom/squareup/workflow1/NullableInitBox; + public static fun constructor-impl (Ljava/lang/Object;)Ljava/lang/Object; + public static synthetic fun constructor-impl$default (Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)Ljava/lang/Object; + public fun equals (Ljava/lang/Object;)Z + public static fun equals-impl (Ljava/lang/Object;Ljava/lang/Object;)Z + public static final fun equals-impl0 (Ljava/lang/Object;Ljava/lang/Object;)Z + public static final fun getOrThrow-impl (Ljava/lang/Object;)Ljava/lang/Object; + public fun hashCode ()I + public static fun hashCode-impl (Ljava/lang/Object;)I + public static final fun isInitialized-impl (Ljava/lang/Object;)Z + public fun toString ()Ljava/lang/String; + public static fun toString-impl (Ljava/lang/Object;)Ljava/lang/String; + public final synthetic fun unbox-impl ()Ljava/lang/Object; +} + +public final class com/squareup/workflow1/NullableInitBox$Uninitialized { + public static final field INSTANCE Lcom/squareup/workflow1/NullableInitBox$Uninitialized; +} + public final class com/squareup/workflow1/PropsUpdated : com/squareup/workflow1/ActionProcessingResult { public static final field INSTANCE Lcom/squareup/workflow1/PropsUpdated; } diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/NullableInitBox.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/NullableInitBox.kt new file mode 100644 index 0000000000..ec6504e564 --- /dev/null +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/NullableInitBox.kt @@ -0,0 +1,28 @@ +package com.squareup.workflow1 + +import kotlin.jvm.JvmInline + +/** + * Used to wrap immutable nullable values whose holder may not yet be initialized. + * Check [isInitialized] to see if the value has been assigned. + */ +@JvmInline +public value class NullableInitBox(private val _value: Any? = Uninitialized) { + /** + * Whether or not a value has been set for this [NullableInitBox] + */ + public val isInitialized: Boolean get() = _value !== Uninitialized + + /** + * Get the value this has been initialized with. + * + * @throws [IllegalStateException] if the value in the box has not been initialized. + */ + @Suppress("UNCHECKED_CAST") + public fun getOrThrow(): T { + check(isInitialized) { "NullableInitBox was fetched before it was initialized with a value." } + return _value as T + } + + public object Uninitialized +} diff --git a/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/NullableInitBoxTest.kt b/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/NullableInitBoxTest.kt new file mode 100644 index 0000000000..0b18c21b72 --- /dev/null +++ b/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/NullableInitBoxTest.kt @@ -0,0 +1,41 @@ +package com.squareup.workflow1 + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class NullableInitBoxTest { + + @Test fun reports_not_initialized() { + val box = NullableInitBox() + + assertFalse(box.isInitialized) + } + + @Test fun reports_initialized() { + val box = NullableInitBox("Hello") + + assertTrue(box.isInitialized) + } + + @Test fun returns_value() { + val box = NullableInitBox("Hello") + + assertEquals("Hello", box.getOrThrow()) + } + + @Test fun throws_exceptions() { + val box = NullableInitBox() + + val exception = assertFailsWith { + box.getOrThrow() + } + + assertEquals( + "NullableInitBox was fetched before it was initialized with a value.", + exception.message + ) + } +} diff --git a/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkerTest.kt b/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkerTest.kt index 25581e0b8d..8a4cdcff22 100644 --- a/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkerTest.kt +++ b/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkerTest.kt @@ -16,7 +16,6 @@ import kotlin.test.assertNotSame import kotlin.test.assertTrue @ExperimentalCoroutinesApi -@OptIn(ExperimentalStdlibApi::class) class WorkerTest { @Test fun timer_returns_equivalent_workers_keyed() { diff --git a/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt b/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt index 8ae6aa9a93..de3855ec9c 100644 --- a/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt +++ b/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt @@ -12,7 +12,6 @@ import kotlin.test.assertFailsWith import kotlin.test.assertNotEquals import kotlin.test.assertNull -@OptIn(ExperimentalStdlibApi::class) internal class WorkflowIdentifierTest { @Test fun restored_identifier_toString() { diff --git a/workflow-runtime/api/workflow-runtime.api b/workflow-runtime/api/workflow-runtime.api index 5591211f0a..cc40aaf0dc 100644 --- a/workflow-runtime/api/workflow-runtime.api +++ b/workflow-runtime/api/workflow-runtime.api @@ -25,6 +25,7 @@ public final class com/squareup/workflow1/RenderingAndSnapshot { public final class com/squareup/workflow1/RuntimeConfigOptions : java/lang/Enum { public static final field CONFLATE_STALE_RENDERINGS Lcom/squareup/workflow1/RuntimeConfigOptions; public static final field Companion Lcom/squareup/workflow1/RuntimeConfigOptions$Companion; + public static final field PARTIAL_TREE_RENDERING Lcom/squareup/workflow1/RuntimeConfigOptions; public static final field RENDER_ONLY_WHEN_STATE_CHANGES Lcom/squareup/workflow1/RuntimeConfigOptions; public static fun getEntries ()Lkotlin/enums/EnumEntries; public static fun valueOf (Ljava/lang/String;)Lcom/squareup/workflow1/RuntimeConfigOptions; diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RuntimeConfig.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RuntimeConfig.kt index 52c445f86b..5bbc237b19 100644 --- a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RuntimeConfig.kt +++ b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RuntimeConfig.kt @@ -38,6 +38,22 @@ public enum class RuntimeConfigOptions { @WorkflowExperimentalRuntime RENDER_ONLY_WHEN_STATE_CHANGES, + /** + * Only re-render each active Workflow node if: + * 1. Its own state changed, OR + * 2. One of its descendant's state has changed. + * + * Otherwise return the cached rendering (as there is no way it could have changed). + * + * Note however that you must be careful using this because there may be external + * state that your Workflow draws in and re-renders, and if that is not explicitly + * tracked within that Workflow's state then the Workflow will not re-render. + * In this case make sure that the implicit state is tracked within the Workflow's + * `StateT` in some way, even if only via a hash token. + */ + @WorkflowExperimentalRuntime + PARTIAL_TREE_RENDERING, + /** * If we have more actions to process, do so before passing the rendering to the UI layer. */ diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt index 5a2672dff2..ea4b25299a 100644 --- a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt +++ b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt @@ -3,14 +3,17 @@ package com.squareup.workflow1.internal import com.squareup.workflow1.ActionApplied import com.squareup.workflow1.ActionProcessingResult import com.squareup.workflow1.NoopWorkflowInterceptor +import com.squareup.workflow1.NullableInitBox import com.squareup.workflow1.RenderContext import com.squareup.workflow1.RuntimeConfig import com.squareup.workflow1.RuntimeConfigOptions +import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.StatefulWorkflow import com.squareup.workflow1.TreeSnapshot import com.squareup.workflow1.Workflow import com.squareup.workflow1.WorkflowAction import com.squareup.workflow1.WorkflowExperimentalApi +import com.squareup.workflow1.WorkflowExperimentalRuntime import com.squareup.workflow1.WorkflowIdentifier import com.squareup.workflow1.WorkflowInterceptor import com.squareup.workflow1.WorkflowInterceptor.WorkflowSession @@ -44,7 +47,7 @@ import kotlin.coroutines.CoroutineContext * hard-coded values added to worker contexts. It must not contain a [Job] element (it would violate * structured concurrency). */ -@OptIn(WorkflowExperimentalApi::class) +@OptIn(WorkflowExperimentalApi::class, WorkflowExperimentalRuntime::class) internal class WorkflowNode( val id: WorkflowNodeId, workflow: StatefulWorkflow, @@ -86,9 +89,11 @@ internal class WorkflowNode( ) private val sideEffects = ActiveStagingList() private var lastProps: PropsT = initialProps + private var lastRendering: NullableInitBox = NullableInitBox() private val eventActionsChannel = Channel>(capacity = UNLIMITED) private var state: StateT + private var subtreeStateDidChange: Boolean = true private val baseRenderContext = RealRenderContext( renderer = subtreeManager, @@ -135,7 +140,7 @@ internal class WorkflowNode( fun snapshot(workflow: StatefulWorkflow<*, *, *, *>): TreeSnapshot { @Suppress("UNCHECKED_CAST") val typedWorkflow = workflow as StatefulWorkflow - updateCachedWorkflowInstance(typedWorkflow) + maybeUpdateCachedWorkflowInstance(typedWorkflow) return interceptor.onSnapshotStateWithChildren({ val childSnapshots = subtreeManager.createChildSnapshots() val rootSnapshot = interceptedWorkflowInstance.snapshotState(state) @@ -209,7 +214,7 @@ internal class WorkflowNode( * Call this after we have been passed any workflow instance, in [render] or [snapshot]. It may * have changed and we should check to see if we need to update our cached instances. */ - private fun updateCachedWorkflowInstance( + private fun maybeUpdateCachedWorkflowInstance( workflow: StatefulWorkflow ) { if (workflow !== cachedWorkflowInstance) { @@ -227,31 +232,51 @@ internal class WorkflowNode( workflow: StatefulWorkflow, props: PropsT ): RenderingT { - updateCachedWorkflowInstance(workflow) - updatePropsAndState(props) + updatePropsAndState(props, workflow) - baseRenderContext.unfreeze() - val rendering = interceptedWorkflowInstance.render(props, state, context) - baseRenderContext.freeze() + if (!runtimeConfig.contains(PARTIAL_TREE_RENDERING) || + !lastRendering.isInitialized || + subtreeStateDidChange + ) { + // If we haven't already updated the cached instance, better do it now! + maybeUpdateCachedWorkflowInstance(workflow) - workflowTracer.trace("UpdateRuntimeTree") { - // Tear down workflows and workers that are obsolete. - subtreeManager.commitRenderedChildren() - // Side effect jobs are launched lazily, since they can send actions to the sink, and can only - // be started after context is frozen. - sideEffects.forEachStaging { it.job.start() } - sideEffects.commitStaging { it.job.cancel() } + baseRenderContext.unfreeze() + lastRendering = NullableInitBox(interceptedWorkflowInstance.render(props, state, context)) + baseRenderContext.freeze() + + workflowTracer.trace("UpdateRuntimeTree") { + // Tear down workflows and workers that are obsolete. + subtreeManager.commitRenderedChildren() + // Side effect jobs are launched lazily, since they can send actions to the sink, and can only + // be started after context is frozen. + sideEffects.forEachStaging { it.job.start() } + sideEffects.commitStaging { it.job.cancel() } + } + // After we have rendered this subtree, we need another action in order for us to be + // considered dirty again. + subtreeStateDidChange = false } - return rendering + return lastRendering.getOrThrow() } + /** + * Update props if they have changed. If that happens, then check to see if we need + * to update the cached workflow instance, then call [StatefulWorkflow.onPropsChanged] and + * update the state from that. We consider any change to props as [subtreeStateDidChange] because + * the props themselves are used in [StatefulWorkflow.render] (they are the 'external' part of + * the state) so we must re-render. + */ private fun updatePropsAndState( - newProps: PropsT + newProps: PropsT, + workflow: StatefulWorkflow, ) { if (newProps != lastProps) { + maybeUpdateCachedWorkflowInstance(workflow) val newState = interceptedWorkflowInstance.onPropsChanged(lastProps, newProps, state) state = newState + subtreeStateDidChange = true } lastProps = newProps } @@ -260,6 +285,7 @@ internal class WorkflowNode( * Applies [action] to this workflow's [state] and then passes the resulting [ActionApplied] * via [emitAppliedActionToParent] to the parent, with additional information as to whether or * not this action has changed the current node's state. + * */ private fun applyAction( action: WorkflowAction, @@ -272,7 +298,21 @@ internal class WorkflowNode( // Changing state is sticky, we pass it up if it ever changed. stateChanged = actionApplied.stateChanged || (childResult?.stateChanged ?: false) ) - return if (actionApplied.output != null) { + // Our state changed or one of our children's state changed. + subtreeStateDidChange = aggregateActionApplied.stateChanged + return if (actionApplied.output != null || + runtimeConfig.contains(PARTIAL_TREE_RENDERING) + ) { + // If we are using the optimization, always return to the parent, so we carry a path that + // notes that the subtree did change all the way to the root. + // + // We don't need that without the optimization because there is nothing + // to output from the root of the runtime -- the output has propagated + // as far as it needs to causing all corresponding state changes. + // + // However, the root and the path down to the changed nodes must always + // re-render now, so this is the implementation detail of how we get + // subtreeStateDidChange = true on that entire path to the root. emitAppliedActionToParent(aggregateActionApplied) } else { aggregateActionApplied diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt index 50f48e7c4d..4a1df64f64 100644 --- a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt +++ b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt @@ -1,6 +1,7 @@ package com.squareup.workflow1 import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.internal.ParameterizedTestRunner import kotlinx.coroutines.CancellationException @@ -50,7 +51,9 @@ class RenderWorkflowInTest { RuntimeConfigOptions.RENDER_PER_ACTION, setOf(RENDER_ONLY_WHEN_STATE_CHANGES), setOf(CONFLATE_STALE_RENDERINGS), - setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES) + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), ) private val tracerOptions = setOf( @@ -1182,6 +1185,146 @@ class RenderWorkflowInTest { } } + @Test + fun `for_partial_tree_rendering_we_do_not_render_nodes_if_state_not_changed_even_in_render_pass`() { + runtimeTestRunner.runParametrizedTest( + paramSource = runtimeOptions.filter { + it.first.contains(PARTIAL_TREE_RENDERING) + }, + before = ::setup, + ) { (runtimeConfig: RuntimeConfig, workflowTracer: WorkflowTracer?) -> + runTest(UnconfinedTestDispatcher()) { + check(runtimeConfig.contains(PARTIAL_TREE_RENDERING)) + + val trigger = MutableSharedFlow() + var childRenderCount = 0 + var parentRenderCount = 0 + + val childWorkflow = Workflow.stateful( + // Starts with "state 1" + initialState = "state 1", + render = { renderState -> + runningWorker( + trigger.asWorker() + ) { + action("") { + state = it // Change state, but no output for parent. + } + } + renderState.also { + childRenderCount++ + } + } + ) + + val workflow = Workflow.stateful( + initialState = "state 0", + render = { renderState -> + renderChild(childWorkflow) { childOutput -> + action("childHandler") { + state = childOutput + } + } + runningWorker( + trigger.asWorker() + ) { + action("") { + state = it + } + } + renderState.also { + parentRenderCount++ + } + } + ) + val props = MutableStateFlow(Unit) + renderWorkflowIn( + workflow = workflow, + scope = backgroundScope, + props = props, + runtimeConfig = runtimeConfig, + workflowTracer = workflowTracer, + ) {} + + trigger.emit("state 1") // same value as the child starts with. + advanceUntilIdle() + + assertEquals(2, parentRenderCount) + assertEquals(1, childRenderCount) + } + } + } + + @Test fun for_partial_tree_rendering_we_render_nodes_if_state_changed() { + runtimeTestRunner.runParametrizedTest( + paramSource = runtimeOptions.filter { + it.first.contains(PARTIAL_TREE_RENDERING) + }, + before = ::setup, + ) { (runtimeConfig: RuntimeConfig, workflowTracer: WorkflowTracer?) -> + runTest(UnconfinedTestDispatcher()) { + check(runtimeConfig.contains(PARTIAL_TREE_RENDERING)) + + val trigger = MutableSharedFlow() + var childRenderCount = 0 + var parentRenderCount = 0 + + val childWorkflow = Workflow.stateful( + // Starts with "state 0" + initialState = "state 0", + render = { renderState -> + runningWorker( + trigger.asWorker() + ) { + action("") { + state = it // Change state, but no output for parent. + } + } + renderState.also { + childRenderCount++ + } + } + ) + + val workflow = Workflow.stateful( + initialState = "state 0", + render = { renderState -> + renderChild(childWorkflow) { childOutput -> + action("childHandler") { + state = childOutput + } + } + runningWorker( + trigger.asWorker() + ) { + action("") { + state = it + } + } + renderState.also { + parentRenderCount++ + } + } + ) + val props = MutableStateFlow(Unit) + renderWorkflowIn( + workflow = workflow, + scope = backgroundScope, + props = props, + runtimeConfig = runtimeConfig, + workflowTracer = workflowTracer, + ) {} + + trigger.emit("state 1") // different value than the child starts with. + advanceUntilIdle() + + assertEquals(3, parentRenderCount) + // Parent needs to be rendered 3x, but child only 2x as the 3rd time its the same. + assertEquals(2, childRenderCount) + } + } + } + @Test fun for_render_on_change_only_and_conflate_we_drain_action_but_do_not_render_no_state_changed() { runtimeTestRunner.runParametrizedTest( diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowOperatorsTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowOperatorsTest.kt index 735b966f3a..1bc5bc0c2f 100644 --- a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowOperatorsTest.kt +++ b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowOperatorsTest.kt @@ -207,23 +207,37 @@ class WorkflowOperatorsTest { private abstract class StateFlowWorkflow( val name: String, val flow: StateFlow - ) : StatelessWorkflow() { + ) : StatefulWorkflow() { var starts: Int = 0 private set + override fun initialState( + props: Unit, + snapshot: Snapshot? + ): T { + return flow.value + } + private val rerenderWorker = object : Worker { override fun run(): Flow = flow.onStart { starts++ } } override fun render( renderProps: Unit, + renderState: T, context: RenderContext ): T { // Listen to the flow to trigger a re-render when it updates. - context.runningWorker(rerenderWorker as Worker) { WorkflowAction.noAction() } - return flow.value + context.runningWorker(rerenderWorker) { output: T -> + action("rerenderUpdate") { + state = output + } + } + return renderState } + override fun snapshotState(state: T): Snapshot? = null + override fun toString(): String = "StateFlowWorkflow($name)" } } diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt index 7efab5a762..18e657ce44 100644 --- a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt +++ b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt @@ -5,6 +5,7 @@ import com.squareup.workflow1.NoopWorkflowInterceptor import com.squareup.workflow1.RuntimeConfig import com.squareup.workflow1.RuntimeConfigOptions import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.Worker import com.squareup.workflow1.Workflow @@ -34,7 +35,9 @@ internal class WorkflowRunnerTest { RuntimeConfigOptions.RENDER_PER_ACTION, setOf(RENDER_ONLY_WHEN_STATE_CHANGES), setOf(CONFLATE_STALE_RENDERINGS), - setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES) + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), ).asSequence() private fun setup() { diff --git a/workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt b/workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt index 15112aedf2..51a1b648bc 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt @@ -1,6 +1,7 @@ package com.squareup.workflow1 import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.testing.headlessIntegrationTest import kotlinx.coroutines.Job @@ -19,7 +20,9 @@ class WorkflowsLifecycleTests { RuntimeConfigOptions.RENDER_PER_ACTION, setOf(RENDER_ONLY_WHEN_STATE_CHANGES), setOf(CONFLATE_STALE_RENDERINGS), - setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES) + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), ).asSequence() private val runtimeTestRunner = ParameterizedTestRunner()