-
Notifications
You must be signed in to change notification settings - Fork 76
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
Observables continue being evaluated after stream completes #22
Comments
Any news on this? |
This is due to the |
Hello, for anyone who might be interested in this topic: I've decided to maintain my own fork (https://github.com/4O4/lua-reactivex) which includes a refactoring in order to fix all the underlying issues and introduce a proper automatic unsubscription mechanism inspired by RxJS internals. This was a major redesign of some core parts of the code which would probably never be merged in this repo, that's why I decided to make a friendly fork under totally different name to indicate that these two projects are totally different, yet mostly still compatible (unless you rely on bugs like the one described in this issue). You can check out what was the scope of changes in the diff in this PR: 4O4#2 I'm planning to include in this fork some other patches which were sent to this repository but never merged. Also in the meantime I discovered that the tests don't cover the code very well and are leaking, so I started to improve them and also reconfigure coverage reports to better visualize the parts of code which were assumed to be tested, but actually are not (this effort is partially finished and can be found in this PR: 4O4#4). So I guess my focus will be on incrementally improving the tests too, not only a standard feature / bugfix development. Contributions are welcome, if anyone wants to help me with development and bringing some important patches to this fork I would really appreaciate it. Currently I'm maintaining it for my own needs, but I'm sure there are lot of people who could benefit from an updated and actively maintained version of this library. Maybe we will even be able to merge back with this original repository some day if bjornbytes will be interested, who knows. But currently it is as it is, this repo is getting a bit stale and I can't wait for critical patches forever so that's why I'm choosing this alternative path and am willing to move the development forward myself under the separate "brand" :) |
Here's the output of the code from the first post in this issue, while using the current version of my
As you can see it is exactly the same output as expected and exactly the same as RxJS is giving. All of that is achieved without any hacky |
When a stream (chain of observables) completes, subscribers will stop receiving events (as expected), but any observables higher up the chain that would have been evaluated if the stream was still active, are still evaluated after the stream completes.
See the following Lua snippet:
This creates three streams sharing the same subject. The positiveCubes stream completes after two values are pushed for which x^3 is positive. The positiveEvenCubesTimesTwo stream completes after one value is pushed for which x^3 is even. The negativeCubes stream never completes.
Each observable produces a side effect (the print statement). Execting the script produces the following output:
As can be seen from the output, the observables in the positiveCubes stream are still being called after the stream completes. Interestingly, the observables in the positiveEvenCubesTimesTwo streams are also called after the stream completes, but only once, after that they are not evaluated anymore (?).
Recreating the same example using rxJS:
The output looks as expected:
Observables not being disposed after the stream completes becomes a problem if the number of streams is large, or if temporary/transient streams are created. Over time, the number of observables that are being evaluated unnecessarily because they are part of a completed stream will start to add up.
The text was updated successfully, but these errors were encountered: