Effect.concatenate() and recursively firing effects

[ghost]

Hello :)

After much exploration on my own and digging through the forum discussions, I was shocked to find out that concatenate does not work when trying to fire effects recursively.

I'd like to suggest that an effect supporting recursiveness be added to the library because setting up, cleaning up, and dismissing scenes in a navigation stack are extremely common and very basic operations that are more easily achieved recursively.

The only alternative that I can see is to have explicit actions for starting and completing each of those 3 processes. That's 6 actions plus their reducer logic, per child scene, just to support navigation. So, if a parent scene can present 5 different child scenes, that's 30 actions in the parent scene, just to support navigation! 😳

[mbrandonw]

Hi @wtruppel, I'm not entirely sure what you mean by "concatenate does not work when trying to fire effects recursively.".

Could you give us a failing test case that demonstrates the behavior you want to have?

[mbrandonw]

We've found that concatenate(concatenate(concatenate(A, B), C), D) does not appear to execute the effects in the order (A, B, C, D). Essentially, it looks like concatenate builds a queue of effects rather than a stack of effects.

Would it be possible to provide a test case to demonstrate this behavior? It would definitely be a bug if that is the case, but when I write a test case for concat(concat(concat(A, B), C), D), I do see it emit in that order:

func testConcat() {
  enum Action { case tap, response1, response2, response3, response4 }
  let store = TestStore(
    initialState: 0,
    reducer: Reducer<Int, Action, Void> { state, action, _ in
      switch action {
      case .tap:
        return .concatenate(
          .concatenate(
            .concatenate(
              .init(value: .response1),
              .init(value: .response2)
            ),
            .init(value: .response3)
          ),
          .init(value: .response4)
        )
      case .response1:
        state = 1
        return .none
      case .response2:
        state = 2
        return .none
      case .response3:
        state = 3
        return .none
      case .response4:
        state = 4
        return .none
      }
    },
    environment: ()
  )

  store.send(.tap)
  store.receive(.response1) {
    $0 = 1
  }
  store.receive(.response2) {
    $0 = 2
  }
  store.receive(.response3) {
    $0 = 3
  }
  store.receive(.response4) {
    $0 = 4
  }
}

[ghost]

Hi @mbrandonw. Here's a small toy project with the use case we're interested in and a test that shows .concatenate not firing the effects in the expected recursive order. The toy project has 5 scenes (A, B, C, D, E), where A can present B and C, B can present C and D, C can present D and E, and E doesn't present anything. The test starts by presenting B through E in succession, and that works fine. Then the test attempts to dismiss B from A. But because B is presenting C, it has to dismiss C first. But C is presenting D... and so on.

(By the way, there's a lot of structurally identical repeated code that we plan to simplify by having protocols that the scene states need to conform to, which will take care of most of the code. For now, I just want to make sure the recursion works.)

(In case you already downloaded this, I found a bug, fixed it, and replaced the zipped file with a newer one)
ConcatTest.zip

[mbrandonw]

Hi @wtruppel, I'm sorry, but I don't have enough time to dig into such a complex example. Can you show the problem with just 2 layers?

If your problem really is identical to what is demonstrated in #547, then there really is no fix. There is no way (AFAICT) to return a .concat(a, b) from a reducer such that b waits for all the subsequent effects induced by a to complete. And if it were possible, it would come with its own complications and subtleties. Such an operation certainly shouldn't be called Effect.concatenate.

From my cursory glance of your code, I feel that this is not a good direction to go. Complicated communication patterns via synchronous effects (i.e. .cleanup, .cleanupCompleted, etc.) is not an ideal way to structure these things. Actions should be more modeled after things happening in the UI rather than concrete descriptions of what you want your reducer to do. This becomes apparent looking at the tests because they don't read as a script of things the user is doing, but instead as a raw list of atomic commands for the reducer to interpret.

In general, it is not a good pattern to share reducer logic and effects by sending around synchronous actions. Instead, you can create functions that bundle up that logic, and invoke them from multiple actions. Those functions can even live as mutating functions on the state, and soon with our work on reducer protocols such operations can naturally live on the conformances.

Hope this helps.

[ghost]

Hi @mbrandonw. Attached is a new version that only has 3 scenes (A, B, C). Same scenario as before. A -> B -> C then A attempts to dismiss B, which requires first dismissing C.

There is no way (AFAICT) to return a .concat(a, b) from a reducer such that b waits for all the subsequent effects induced by a to complete. And if it were possible, it would come with its own complications and subtleties. Such an operation certainly shouldn't be called Effect.concatenate.

Why shouldn't it be possible? It's the difference between a queue and a stack. Sure, it would be impossible to guarantee the order if async effects are involved but, for synchronous ones, that guarantee should be possible, no? It seems to me that waiting for an effect to complete should not be concatenate's responsibility but it should be possible to have a version of concatenate that arranges its effects in a stack.

Complicated communication patterns via synchronous effects (i.e. .cleanup, .cleanupCompleted, etc.) is not an ideal way to structure these things

Could you please offer an alternative? Navigation is the bread-and-butter of iOS and being able to set up, clean up, present and dismiss scenes in a recursive and reasonably easy-to-maintain manner should be a goal of the library, in my opinion. I do agree that the parent scene shouldn't have to explicitly tell its child to set itself up or clean itself up but we have been struggling (a lot!) to encapsulate these behaviours in each scene, while using TCA. To be honest, I've been eagerly awaiting your series of videos on navigation using TCA because it's been a big hurdle so far (we had things working until deep-linking obliterated the navigation stack without giving scenes a chance to clean themselves up).

In general, it is not a good pattern to share reducer logic and effects by sending around synchronous actions. Instead, you can create functions that bundle up that logic, and invoke them from multiple actions. Those functions can even live as mutating functions on the state,

That's what our implementation does for setup and cleanup, but our code only worked "one-level deep", ie, with no recursion. If you present a scene then move out of it, it got set up and then cleaned up correctly, even with async effects as part of that setup and cleanup. But the moment we needed to do this recursively, all hell broke loose.

and soon with our work on reducer protocols such operations can naturally live on the conformances.

I'm looking forward to those improvements to the library.

Thanks again for your time responding!

ConcatTest-2.zip

[mbrandonw]

Why shouldn't it be possible?

If you can come up with an implementation I would be interested in seeing it.

Effect.concatenate only has access to the effects directly passed to it. But what you are asking for is the effects that are started from sending other actions into the store. The .concatenate operator doesn't have access to those effects.

Could you please offer an alternative?

We don't have anything immediately usable to share right now. We have previously shared some tools that we have been honing for many months (here's a gist), and recently we shared some experiments with using NavigationStack in TCA (see here).

The main observation behind those experiments is that navigation is driven by optional (or enum) state. When state becomes non-nil, the drill-down/sheet/alert/etc is presented. On the reducer level, we tag every effect from the child domain with an identifier so that when state is nil'd out we cancel all of those effects. That is the clean up phase.