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

Implement [Symbol.observable] for ES7 interop compliance #160

Closed
benlesh opened this issue Sep 22, 2015 · 20 comments
Closed

Implement [Symbol.observable] for ES7 interop compliance #160

benlesh opened this issue Sep 22, 2015 · 20 comments

Comments

@benlesh
Copy link

benlesh commented Sep 22, 2015

Hey @briancavalier! I know you and @jhusain and I talked a few months ago about interop between reactive streaming libraries like this and I'm here to offer a little help bridging that gap.

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.

Implementing [Symbol.observable]

Really all that is involved with that is to add a [Symbol.observable]() method to your streaming type, and have it return any object with a subscribe(observer) method, and a [Symbol.observable]() method.

The benefits here are:

  • interop between streaming libraries
  • makes your type usable with frameworks looking for that contract (Angular 2 is likely to have support for this, for example)
  • eventual interop with JavaScript core whenever ES7 Observable lands.

I think a basic implementation might look like:

[Symbol.observable]() {
  var self = this;
  return {
    subscribe(observer) {
      self.observe(observer.next.bind(observer))
        .then(observer.complete.bind(observer), observer.error.bind(observer));
    }
    [Symbol.observable]() { return this; }
  };
}

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 call subscribe(observer) on the returned object, where observer has next, error and complete 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 Most.js and RxJS using flatMaps and switches, etc. I'll be reaching out to the Bacon.js community with the same information.

@briancavalier
Copy link
Member

Hey @Blesh, thanks for the update. On a quick skim, this sounds good. I'll take a closer look later this week.

@benlesh
Copy link
Author

benlesh commented Sep 22, 2015

👍

@briancavalier
Copy link
Member

Hey @Blesh, reading @zenparsing's repo, it looks like subscribe should return a function, presumably to unsubscribe. However, looking at RxJS's Observable implementation, it appears to return a Subscription object with an unsubscribe method.

Can you offer an insight into what's going on there? Am I missing something? Thanks for any info as I start to work on this :)

@briancavalier
Copy link
Member

I see there is quite a bit of discussion in tc39/proposal-observable#48. It seems like returning a function is still more "official", so I'll go with that for now. It's not hard to switch if there is a change.

@briancavalier
Copy link
Member

@Blesh Is there any information on the synchronous vs. asynchronous delivery of the first event after subscribing? IOW, is an observable allowed to call subscriber.next(x) before subscribe(subscriber) returns?

@briancavalier
Copy link
Member

Initial strawman: https://github.com/cujojs/most/tree/add-es7-observable-160

Would love to get some feedback from @davidchase as well.

@benlesh
Copy link
Author

benlesh commented Sep 26, 2015

@briancavalier, yes subscription is synchronous.

I doubt that returning a function from subscribe will hold water. But RxJS can handle either. I don't plan I returning anything but an object with an unsubscribe function right now. I can change that for interop, but I think it's wrong for the type.

Cc @zenparsing

@davidchase
Copy link
Collaborator

@briancavalier 👍 seems straightforward to me...

I am curious to see what they do with the subscribe either returning a singular function or an object with that a dispose method on it. Don't really have a "horse in that race" but nonetheless curious to see the direction it goes i can see both sides.

Personally when using other rp libraries in the past that provided a dispose method from subscribe i didnt find myself using it... since it was possible to do some like a takeUntil and it seemed like most.js went into that direction of being declarative. Even now it seems that libraries like Bacon are also removing their dispose method from subscribe in favor of chaining some methods which currently cannot be done.

@briancavalier
Copy link
Member

yes subscription is synchronous.

@Blesh @zenparsing Hrm, that seems unfortunate. I'd be interested to hear the reasoning behind the decision.

i didnt find myself using it... since it was possible to do some like a takeUntil
... being declarative

@davidchase This is a great point, and personally, I agree. I think imperative unsubscribe is a foot gun for app devs. I'd much rather see RP implementors promote declarative APIs to devs. That said, imperative unsub is probably a necessity for implementation interop. So, I can see it being a part of an interop spec. It exists internally in most.js because it has to, but this is really the first time it's exposed.

@davidchase
Copy link
Collaborator

imperative unsub is probably a necessity for implementation interop

definitely understand that as well the need for it in most.js for that reason.

I'd much rather see RP implementors promote declarative APIs to devs

I do as well, especially when the allow for a more functional way of programming those with those APIs, its like here's map, filter, reduce, etc but when you want to add more events per say just do bus.push... meh not a fan 😝 not saying that i dont understand a need for imperative pushes or end calls.

@benlesh
Copy link
Author

benlesh commented Sep 27, 2015

The reason for synchronous subscription was so it could model DOM events, which can be completely synchronous.

@briancavalier
Copy link
Member

The reason for synchronous subscription was so it could model DOM events, which can be completely synchronous.

Hmm, maybe we're talking about different things. I'll try to differentiate based on what I think you're saying (and I could be misunderstanding!):

  1. Adding a new subscriber to a list (or whatever) so that events can be dispatched to it. It's fine for this to happen synchronously, and doing so would indeed match DOM event behavior. This is important because it helps keep subscribers from "missing" events.
  2. Dispatching an event to a new subscriber before the call to subscribe which registered it. Allowing this would be bad, imho, and would be the equivalent of a fulfilled promise's then calling a callback synchronously.

