-
Notifications
You must be signed in to change notification settings - Fork 90
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
Observable should be async #206
Comments
The synchronous option is a feature. |
Observables currently try to allow for both things, being sync and async. You say this is a feature. As I showed in my post, for sync multi-valued operations there is already Iterator, which works just as well because for sync operations there is no difference between push and pull. Why didn't Promises also allow for sync usage? Because for single-value operations this is already provided by Functions. For the same reason Observables should be strictly async. There doesn't need to be a feature implemented twice. But even if, even if Observables would allow for sync usage, it should not be the default behavior. A fully redundant behavior should not be the default. |
There is a difference between push and pull for sync, it’s just a smaller one. |
If Observables could not be synchronous, they could not model EventTarget, which can be registered and triggered synchronously. |
Imagine situation, when you don't know if data will be delivered synchronously or asynchronously (in example, in UI framework, that I'm creating (Atom-iQ), I cannot know it, cause everything to process is coming from the framework consumer) - RxJS with sync observables provides a way to handle it, in exactly the same way. To be honest, for my framework it's the "must have" feature, far from calling it "redundant" |
If I understand correctly, Observables don't use the microtask queue unlike Promises. This makes Observables by default not async. This is weird. Even in the motivation it says
In the implementation RxJS Observables for example, the following logs the values
21
and42
synchronously.I have to admit I didn't read the proposal, so this may be just in the RxJS implementation. But there hasn't been an issue filed neither there nor here, so it seems like this is intended behavior. I believe this should be subject to a serious discussion.
I don't see the point in synchronous push-based operations. One could just as well use a pull-based operation with the already existing
Iterator
. Actually it would look almost identical. Here's the above example using an iterator (created with a generator for brevity).The only real difference is that the consumer would need to loop through the values himself instead of having his "subscription" having looped automatically by the producer.
In conclusion, synchronous push-based operations make little sense, because they can just as well be implemented as pull-based using the existing
Iterator
. Push operations only really start to make sense with asynchronous operations, like with promises.With observables being properly async, the above example would give.
Currently this needs to be manually ensured by wrapping the
subscriber.next(..)
calls in aPromise.resolve().then(() => {..})
or asetTimeout(() => {..}, 0)
call (depending if you want to use the microtask queue or the normal queue).Observables would make much more sense as generalised promises which can resolve multiple times, meaning they can push multiple values over time asynchronously. Everything else in the famous
sync / async / single value / multiple values
diagram is already covered well by the language.I strongly believe it would be a lost opportunity to not use the microtask queue for Observables. We don't need to have another addition to the language, which an external library could fully implement, which is the case for Observables in the current synchronous fashion. Using the microtask queue would however be a real reason to include Observables into the language, because this is what a third-party library could not implement (without the
Promise.resolve(..)
hack).The text was updated successfully, but these errors were encountered: