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.24k stars 1.42k forks source link

Potential skip of actions of cancellable Effect #121

Closed mpsnp closed 4 years ago

mpsnp commented 4 years ago

If underlying Effect produces Action immediately on subscription (for example to indicate that loading started), these actions will be lost because PassthroughSubject was not subscribed to yet:

https://github.com/pointfreeco/swift-composable-architecture/blob/115b7c68d533983bdfd4a0b7d6348b1cd161e2f3/Sources/ComposableArchitecture/Effects/Cancellation.swift#L42

mbrandonw commented 4 years ago

Hi @mpsnp, thanks for the issue.

Is it possible for you to share some code to show the problem? Perhaps a simplified reducer that shows the current behavior and a description of what you expect to happen?

Thanks

mpsnp commented 4 years ago

Hm.. Looks like it's Combine's implementation detail. I'm used to RxSwift. But let's check this case:


struct TestState: Equatable {
    var isLoading: Bool = false
    var result: String?
}

enum TestAction: Equatable {
    case load
    case loadingStarted
    case receivedResult(String)
}

let testReducer: Reducer<TestState, TestAction, Void> = .init { state, action, env in
    switch action {
    case .load:
        struct ID: Hashable {}
        return Effect
            .concatenate(
                Effect(value: .loadingStarted),
                Effect(value: .receivedResult("Some stuff"))
                    .delay(for: 2, scheduler: DispatchQueue.main)
                    .eraseToEffect()
            )
            .cancellable(id: ID(), cancelInFlight: true)
    case .loadingStarted:
        state.isLoading = true
        return .none
    case .receivedResult(let result):
        state.result = result
        return .none
    }
}

Because at the moment of the subscription I've referenced above, in case of RxSwift .loadingStarted will be emitted immediately upon subscription, and as nothing listens to the subject itself yet, RxSwift will drop those events.

Combine, don't emit elements before they are requested by subscriber, so in this case everything alright.

So, sorry about taking your time for this issue.