AFAIK, like promises, DOM events do not allow 2. No DOM event will ever invoke a callback before the associated call to addEventListener returns. IOW, in this case:

node.addEventListener('<any event>', handleEvent, false);

handleEvent is guaranteed never to be called before node.addEventListener returns. That's even true, I believe, if addEventListener is called during event dispatch (i.e. inside an event handler itself).

So, it seems to me that there's precedent in the DOM and promises for not allowing subscriber.next to be called before subscribe() returns.

Is there a discussion thread you can point me to on this? I'm happy to do background reading to try to understand.

@benlesh
Copy link
Author

benlesh commented Sep 27, 2015

AFAIK, like promises, DOM events do not allow 2. No DOM event will ever invoke a callback before the associated call to addEventListener returns.

Correct, but you can trigger synchronously immediately after addEventListener returns.

So, it seems to me that there's precedent in the DOM and promises for not allowing subscriber.next to be called before subscribe() returns.

I don't see how there's a precedent that relates to Observable. They're different types with different purposes.

Here's all of it in a nutshell:

  1. You can't have subscribe be asynchronous, because you can synchronously trigger events on DOM elements immediately after you addEventListener.
  2. Since subscribe has to be synchronous, you MUST allow next to be called before it returns, otherwise you have an unbounded buffer of events until subscribe completes.

On 1 above:

var subscription = Observable.fromEvent(button, 'foo').subscribe(::console.log(x));
button.dispatchEvent(new Event('foo')); // completely synchronous to the subscription above.

On 2 above... imagine a range to 1,000,000 or something like that. If subscribe must be synchronous, but next can't do anything until after it returns, that means you'll have to queue up all 1,000,000 in a buffer until subscribe returns. Perhaps worse than that, you'll likely have to put them on some sort of microtask scheduler, at every level of your observable chain, and now all of your call stacks look like disconnected junk.

@briancavalier
Copy link
Member

Correct, but you can trigger synchronously immediately after addEventListener returns.

Yep. That's why I was using the terminology "subscribe.next must not be called before subscribe() returns". We're saying the same thing re: DOM events, I believe.

I don't see how there's a precedent that relates to Observable

It relates because both have the potential of unleashing zalgo.

Since subscribe has to be synchronous, you MUST allow next to be called before it returns, otherwise you have an unbounded buffer of events until subscribe completes.

Ah, thanks, I understand the concern here now. This is an interesting tradeoff vs zalgo. I can see for cold observables, this could be important to prevent buffering. For hot observables, I'm having trouble coming up with a realistic case where this would actually happen. Do you have an example handy? I'll think about it more.

@briancavalier
Copy link
Member

I just added fromObservable to the branch. It also turned out to be very simple.

@Blesh I updated the branch to use a subscription object with an unsubscribe method, rather than a closure. Either way seems ok to me. The closure is certainly nice for callers, but if there's concern about creating large numbers of closures, perhaps in scenarios with large numbers of concurrent active observables, an object will use less mem.

I'm still concerned about allowing observables to deliver events before subscribe returns, but certainly open to trying to work out the apparent buffering vs. zalgo tradeoff. Sorry I was gruff about it earlier. It was a serious point of contention with Promises/A+ because it can lead to major foot-shooting. That's why I keep expressing my concern.

@zenparsing
Copy link

@briancavalier Regarding the return type, feel free to join in the discussion at tc39/proposal-observable#48

As far as zalgo goes, let me explain how I came to my current position.

First, I think zalgo avoidance is important, and that's why I have Observable.of and Observable.from sending data in a future turn. However, subscription cannot in general be deferred to a future turn because that would break any abstraction that models events which can occur before the "next tick". DOM events are one such abstraction, but the same consideration applies to any use of Rx's Subject; the subject can always send data before the current stack is unwound (and for some combinators it does just that).

I've always been against the idea of having two subscription methods which differ only in sync-vs-async subscription behavior, since subtle differences like that will most likely only result in confusion.

I briefly toyed with the idea of "next", etc, throwing an error if invoked before subscribe had returned, but that felt overly "nannyish".

So my current position is basically that creators of Observable should, as a best practice, follow the lead of Observable.from and not send data before the subscriber function has completed. It's not an absolutely ideal situation, but given the design constraints it seems to me like the best one.

Thoughts?

@jasonkuhrt
Copy link

Hey everyone, what does Zalgo mean...?

@zenparsing
Copy link

@jasonkuhrt In this context, it means situations where sometimes a callback will execute synchronously (before the function call returns) and sometimes asynchronously. This can lead to difficult-to-reproduce errors. Promises avoid zalgo by always invoking their success and error handlers in a future turn of the event loop.

@briancavalier
Copy link
Member

Thanks @zenparsing. Sorry I haven't had time to respond yet :/ I'll try to soon.

@jasonkuhrt what @zenparsing said :)

benlesh added a commit to benlesh/most that referenced this issue Apr 26, 2016
This will enable Most to interop with Kefir, RxJS and libraries like Redux

Related cujojs#160
@briancavalier
Copy link
Member

ES7 Observable interop is nearly done in #254.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants