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

Deprecate partition, add splitBy method that multicasts the source sequence #3807

Closed
trxcllnt opened this issue Jun 6, 2018 · 6 comments
Closed
Assignees
Labels
feature PRs and issues for features

Comments

@trxcllnt
Copy link
Member

trxcllnt commented Jun 6, 2018

Related to #3797, the ideal replacement would be a method that mutlicasts the source sequence and returns two Observable "lanes", and sends values down one or the other lane based on the predicate. This is conceptually similar to groupBy with only two groups, except it returns an Array of the grouped Observables instead of an Observable of the groups since the groups can be known ahead of time.

@lmanzke
Copy link

lmanzke commented Dec 9, 2019

Seconded. To my mind, the partition operator is not fully useful yet: There needs to be another one that does not subscribe twice.

For trivial examples like in the docs, it works totally fine. Behind the scenes however, it subscribes to the observable behind partition twice which might double the work the code does.

In my concrete case, I followed the docs to split success and error cases. However, instead of calculating if a number is odd or even, I did an API call (with flatMap / catchError in case anything goes wrong). In my opinion, this is quite a common use case. If you now partition this by success / failure and subscribe to both partitioned streams, the API call is actually performed twice because there are two subscriptions that are merged back into one. This might be undesirable because it could double the load on the server (if there is no caching in place).

For this scenario, I might be missing a better operator. In this case, I am sorry. However, the splitBy that is described here seems like a perfect match for the situation I'm facing. As a workaround, i am using a custom operator that uses two subjects and passes elements on correctly, but I doubt this is production-ready yet.

@cartant
Copy link
Collaborator

cartant commented Dec 9, 2019

FWIW, I implemented this, but never got around to discussing it or opening a PR: https://github.com/cartant/rxjs-etc/blob/master/source/operators/splitBy.ts

@lmanzke
Copy link

lmanzke commented Dec 9, 2019

That looks very promising, thanks. Maybe it would be a good idea to merge it back 😄

@nathggns
Copy link

Would be good to get splitBy into rxjs.

@benlesh benlesh added the AGENDA ITEM Flagged for discussion at core team meetings label May 4, 2021
@benlesh
Copy link
Member

benlesh commented May 5, 2021

Core Team Meeting: Have @cartant move this into core, whenever he gets the chance. It seems like there's a good deal of interest.

@benlesh benlesh removed the AGENDA ITEM Flagged for discussion at core team meetings label May 5, 2021
@cartant cartant mentioned this issue May 5, 2021
40 tasks
@benlesh
Copy link
Member

benlesh commented May 21, 2021

Okay, after a PR and some careful deliberation, we decided we aren't going to pursue getting this into core right now. For reasons that are outlined here: #6388 (comment)

@benlesh benlesh closed this as completed May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature PRs and issues for features
Projects
None yet
Development

No branches or pull requests

5 participants