Skip to content
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

Cascade does not complete if the last observable completes immediately and synchronously #218

Open
scalbatty opened this issue Jun 25, 2019 · 3 comments

Comments

@scalbatty
Copy link

Hi everyone,

I'm using the cascade operator in many kinds of scenarios and I'm very grateful for it.

There is an issue though, when for some reason the last observable completes immediately (i.e. in a synchronous manner), the resulting cascading observable will not complete.

I believe that is because of the if initialized test when subscribing to underlying observables: https://github.com/RxSwiftCommunity/RxSwiftExt/blob/master/Source/RxSwift/cascade.swift#L57

For example, if the last observable in the list is a .just(whatever), the cascade will never complete.

It looks like it's a regression that was introduced with commit 4ba2dd6

The following test can help understand the issue (it's red at the moment):

    func testSynchronousCompletion() {
        let xs = scheduler.createHotObservable([
            .next(110, 1),
            .next(180, 2),
            .next(230, 3),
            .next(270, 4),
            .next(340, 5),
            .completed(200)
            ])
            .asObservable()

        let ys = scheduler.createHotObservable([
            .next(100, 21),
            .completed(210, Int.self)
            ])
            .asObservable()

        let zs = Observable<Int>.just(31)

        let res = scheduler.start { () -> Observable<Int> in
            Observable.cascade([xs, ys, zs])
        }

        XCTAssertEqual(res.events, [
            .next(200, 31),
            .completed(200)
            ]) // <--- it's just `[.next(200, 31)`
    }
@freak4pc
Copy link
Member

Hey :)
I'm not too knowledgable with this operator - but since that commit is over 2 years old, I wouldn't want to change the behavior without being positive about this.

@fpillet could you comment ?

@fpillet
Copy link
Member

fpillet commented Jun 26, 2019

Ouch, the latest commit was there to harden Cascade, not weaken it. Thanks for the detailed error case, I'll fix it.

@scalbatty
Copy link
Author

Thanks so much @fpillet!

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

No branches or pull requests

3 participants