pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.25k stars 1.42k forks source link

Cancellable issue with children #2888

Closed arnauddorgans closed 6 months ago

arnauddorgans commented 6 months ago

Description

I have an issue when a parent feature contains two child features with a cancellable effect. The cancellable effect with ID is propagated between the two child features. I made a simple app to reproduce the issue. It may be a feature, but it seems strange that a child feature can impact neighbors.

Screenshot 2024-03-05 at 12 35 30 Screenshot 2024-03-05 at 12 35 56

I expect both children to complete their tasks instead of canceling the other.

Checklist

Expected behavior

Both child features are loaded

Actual behavior

One child feature is cancelled

Steps to reproduce

import SwiftUI
import ComposableArchitecture

@main
struct TCABugApp: App {
    var body: some Scene {
        WindowGroup {
            ContentView(store: .init(initialState: .init()) {
                MainFeature(firstChild: .init(), secondChild: .init())
            })
        }
    }
}

@Reducer
struct MainFeature {
    @ObservableState
    struct State: Equatable {
        var firstChild: ChildFeature.State = .init(title: "First Child")
        var secondChild: ChildFeature.State = .init(title: "Second Child")
    }

    enum Action: ViewAction {
        case firstChild(ChildFeature.Action)
        case secondChild(ChildFeature.Action)

        case view(ViewAction)

        enum ViewAction {
            case onAppear
        }
    }

    let firstChild: ChildFeature
    let secondChild: ChildFeature

    var body: some Reducer<State, Action> {
        Scope(state: \.firstChild, action: \.firstChild) {
            firstChild
        }
        Scope(state: \.secondChild, action: \.secondChild) {
            secondChild
        }
        Reduce { state, action in
            switch action {
            case .view(.onAppear):
                return fetchEffect(state: &state)
            case .firstChild, .secondChild:
                return .none
            }
        }
    }

    private func fetchEffect(state: inout State) -> Effect<Action> {
        return .merge(
            firstChild.reduce(into: &state.firstChild, action: .fetch).map(Action.firstChild),
            secondChild.reduce(into: &state.secondChild, action: .fetch).map(Action.secondChild)
        )
    }
}

@Reducer
struct ChildFeature {
    @ObservableState
    struct State: Equatable {
        let title: String

        var isLoading: Bool = false
        var isLoaded: Bool = false
    }

    enum Action {
        case fetch
        case fetchResponse(success: Bool)
    }

    enum CancelID {
        case fetch
    }

    var body: some Reducer<State, Action> {
        Reduce { state, action in
            switch action {
            case .fetch:
                state.isLoading = true
                return .run { send in
                    try await Task.sleep(for: .seconds(1))
                    await send(.fetchResponse(success: true))
                } catch: { _, send in
                    await send(.fetchResponse(success: false))
                }
                .cancellable(id: CancelID.fetch, cancelInFlight: true)
            case let .fetchResponse(success: success):
                state.isLoading = false
                state.isLoaded = success
                return .none
            }
        }
    }
}

@ViewAction(for: MainFeature.self)
struct ContentView: View {
    let store: StoreOf<MainFeature>

    var body: some View {
        ScrollView {
            VStack(spacing: 8) {
                ChildView(store: store.scope(state: \.firstChild, action: \.firstChild))
                ChildView(store: store.scope(state: \.secondChild, action: \.secondChild))
            }
        }
        .onAppear {
            send(.onAppear)
        }
    }
}

struct ChildView: View {
    let store: StoreOf<ChildFeature>

    var body: some View {
        VStack {
            if store.isLoading {
                ProgressView()
            } else {
                Text(store.isLoaded ? "✅" : "❌")
            }
        }
        .padding()
        .background(.ultraThinMaterial)
        .clipShape(RoundedRectangle(cornerRadius: 8))
    }
}

The Composable Architecture version information

1.9.2

Destination operating system

iOS 17

Xcode version information

Xcode 15.2

Swift Compiler version information

swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5)
Target: arm64-apple-macosx14.0
stephencelis commented 6 months ago

Hi @arnauddorgans. You are reusing the exact same cancel ID for effects, which are both created and executed in the parent feature using cancelInFlight. This means that when the second effect is created it will immediately cancel the first.

You can make the cancel IDs unique to fix things:

 @Reducer
 struct ChildFeature {
     …
-    enum CancelID {
-        case fetch
+    enum CancelID: Hashable {
+        case fetch(UUID)
     }
+
+    let id = UUID()

     var body: some Reducer<State, Action> {
         Reduce { state, action in
             switch action {
             case .fetch:
                 state.isLoading = true
                 return .run { send in
                     …
                 }
-                .cancellable(id: CancelID.fetch, cancelInFlight: true)
+                .cancellable(id: CancelID.fetch(self.id), cancelInFlight: true)
             …
         }
     }
 }

But I would strongly suggest moving the onAppear to the non-optional child features instead of invoking the reducers from the parent.

stephencelis commented 6 months ago

Because this isn't a bug in the library, I'm going to convert to a discussion.