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(first): force unsubscription logic on complete and error #2463

Merged
merged 1 commit into from
Jun 14, 2017
Merged

fix(first): force unsubscription logic on complete and error #2463

merged 1 commit into from
Jun 14, 2017

Conversation

mpodlasin
Copy link
Contributor

@mpodlasin mpodlasin commented Mar 15, 2017

Description

Force unsubscription logic when operator completes or errors, so that source Observable is only subscribed as long as it has to, even when combined with operators that do not immediately unsubscribe
from 'first' when it completes or errors.

Introduces test-only doNotUnsubscribe operator, which simulates operators that are unreliable with managing subscriptions (like switchMap for example - see #2459).

Related issue (if exists):

Closes #2455

This issue is a concrete example of more general one: #2459

Force unsubscription logic when operator completes or errors, so
that source Observable is only subscribed as long as it has to,
even when combined with operators that do not immediately unsubscribe
from 'first' when it completes or errors.

Closes #2455
@coveralls
Copy link

coveralls commented Mar 15, 2017

Coverage Status

Coverage increased (+0.001%) to 97.689% when pulling a55f8da on Podlas29:first-with-switch-map-fix into 69d051b on ReactiveX:master.

@benlesh
Copy link
Member

benlesh commented Mar 21, 2017

While this is a fix, it also might be a breaking change for some. @staltz, @kwonoj what do you think? Next major version?

@staltz
Copy link
Member

staltz commented Mar 23, 2017

Yeah it's major. If it could possibly break existing behavior for anyone out there, it's a breaking change. There's no shame in bumping the major num. In the changelog we can say "this will probably not affect you, but" type of message.

@benlesh benlesh merged commit af747f0 into ReactiveX:master Jun 14, 2017
benlesh pushed a commit that referenced this pull request Jun 14, 2017
#2463)

Force unsubscription logic when operator completes or errors, so that source Observable is only subscribed as long as it has to, even when combined with operators that do not immediately unsubscribe from 'first' when it completes or errors.

Closes #2455

BREAKING CHANGE: unsubscription cadence has changed for `first`, this means side-effects related to unsubscription may occur at a different time than in previous versions.
benlesh added a commit that referenced this pull request Jun 14, 2017
benlesh added a commit that referenced this pull request Jun 14, 2017
@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 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

Successfully merging this pull request may close these issues.

4 participants