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

combineLatest of an empty array completes immediately #1879

Closed
5 tasks done
fb55 opened this issue Feb 16, 2019 · 5 comments
Closed
5 tasks done

combineLatest of an empty array completes immediately #1879

fb55 opened this issue Feb 16, 2019 · 5 comments

Comments

@fb55
Copy link

fb55 commented Feb 16, 2019

Short description of the issue:

Calling combineLatest on an empty collection does not produce any elements.

Expected outcome:

Intuitively, I expect combineLatest to be an updating map of the array, replacing the observables with its elements. Given that there are no elements, I expect to receive an empty array.

What actually happens:

The sequence terminates immediately.

Self contained code example that reproduces the issue:

Very basic example, adapted from #1205:

Observable.combineLatest([Observable<String>]())
    .bind { _ in print("next!") } // never called

The code I'm actually running:

subscriptions = subscribers
    .flatMapLatest {
        Observable.combineLatest($0.map { $0.feedSubscriptions })
    }
    .scan { [diffSubs] current, newSubs in
		diffSubs(current, newSubs)
    }
    .map { $0.changes }

I never reach the point where there are no subscriptions.

RxSwift/RxCocoa/RxBlocking/RxTest version/commit

4.3.1

Platform/Environment

  • iOS

How easy is to reproduce? (chances of successful reproduce after running the self contained code)

  • easy, 100% repro

Xcode version:

10.1

Installation method:

  • CocoaPods

I have multiple versions of Xcode installed:
(so we can know if this is a potential cause of your issue)

  • no

Level of RxSwift knowledge:
(this is so we can understand your level of knowledge
and formulate the response in an appropriate manner)

  • I have a significant code base
@kzaher
Copy link
Member

kzaher commented Feb 19, 2019

Hi @fb55 ,

I guess for this one I can imagine a lot of potential behaviors. combineLatest will combine latest elements from an array of observables.

One could argue that if that array is empty then there are no latest elements so there is nothing to return.

Please bear in mind that the implementation will only start to return values once all values are received.

0 is kind of faulty value I guess.

On the other hand I can also see a rationale why[] should be returned after the subscription. One could argue that if 0 elements need to be fetched to return the value then 0 elements are already fetched and [] should be emitted.

I would want to double check rxjs here, but we could change this in 5.0 version since this is a breaking change.

@mime29
Copy link

mime29 commented Feb 20, 2019

Observable.combineLatest($0.map { $0.feedSubscriptions })

is it not enough to just return feedSubscriptions here?
Otherwise, if at least one feedSubscriptions can be not emitting any value, you should think about a merge logic instead.
Another approach is to go with BehaviorRelay with an initial value that will automatically trigger the combineLatest.

About:

Observable.combineLatest([Observable<String>]())
    .bind { _ in print("next!") } // never called

It's normal, none of the Observable<String> are emitting any value and the bind won't be triggered without all of them emitting at least once.

@fb55
Copy link
Author

fb55 commented Mar 27, 2019

Is it not enough to just return feedSubscriptions here?

feedSubscriptions are observable themselves, which can change their data needs depending on network state and user interactions. All of the data sources are functionally equivalent to behavior relays. Subscribers are removed once they have no data needs anymore, which triggers the current issue once all subscribers are removed.

It's normal, none of the Observable<String> are emitting any value and the bind won't be triggered without all of them emitting at least once.

That is a good explanation, but might not be very intuitive. If you expect an "updating map of the array" (see above), then the current behavior is surprising.

I would also want to argue that returning an array is more useful behavior.

@sharshuv-quotient
Copy link

Hey, I agree with @fb55, I would expect this stream to emit an empty array.
This is useful in cases where the array isn't predetermined (i.e. represents some sort of request response / user input), than you want to apply some asynchronous action to each element (i.e. call a function that returns an Observable).
If for some reason the input was empty (no items in the array) the stream will be dead. (completed) without any emission, which causes an hard to debug "deadlock".
You can obviously solve this by checking if the array is empty before passing this to the combineLatest function, but why shouldn't the combineLatest function do it itself?

If you disagree I challenge you to give an opposite example, in which the user will prefer the return value to be a never-emitting observable that completes immediately.

@freak4pc
Copy link
Member

freak4pc commented Jun 1, 2020

This is almost a year and a half old, it would be better if you could open a new issue to track this. Thanks ! @sharshuv-quotient

@ReactiveX ReactiveX locked and limited conversation to collaborators Jun 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants