Skip to content

Performance Article: Sharing logic in child features issue #3631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 26, 2025

Conversation

arnauddorgans
Copy link
Contributor

I have found a bug when applying what is described in the Performance Article in the Sharing logic in child features section

Calling Child().reduce may break cancellation logic, while calling self.reduce makes it work perfectly

I have written a sample app to reproduce the issue, you can uncomment the code to fix the issue.

Child().reduce self.reduce
Simulator Screenshot - iPhone 16 - 2025-03-21 at 01 40 15 simulator_screenshot_4D98112C-F707-49F6-906A-0BBF833E8971

But I am not sure about the performances of self.reduce against .send(.child) tho

Full app code
import SwiftUI
import ComposableArchitecture

@main
struct TCABugApp: App {
    static let store: StoreOf<Feature> = .init(initialState: .init(), reducer: { Feature() })

    var body: some Scene {
        WindowGroup {
            FeatureView(store: Self.store)
        }
    }
}

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

    var body: some View {
        VStack {
            ForEach(store.scope(state: \.sub, action: \.sub)) {
                SubFeatureView(store: $0)
            }
        }
        .onAppear {
            send(.onAppear)
        }
    }
}

struct SubFeatureView: View {
    let store: StoreOf<SubFeature>
    
    var body: some View {
        if store.isLoaded {
            Text("OK")
        } else {
            ProgressView()
        }
    }
}

@Reducer
struct Feature {
    @ObservableState
    struct State {
        var sub: IdentifiedArrayOf<SubFeature.State> = [
            .init(id: 1),
            .init(id: 2),
            .init(id: 3)
        ]
    }

    enum Action: ViewAction {
        case sub(IdentifiedActionOf<SubFeature>)
        case view(ViewAction)
        
        enum ViewAction {
            case onAppear
        }
    }

    var body: some Reducer<State, Action> {
        Reduce { state, action in
            switch action {
            case .view(.onAppear):
//                return .merge(
//                    state.sub.map {
//                        reduce(into: &state, action: .sub(.element(id: $0.id, action: .fetch)))
//                    }
//                )
                return .merge(state.sub.indices.map { index in
                    let id = state.sub[index].id
                    return SubFeature().reduce(into: &state.sub[index], action: .fetch).map {
                        Action.sub(.element(id: id, action: $0))
                    }
                })
            case .sub:
                return .none
            }
        }
        .forEach(\.sub, action: \.sub) {
            SubFeature()
        }
    }
}

@Reducer
struct SubFeature {
    @ObservableState
    struct State: Identifiable {
        let id: Int
        fileprivate(set) var isLoaded: Bool = false
    }
    
    enum Action {
        case fetch
        case fetchResponse(Result<Void, Error>)
    }
    
    enum CancelID {
        case fetch
    }
    
    @Dependency(\.continuousClock) var clock
    
    var body: some ReducerOf<Self> {
        Reduce { state, action in
            switch action {
            case .fetch:
                return .run { send in
                    try await clock.sleep(for: .seconds(1))
                    await send(.fetchResponse(.success(())))
                } catch: { error, send in
                    await send(.fetchResponse(.failure(error)))
                }
                .cancellable(id: CancelID.fetch, cancelInFlight: true)
            case let .fetchResponse(result):
                switch result {
                case .success:
                    state.isLoaded = true
                case .failure:
                    break
                }
                return .none
            }
        }
    }
}

@arnauddorgans arnauddorgans changed the title Performance child action article may produce bug Performance Article: Sharing logic in child features issue Mar 21, 2025
Copy link
Member

@stephencelis stephencelis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

@stephencelis stephencelis merged commit 7b2e742 into pointfreeco:main Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants