Effect is not correctly cancelled when calling a child action from the parent reducer

[b1ackturtle]

Description

When embedding child reducers in a parent reducer using forEach, and calling a child action from the parent reducer by invoking the child reducer directly (without using the send effect), the test case for the parent reducer fails because the effect in the child reducer is still running.

Checklist

• I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
• If possible, I've reproduced the issue using the main branch of this package.
• This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

We expect the test case in the following reproducing project to pass.

Actual behavior

The following error occurs, and the test does not pass.

failed - An effect returned for this action is still running. It must complete before the end of the test.

Reproducing project

The implementation and test code for reproduction are as follows:

@Reducer
struct Parent {
    struct State: Equatable {
        var childs: IdentifiedArrayOf<Child.State>

        init() {
            self.childs = .init(uniqueElements: [.init()])
        }
    }

    enum Action {
        case onAppear
        case childs(IdentifiedActionOf<Child>)
    }

    var body: some ReducerOf<Self> {
        Reduce { state, action in
            switch action {
            case .onAppear:
                return Child().reduce(into: &state.childs[0], action: .refresh)
                    .map { [id = state.childs[0].id] in
                        Action.childs(.element(id: id, action: $0))
                    }

            case .childs:
                return .none
            }
        }
        .forEach(\.childs, action: \.childs) {
            Child()
        }
    }
}

@Reducer
struct Child {
    struct State: Equatable, Identifiable {
        var id = UUID()
    }

    enum Action {
        case refresh
        case cancel
    }

    private enum CancelID: Hashable {
        case send(State.ID)
        case sleep(State.ID)
    }

    var body: some ReducerOf<Self> {
        Reduce { state, action in
            switch action {
            case .refresh:
                return .merge(
                    .run { send in
                        try await Task.sleep(for: .seconds(10))
                    }.cancellable(id: CancelID.sleep(state.id), cancelInFlight: true),
                    .run { send in
                        await send(.cancel)
                    }.cancellable(id: CancelID.send(state.id), cancelInFlight: true)
                )

            case .cancel:
                return .merge(
                    .cancel(id: CancelID.sleep(state.id)),
                    .cancel(id: CancelID.send(state.id))
                )
            }
        }
    }
}

@MainActor
func test() async {
    let store = TestStore(initialState: Parent.State()) {
        Parent()
    }

    await store.send(.onAppear)
    await store.receive(\.childs.element)
}

If the method of calling a child action is changed from invoking the child reducer directly to using the send effect, the effect in the child reducer is correctly canceled in the test case.

switch action {
case .onAppear:
+    return .send(.childs(.element(id: state.childs[0].id, action: .refresh)))
-    return Child().reduce(into: &state.childs[0], action: .refresh)
-        .map { [id = state.childs[0].id] in
-            Action.childs(.element(id: id, action: $0))
-        }

The Composable Architecture version information

1.14.0

Destination operating system

iOS 17.5

Xcode version information

Xcode 15.4

Swift Compiler version information

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0

[mbrandonw]

Hi @b1ackturtle, this is actually working as expected, though admittedly it's not ideal. The operators that handle all the effect cancellation logic is ifLet/forEach, and here you are completely bypassing that:

return Child().reduce(into: &state.childs[0], action: .refresh)
  .map { [id = state.childs[0].id] in
    Action.childs(.element(id: id, action: $0))
  }

So it shouldn't be surprising that effect cancellation isn't working correctly.

Instead you could do something like this, though it is a bit ugly:

return EmptyReducer()
  .forEach(\.childs, action: \.childs) {
    Child()
  }
  .reduce(into: &state.childs, action: .childs(.element(id: …, action: .refresh)

Although we do largely recommend from sending actions via Effect.send as a means for sharing logic, this may be a case that it works well for you. Though, is there a reason you don't execute this refresh logic in the onAppear of the child feature?

Also, we do have some planned updates to how reducers work that will improve these kinds of things, but what the final shape will look like is not yet known.

Since this isn't an issue with the library I am going to convert it to a discussion. Please feel free to continue the conversation over there!

[b1ackturtle]

Thank you for your explanation. It was very helpful. I understand that my code bypasses the cancellation logic.

Though, is there a reason you don't execute this refresh logic in the onAppear of the child feature?

In my production code, the child reducer does not correspond to a SwiftUI view. Specifically, the parent reducer is related to UIViewControllerRepresentable for using QLPreviewController in a SwiftUI context, while the child reducer is related to QLPreviewItem. Due to this separation, the child reducer cannot effectively handle the onAppear event by itself.

What is the best practice for this case under the current version? It doesn't seem very readable, but should I use the code you mentioned?