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

zip does not do anything if passed all iterators #4304

Closed
benlesh opened this issue Oct 30, 2018 · 1 comment · Fixed by #5688
Closed

zip does not do anything if passed all iterators #4304

benlesh opened this issue Oct 30, 2018 · 1 comment · Fixed by #5688
Assignees
Labels
bug Confirmed bug

Comments

@benlesh
Copy link
Member

benlesh commented Oct 30, 2018

Ages ago, I had this amazing idea to give zip special handling for iterables, such that, as it was "zipping" it would attempt to only next out values from the iterable as needed.

Unfortunately, this has introduced a lot of complexity in that operator as well as a bug...

zip('abc', 'def').subscribe(x => console.log); // this does nothing.

It should yield:

['a', 'd']
['b', 'e']
['c', 'f']

But it does nothing.

While it seemed like a clever idea at the time, I now think it deviates from the "push only" nature of RxJS, and this behavior should be removed in the upcoming v7.

@benlesh benlesh added the AGENDA ITEM Flagged for discussion at core team meetings label Oct 30, 2018
@benlesh
Copy link
Member Author

benlesh commented Dec 19, 2018

Core team: Agree to go back to v4 style behavior and doing from on the iterable passed in.

@benlesh benlesh removed the AGENDA ITEM Flagged for discussion at core team meetings label Dec 19, 2018
@benlesh benlesh self-assigned this Sep 2, 2020
@benlesh benlesh added the bug Confirmed bug label Sep 2, 2020
benlesh added a commit to benlesh/rxjs that referenced this issue Sep 2, 2020
…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 added a commit to benlesh/rxjs that referenced this issue Sep 3, 2020
…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 added a commit that referenced this issue Sep 3, 2020
…e sources (#5688)

* fix(zip): zip operators and functions are now able to zip all iterable 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 #4304

* chore: update golden file

* chore: I hate ts-api-guardian sometimes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant