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

unsubscribe is not work #370

Closed
ntduan opened this issue Nov 1, 2018 · 4 comments · Fixed by #373
Closed

unsubscribe is not work #370

ntduan opened this issue Nov 1, 2018 · 4 comments · Fixed by #373
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability.

Comments

@ntduan
Copy link
Contributor

ntduan commented Nov 1, 2018

For api-observable, I try unsubscribe bestNumber, but it not send chain_unsubscribeNewHead request

@ntduan
Copy link
Contributor Author

ntduan commented Nov 1, 2018

image
ReactiveX/rxjs#3336
I think wo can use publishReplay and refCount

@jacogr
Copy link
Member

jacogr commented Nov 2, 2018

Yes, I'm suprised it doesn't work, but at the same time it has not seen unsubscribe testing in a very, very long while and a lot of changes has gone into it since it was known working. (There really should be tests around this, that is a failing here)

So the state of the RxJS versions -

  1. We are generally trying to point people towards api/rx, hence the documentation all around that
  2. api-observable is what the UI uses, but it still needs a bit of TLC to get it nicely aligned with where everything else is now, i.e.
  • it doesn't quite extend from api/rx like it should
  • really should only have the functions which are not available in the base, i.e. things like combines (and bestNumber is actually a good example of useful things in there since it takes another sub and then pulls info from it)

With all the above background, the fix should indeed go into the rpc-rx package (as indicated above) - so it pulls into both the above mentioned uses.

@jacogr jacogr added Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. @api-rx labels Nov 2, 2018
@jacogr
Copy link
Member

jacogr commented Nov 2, 2018

So reading through the linked issue, def. seems we introduced this very recently when swapping from BehaviourSubject to ReplaySubject. (Unless the former also has the issue).

I've followed your suggested approach and it seems to do the trick with a test, i.e. unsub seems to be called in this case - #373

@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants