1311: Enable applying Multiple Actions on immediate dispatcher for `CONFLATE_STALE_RENDERINGS` #1344

steve-the-edwards · 2025-06-13T15:51:21Z

First commits adds a failing Android test to demonstrate the problem reported in #1311 .

Second commit fixes this for CONFLATE_STALE_RENDERINGS by yield()ing before applying the first action.

The issue with yield() in the middle of CSR's inner loop was that it breaks our workflow runtime stabilizing into two main thread messages (yield() always posts to the Handler). We want to keep the guarantee of each Workflow update in response to an event as happening within 1 main thread message.

This solution satisfies that constraint by yield()ing before we apply the first action. With these optimizations enabled, when select resumes with a deferrable action ready to be processed, we provide it to the runtime loop as a Deferred - then we yield() before applying it. This allows any other possible collectors to go ahead and queue actions. We can then drain them manually without select in the inner loop for conflate (and later DEA).

Note: We add a field for isDeferrable on action. Worker actions are by default deferrable. This allows us to opt actions from UI handlers out of this type of optimization.

We also add tests on Main.immediate that confirm that non-deferrable actions are handled in one main thread message from the callback. Otherwise they are handled in one main thread message from the action application.

This also adds new methods for fetching those available actions to apply without select (using tryReceive) to the WorkflowRunner and WorkflowNode as well as WorkflowChildNode.

Fixes #1311

rjrjr · 2025-06-13T16:27:31Z

...w-runtime-android/src/androidTest/java/com/squareup/workflow1/AndroidRenderWorkflowInTest.kt

+            key = "Worker3"
+          ) {
+            action("") {
+              // Update the rendering in order to show conflation.


Don't you mean "Update the state"?

Wouldn't the test be more rigorous if you accumulated a bunch of deltas to the state rather than clobbering each time? So at the end you could assert that the rendering = "initial+1+2+3+4"

Yes, that's a much better test!

rjrjr · 2025-06-13T16:29:58Z

...w-runtime-android/src/androidTest/java/com/squareup/workflow1/AndroidRenderWorkflowInTest.kt

+
+      collectionJob.cancel()
+
+      // 2 renderings (initial and then the update.) Not *5* renderings.


rjrjr · 2025-06-13T16:30:47Z

...w-runtime-android/src/androidTest/java/com/squareup/workflow1/AndroidRenderWorkflowInTest.kt

+    }
+
+  @Test
+  fun with_conflate_we_conflate_stacked_actions_from_side_effects_into_one_rendering() =


What made you decide both tests were necessary? (Don't get me wrong, more is better as far as I'm concerned, just curious.)

it was early on when i wondered if the Worker machinery was introducing any impedence in resuming to get the actions queued. Its not.

rjrjr · 2025-06-13T16:45:12Z

...w-runtime-android/src/androidTest/java/com/squareup/workflow1/AndroidRenderWorkflowInTest.kt

+import org.junit.Test
+import kotlin.test.assertEquals
+
+@OptIn(WorkflowExperimentalRuntime::class, ExperimentalCoroutinesApi::class)


Should you add a test that simulates having an action enqueued via an event handler lambda on a rendering? End to end demonstration of the primary promise we're trying to make to ourselves. Perhaps it should be parameterized to run w/all the config cominbations.

I added these tests (both for deferrable and non deferrable actions). You and @pyricau can double check my logic for asserting on the main thread message.

rjrjr · 2025-06-13T16:48:05Z

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt

@@ -348,6 +368,59 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
    }
  }

+  //
+  // /**


Commented out b/c not necessary after all? Or is this the thing you have to add to make it actually work?

Not necessary. I just forgot to remove this.

A deferrable action is one that can wait a main thread post - such as from an async trigger (Worker). Not a UI action. To allow other actions to queue up, so they can be drained.

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

steve-the-edwards · 2025-06-13T20:50:45Z

So this adds the ability to set an action as deferrable in order to try and keep the keyUp in the same main thread for UI interactions.

Ultimately I think we don't want that. we would rather have everything from Workflow's perspective in one main thread message (including teh click action handling). SO I think eventually I'd prefer to make all actions deferrable. but we can go one step at a time?

1311: Add failing test for Main.immediate conflation

9dcac88

steve-the-edwards mentioned this pull request Jun 13, 2025

With the Main.immediate dispatcher process side effects in time to drain multiple actions without yield(). #1311

Open

rjrjr reviewed Jun 13, 2025

View reviewed changes

steve-the-edwards force-pushed the sedwards/update-csr branch 2 times, most recently from f62f5d7 to 455068b Compare June 13, 2025 20:19

steve-the-edwards marked this pull request as ready for review June 13, 2025 20:19

steve-the-edwards requested review from zach-klippenstein and a team as code owners June 13, 2025 20:19

steve-the-edwards requested a review from rjrjr June 13, 2025 20:19

With CSR Yield before applying first Action if deferrable

4601a04

A deferrable action is one that can wait a main thread post - such as from an async trigger (Worker). Not a UI action. To allow other actions to queue up, so they can be drained.

steve-the-edwards force-pushed the sedwards/update-csr branch from 455068b to 4601a04 Compare June 13, 2025 20:33

Apply changes from ktLintFormat

c2a068f

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

1311: Enable applying Multiple Actions on immediate dispatcher for `CONFLATE_STALE_RENDERINGS` #1344

1311: Enable applying Multiple Actions on immediate dispatcher for `CONFLATE_STALE_RENDERINGS` #1344

Uh oh!

steve-the-edwards commented Jun 13, 2025 •

edited

Loading

Uh oh!

rjrjr Jun 13, 2025

Uh oh!

rjrjr Jun 13, 2025

Uh oh!

steve-the-edwards Jun 13, 2025

Uh oh!

steve-the-edwards Jun 13, 2025

Uh oh!

rjrjr Jun 13, 2025

Uh oh!

rjrjr Jun 13, 2025

Uh oh!

steve-the-edwards Jun 13, 2025

Uh oh!

rjrjr Jun 13, 2025

Uh oh!

steve-the-edwards Jun 13, 2025

Uh oh!

steve-the-edwards Jun 13, 2025

Uh oh!

rjrjr Jun 13, 2025

Uh oh!

steve-the-edwards Jun 13, 2025

Uh oh!

steve-the-edwards commented Jun 13, 2025

Uh oh!

Uh oh!


		collectionJob.cancel()

		// 2 renderings (initial and then the update.) Not 5 renderings.

                   }
                 }
+                //
+                // /**

1311: Enable applying Multiple Actions on immediate dispatcher for CONFLATE_STALE_RENDERINGS #1344

Are you sure you want to change the base?

1311: Enable applying Multiple Actions on immediate dispatcher for CONFLATE_STALE_RENDERINGS #1344

Uh oh!

Conversation

steve-the-edwards commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steve-the-edwards commented Jun 13, 2025

Uh oh!

Uh oh!

1311: Enable applying Multiple Actions on immediate dispatcher for `CONFLATE_STALE_RENDERINGS` #1344

1311: Enable applying Multiple Actions on immediate dispatcher for `CONFLATE_STALE_RENDERINGS` #1344

steve-the-edwards commented Jun 13, 2025 •

edited

Loading