Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Unsubscribe when take* completes#43

Open
veyh wants to merge 1 commit into
bjornbytes:master from
veyh:master
Open

Unsubscribe when take* completes #43
veyh wants to merge 1 commit into
bjornbytes:master from
veyh:master

Conversation

@veyh

@veyh veyh commented Jun 30, 2019

Copy link
Copy Markdown

In RxJS, take* unsubscribes after onCompleted/onError. In RxLua, it doesn't. I'm not sure if that's intentional or not, but in case it isn't, here's a fix.

4O4 commented Sep 30, 2019

Copy link
Copy Markdown
Contributor

I've been digging into this as I wasn't exactly optimistic that this PR is gonna solve all issues for me, and now I can say that it is indeed not enough to properly fix the issue, because the problem is actually a bit deeper.
When the observable emits complete or error it should automatically unsubscribe... at least it does so in rxjs implementation where everything works in a predictable way, but not in RxLua.

I've spent a lot of time comparing these two implementations and I've managed to fix the underlaying issue. I'm gonna send PR in a couple days when I polish things up as I was debugging multiple problems and messed the code up a lot :) But the end result will be proper automatic unsubscription in general (not only for take* operators) which will also work properly with cold observables which complete instantly in a synchronous way. This is important because Observables differ from Promises in that they don't have to be asynchronous and it can be leveraged in quite a lot of use cases.

Anyway thank you for your work on this @veyh because even though it's not a full fix, it helped me a bit to find a right direction while researching this issue :)

4O4 commented Oct 17, 2020

Copy link
Copy Markdown
Contributor

Hello, I don't want to send spam but since this is directly related I'll just leave a link to my comment in another issue: #22 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /