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

fix(zip): zip operators and functions are now able to zip all iterable sources #5688

Merged
merged 3 commits into from
Sep 3, 2020

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Sep 2, 2020

Resolves an issue I outlined in #4304. Basically, once upon a time, I had a "brilliant" idea to have iterators iterated "as needed" during a zip operation. There are a few consequences to this, but the nastiest one is that you could not use zip with all-iterable sources. It also doesn't jive with the push-based nature of this library. This PR fixes all of that.

  • Simplifies zip implementations
  • Updates tests as appropriate

BREAKING CHANGE: zip operators will no longer iterate provided iterables "as needed", instead the iterables will be treated as push-streams just like they would be everywhere else in RxJS. This means that passing an endless iterable will result in the thread locking up, as it will endlessly try to read from that iterable. This puts us in-line with all other Rx implementations. To work around this, it is probably best to use map or some combination of map and zip. For example, zip(source$, iterator) could be source$.pipe(map(value => [value, iterator.next().value])).

fixes #4304

@benlesh benlesh requested review from cartant and kwonoj September 2, 2020 14:26
Copy link
Collaborator

@cartant cartant left a comment

Choose a reason for hiding this comment

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

LGTM

…e sources

BREAKING CHANGE: `zip` operators will no longer iterate provided iterables "as needed", instead the iterables will be treated as push-streams just like they would be everywhere else in RxJS. This means that passing an endless iterable will result in the thread locking up, as it will endlessly try to read from that iterable. This puts us in-line with all other Rx implementations. To work around this, it is probably best to use `map` or some combination of `map` and `zip`. For example, `zip(source$, iterator)` could be `source$.pipe(map(value => [value, iterator.next().value]))`.

fixes ReactiveX#4304
@benlesh benlesh force-pushed the Issue-4304-fix-zip-with-iterators branch from c5c2ed5 to b26eb9e Compare September 3, 2020 12:56
@benlesh benlesh merged commit 02c3a1b into ReactiveX:master Sep 3, 2020
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.

zip does not do anything if passed all iterators
2 participants