rxjs
rxjs copied to clipboard
Should `groupBy` keep connections after the result subscription is unsubscribed?
We currently have special case handling for groupBy
that purposefully introduces the following seemingly non-deterministic behavior:
import { tap, interval, groupBy } from 'rxjs';
const subscription = interval(100)
.pipe(
groupBy((n) => (n % 2 === 0 ? 'even' : 'odd')),
tap((grouped$) => {
grouped$.subscribe((n) => console.log(grouped$.key, n));
})
)
.subscribe();
// Toggle to true or false to try different scenarios.
const unsubImmediately = true;
// Unsubscribe here and it stops! (no groups have been recieved yet).
// Result subscription was unsubscribed before any groups could be subscribed to.
if (unsubImmediately) {
subscription.unsubscribe();
} else {
setTimeout(() => {
// Here the source subscription goes on forever!!
// We have 2 groups, and someone's subscribed to them in a way they're not unsubscribed
// when the result subscription is.
subscription.unsubscribe();
}, 1000);
}
Basically, if anything subscribes to a grouped observable and doesn't unsubscribe, unsubscribing from the outer subscription will not unsubscribe from the source. If there are no subscriptions to the grouped observables, then the subscription to the source will be unsubscribed.
What is the issue?
This is what groupBy
has been doing since long before I came onto the project. However, my gut tells me this behavior is inconsistent with the rest of the framework, saving the default behavior of shareReplay()
, which has itself caused a lot of confusion. The behavior, as demonstrated by the Stackblitz above, can be hard to reason about if event sources or subscription management isn't deterministic.
This behavior is such a one-off thing it's wildly unlike any other operator we export.
Proposed change
I feel that outer subscriptions to the resulting observable returned by groupBy
should unsubscribe/disconnect the source from the groups when unsubscribed. In this case, the grouped observables, if still subscribed, could either: A) unsubscribe on their own, or B) emit an error saying they were disconnected. I feel like A
is probably the proper solution, but we could even make that configurable.
(This would be a BREAKING change for groupBy
, in the context that grouped observables would shut down if the source is disconnected)
Quick thoughts, no quorum: Deprecate it (@kwonoj), kill it with fire (@benlesh)
Core Team: We have quorum! Kill it!
ref https://github.com/ReactiveX/rxjs/pull/6434#discussion_r641849113
Core Team Round 2: Still Quorum.
Also:
- Add documentation to groupBy warning people about this weirdness.
- There should be a lint rule, @cartant! This is your fault! (We love you anyway)