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

Dispose the stream if last subscriber terminates. #33

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Dispose the stream if last subscriber terminates. #33

wants to merge 2 commits into from

Conversation

CihanSari
Copy link

In our workflow server is streaming until nobody is listening to. We don't get this information with the current observable. I had to fix this issue in a very similar fashion to this pull request at work. I have tried to isolate the necessary change to address the issue. I did not test the changes, especially because I have updated the rxjs requirement at work, and this meant a complete different interface. I hope this patch delivers the "no more subscribers" information to stream source.

@kondi
Copy link
Owner

kondi commented Jul 18, 2018

Hi!

Thanks for the finding!
What does the finally operator help in this case? Can't we just return a call to cancel as unsubscribe function from the Observable factory function?

@CihanSari
Copy link
Author

Hi kondi!

Yes, that would be a solution. My old PR had the issue with canceling the call prematurely on first subscriber's unsubscription. I have merged the changes that I made for work and updated the PR. I had to clean A LOT OF code, and I couldn't test this version if it works, yet. However, I hope it gives you the idea.

Main decision to make is whether or not to replay the stream on consecutive subscribers to the shared observable.

@CihanSari
Copy link
Author

This solves only half the problem, as our server still keeps publishing without any care on the call status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants