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

Tap kills the Subject on unsubscription #7078

Open
voliva opened this issue Oct 3, 2022 · 2 comments
Open

Tap kills the Subject on unsubscription #7078

voliva opened this issue Oct 3, 2022 · 2 comments
Assignees
Labels
7.x Issues and PRs for version 6.x 8.x Issues and PRs for version 8.x AGENDA ITEM Flagged for discussion at core team meetings bug Confirmed bug

Comments

@voliva
Copy link
Contributor

voliva commented Oct 3, 2022

Describe the bug

When using tap passing in a Subject directly (i.e. without using { next, error, complete }), if the resulting observer creates a subscription and unsubscribes from it, then the Subject is also unsubscribed: If a new subscriber comes, it will throw an ObjectUnsubscribedError.

I've bisected this happened somewhere in between 7.2.0 and 7.3.0

Expected behavior

When the resulting subscription is closed, the Subject should not be killed: The new subscription should work just as fine as the initial one.

Reproduction code

const subject = new Subject();

interval(1000).pipe(
  tap(subject),
  take(2),
  repeat(2)
).subscribe(v => console.log(v))

Reproduction URL

https://stackblitz.com/edit/rxjs-a9l4j2?file=index.ts

Version

7.3.0

Environment

No response

Additional context

No response

@voliva
Copy link
Contributor Author

voliva commented Oct 3, 2022

I see this was introduced in #6527

@voliva
Copy link
Contributor Author

voliva commented Oct 5, 2022

The API change on tap was quite substantial when used with Subjects, and I think it was a valid use case. tap accepted Observer, and a Subject is an Observer - sometimes it was used to replicate a stream for different reasons (e.g. circular references)

Now not only tap will kill the Subject by calling unsubscribe() on it when one of the underlying subscription closes, but it will also create lingering subscriptions every time someone subscribes to the resulting observable, because from that PR, tap also now calls .subscribe() on the subject when someone subscribes.

I'm not really sure what would be the best solution for this now. Ideally that change should've happened in a major release (8.x), or maybe the functions that it would call from observers should've been in past tense (.subscribed(), .unsubscribed(), .finalized()) instead. Another option would be to make special cases for Subjects, but I'm not sure how would this work for all of them.

@jakovljevic-mladen jakovljevic-mladen added the AGENDA ITEM Flagged for discussion at core team meetings label Jul 12, 2023
@benlesh benlesh self-assigned this Mar 6, 2024
@benlesh benlesh added bug Confirmed bug 8.x Issues and PRs for version 8.x 7.x Issues and PRs for version 6.x labels Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.x Issues and PRs for version 6.x 8.x Issues and PRs for version 8.x AGENDA ITEM Flagged for discussion at core team meetings bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

3 participants