-
Notifications
You must be signed in to change notification settings - Fork 330
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
Implement [Symbol.observable] for ES7 interop compliance #633
Comments
I don't see any problem in adding some support into Bacon.js, including
These additions would in fact benefit the cases where you have different versions of Bacon running in the same environment too. There's a related issue... |
Awesome! Let me know if you need anything. |
I need somebody to actually do this :) Cannot assign a very high priority to this, as I don't see ES7 Observable as something that will be useful very soon. I'm willing to discuss this though, at least. A couple of issues came to my mind:
|
Yes. However, you can implement your adapter however you want. Perhaps you collect all of your errors and error once you complete? Or perhaps you just have it error and stop on the first one in that case? I'd say probably the later, since you're going from Bacon -> Observable in that case.
There is a polyfill implementation as part of that spec repository you can use. Otherwise, you could even do something very simple and assume the observer being passed in has the proper assertions in place (because it really should). .. there's actually two sides to this of course. You can consume/convert observables to Bacon observables as well. For that you'll need to implement a basic observer with some guarantees for things like making sure you don't next after an error (because observables shouldn't do that) class Observer {
constructor() {
this.isUnsubscribed = false;
}
next(value) {
if(!this.isUnsubscribed) {
// do what your going to do here
}
}
error(err) {
if(!this.isUnsubscribed) {
this.isUnsubscribed = true;
// handle the error
}
}
complete() {
if(!this.isUnsubscribed) {
this.isUnsubscribed = true;
// handle completion
}
}
}
Yes and no. Rx had a heavy influence no doubt, but the spec itself originally started as a "dual" of
The Observable itself, probably not, I agree... but the interop points are valuable I think. I know that Angular will be using RxJS internally, but that some of their bindings could (and should) use |
I don't suspect it will take long. I just don't know my way around your library. I'm happy to pair sometime, perhaps? In the end, I think having interop and some common standard between streaming types will only serve to boost interest in all libraries. It's not really a contest. It's more of a matter of getting people to stop writing crazy imperative code and start thinking reactively. |
I don't think @Blesh is suggesting the Bacon implement the complete ES7 spec. The only requirement for interop is that Bacon observables:
The returned object must have:
The returned object should have:
With that in place, any other "observable" implementation (including ES7) can then convert from Bacon observables: let es7Observable = Observable.from(baconObservable); And it also gives Bacon a hook for converting from other observable-ish things. I'll try to take a look at the codebase to see if I can come up some concrete suggestions. |
If I'm reading the code correctly, it looks like Bacon's You could write an adapter along these lines: class InteropAdapter {
constructor(observable) { this._observable = observable }
subscribe(sink) {
return this._observable.subscribe(event => {
if (event.isError()) sink.error(event.error);
else if (event.isEnd()) sink.complete();
else sink.next(event);
});
}
[Symbol.observable]() { return this }
}
Bacon.Observable.prototype[Symbol.observable] = function() {
return new InteropAdapter(this);
}; Perhaps you could also overload Bacon's Bacon.Observable.prototype[Symbol.observable] = function() {
return this;
}; But that would be more invasive. |
@zenparsing thanks, your InteropAdapter looks good! Except we should end on the first I think it might actually be worth considering to extend Bacon |
Agreed! I think this is a good idea and I don't see any major obstacles for implementing this right away. Anyone want to take the first shot? I'm keen to see how much polyfilling we need to make it work on current ES5-level environments. I guess |
So far Bacon.js has been developed so that it should work also in pretty old browsers without polyfills. |
You should be able to get by with a partial observable, which is just an object with a |
I might have some time to work on this. To avoid dependencies to any polyfills, we could have the |
@lautis That's right. Only adding the the method if |
Should the observable returned from |
My interpretation of the spec is that emitting events during the same tick is allowed. Only
The reference implementation catches errors, cleans up the stream and re-throws the exception. My assumption is that this behaviour should be matched in the Bacon subscription as well if the stream should adhere to ES7 observable semantics. But the ES7 observable implementations are already supposed to do this, so I did overlook this in the initial stab for getting this implemented. |
You mean there is no issue if we do |
I'm sure there are issues, but in a simple case the EsObservable behaves as described and Bacon stream subscription is cleaned up. This seems pretty reasonable, although I didn't notice any error or end events being emitted.
With the proposed implementation, exception handling would be left as the responsibility of Kefir. There could be problems if the Bacon stream catches exceptions and stops the stream subscription, but Kefir stream still waits for events. |
I've already reached out to the Most.js community so a lot of this will be repeated information.... Please excuse me if some of this has been copied and pasted. ;)
I know that a few months ago we discussed trying to come up with an interop spec, and I think we may have something with what has been developed as part of the ES7 Observable spec.
We've been working hard on the next version of RxJS, and we're getting closer. Part of that work was to try to adhere to the current ES7 Observable spec by @zenparsing. The part of particular interest there for this library is the
[Symbol.observable]()
method.Impl ideas?
Sadly, I'm afraid I don't know enough about Bacon's API to help you come up with a solid implementation. But I'm reaching out to you because I think Bacon has obvious merit and a strong community. What I really want is more support for streaming types throughout the land.
Consuming "lowercase-o" observables...
On your end, you could implement something inside of your
from
method to handle any type with a[Symbol.observable]
method on it. As a guineapig you can use@reactivex/rxjs
over npm, if you choose. Again it's pretty basic. You'd just call[Symbol.observable]()
then callsubscribe(observer)
on the returned object, where observer hasnext
,error
andcomplete
methods.Other things...
I've actively tried to make any monadic-bind-esque operator in RxJS Next support objects that implement
Symbol.observable
. This means, effectively that if you were to implement this, people could jump between Bacon and RxJS using flatMaps and switches, etc.The text was updated successfully, but these errors were encountered: