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

skipUntil unsubscription potential bug #577

Closed
staltz opened this issue Oct 22, 2015 · 3 comments
Closed

skipUntil unsubscription potential bug #577

staltz opened this issue Oct 22, 2015 · 3 comments

Comments

@staltz
Copy link
Member

staltz commented Oct 22, 2015

I'm writing unsubscription tests for skipUntil, and I expect this to be true

var e1 =   hot('--a--b--c--d--e--|');
var e1subs =   '^                !'; // Notice this
var skip = hot('-');
var skipSubs = '^';
var expected = '-';

but in reality RxJS Next behaves like this

var e1 =   hot('--a--b--c--d--e--|');
var e1subs =   '^'; // Notice this
var skip = hot('-');
var skipSubs = '^';
var expected = '-';

RxJS 4, on the other hand, unsubscribes from the source when the source completes, see this JSBin: http://jsbin.com/vumuxurabe/1/edit?js,console.

Should I consider this a bug in RxJS Next?

@kwonoj
Copy link
Member

kwonoj commented Oct 22, 2015

I feel same, subscription need to be unsubscribed.

staltz pushed a commit to staltz/RxJSNext that referenced this issue Oct 23, 2015
Fix operator skipUntil to automatically unsubscribe from its
source whenever it completes. This is to conform RxJS Next with RxJS 4.

Resolves issue ReactiveX#577.
staltz pushed a commit to staltz/RxJSNext that referenced this issue Oct 23, 2015
Fix operator takeUntil to automatically unsubscribe the notifier when it
completes. This is to conform RxJS Next with RxJS 4.

Somewhat related to issue ReactiveX#577.
staltz pushed a commit to staltz/RxJSNext that referenced this issue Oct 27, 2015
Fix operator takeUntil to automatically unsubscribe the notifier when it
completes. This is to conform RxJS Next with RxJS 4.

Somewhat related to issue ReactiveX#577.
benlesh pushed a commit that referenced this issue Oct 27, 2015
Fix operator takeUntil to automatically unsubscribe the notifier when it
completes. This is to conform RxJS Next with RxJS 4.

Somewhat related to issue #577.
staltz added a commit to staltz/RxJSNext that referenced this issue Oct 29, 2015
Change SkipUntilSubscriber to give the destination subscriber to the
constructor in Subscriber. Adapt SkipUntilSubscriber to handle
unsubscription-on-complete correctly, by overriding unsubscribe().

Addresses ReactiveX#577.
staltz pushed a commit to staltz/RxJSNext that referenced this issue Oct 30, 2015
Fix operator skipUntil to automatically unsubscribe from its
source whenever it completes. This is to conform RxJS Next with RxJS 4.

Resolves issue ReactiveX#577.
staltz added a commit to staltz/RxJSNext that referenced this issue Oct 30, 2015
Change SkipUntilSubscriber to give the destination subscriber to the
constructor in Subscriber. Adapt SkipUntilSubscriber to handle
unsubscription-on-complete correctly, by overriding unsubscribe().

Addresses ReactiveX#577.
@benlesh
Copy link
Member

benlesh commented Nov 6, 2015

@staltz ... I'm going through issues and making sure what needs to be closed is closed... is this issue resolved now with #586?

@staltz
Copy link
Member Author

staltz commented Nov 7, 2015

Yes, let's close.

@staltz staltz closed this as completed Nov 7, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants