tc39 / proposal-observable

Observables for ECMAScript
https://tc39.github.io/proposal-observable/
3.06k stars 90 forks source link

Replace Subscriptions with Cancellation Tokens #112

Closed jhusain closed 7 years ago

jhusain commented 7 years ago

In Munich the committee pushed back when Observable was proposed for advancement to stage 2. Instead the committee suggested we examine whether it was possible to integrate Observable with the Cancellation Tokens presented by @domenic at the same meeting. Reasonable questions were raised about whether JavaScript really needs both Cancellation tokens and Subscriptions given both have similar responsibilities and APIs.

@domenic and I looked at this and came up with the following proposal:


class Observable<T> {
    constructor(subscribeDefn: Function)

    subscribe(observer: Observer, token: CancelToken): void

   forEach(nextFn: Function, token: CancelToken): Promise

    [Symbol.observable](): Observable

    static of(...items): Observable

    static from(ObservableLike: Object): Observable
}

interface Observer {

    next(value),

   // try/else equivalent
    else(error),

    complete(value)

    // receives object sent during unsubscription/cancellation
   // try/catch equivalent
    catch(cancel)
}

In this model, Cancellation tokens replace Subscriptions. Instead of an Observable creating a Subscription object when subscribe is called, the consumer creates a CT and passes it to the subscribe method alongside an Observer. Internally each Observable creates its own CT, registers all of its cleanup behavior to run on its CT’s cancellation, then links the internal CT with the one passed to the subscribe method. The Observable runs its cleanup behavior by canceling its own cancellation token whenever the subscription process finishes.

Given that the cancellation promise resolves asynchronously, this introduces a race condition in which an Observer could be notified after the token was canceled. To eliminate this race condition, the Observable always confirms the "CancelToken.requested" property is undefined before notifying.

Note that the Observer’s “error” function has been renamed "else" to match both the proposed language keyword and Promise method. similar to the proposed "else" method for Promise, the Observer’s "else" method will only receive Errors, not Cancellations. A new Observer method “catch” will receive any error that would be caught by a "catch" block, including cancellations passed to the cancellation token source’s cancel function. Either the "catch" or "else" methods can be present on the Observer, but not both. This mirrors the syntactic restriction against having a try/catch/else.

This is a significant change to the proposal and we need to do more work to confirm that all of the edge cases are met. I have tried to implement several combinators and haven't run into any issues yet. However I am also eager to gather feedback from library implementers to help identify any gaps in this proposal.

Here is my perspective on this proposal. Initially I was troubled by the ergonomics of replacing subscriptions with cancellation tokens, because using cancellation tokens requires an additional line of code when subscribing. However I became less concerned about this once I considered the change from the perspective of three personas:

  1. library author
  2. advanced user
  3. novice user

Library authors can wrap a method which returns a subscription and adapt it to the fn accepted by the Observable constructor. The wrapper method would simply attach the unsubscribe logic to the cancellation token promise. This is straightforward. However a larger question is around the composability of CT's. Often times it is useful to compose Subscriptions when writing more complex functions which compose together many Observables (ex. flat map). It's my intuition that Cancellation Token Sources can be composed in much the same way as Subscriptions (ex. CompositeCancellationTokenSource). It would be useful if a library author could falsify this.

In my experience, advanced users of Observable use methods like map, filter, and flatMap far more than subscribe. As a result I do not believe that requiring an extra line of code when subscribing is onerous. It's also important to note that advanced users unsubscribe even less frequently than they subscribe. If you look through the Netflix code base you will find that often subscriptions are not even captured, because takeUntil or switchLatest are used to declaratively trigger unsubscription instead. Given that the cancellation token is an optional argument, the new subscription API is effectively the same as the old one in these cases.

While advanced users will use combinators to coordinate concurrency, I believe novice users will use asynchronous functions. Given the early adoption and excitement around asynchronous functions, I expect that they will quickly become pervasive, and by extension so will cancellation tokens. Over time it will become increasingly likely that when Observable subscriptions occur inside of an asynchronous function, a cancellation token will already be in scope. As a result, the forEach API will only require the same boilerplate as any other asynchronous function: you have to pass the cancellation token as the last argument.

I want to call attention to the fact that using cancellation tokens improves ergonomics in one minor way: there is no longer any need for a "start" method. Both pulling/returning and using the "start" method to push the Subscription object was always an awkward API. It's cleaner to do it only one or the other, and cancellation tokens elegantly solve this problem. It is arguable that cancellation tokens are more appropriate than Subscriptions given the decision to allow Observable to synchronously notify.

Eager to hear your thoughts.

benjamingr commented 7 years ago

CancelTokens have a .race and .all methods for composing them so it shouldn't be a challenge over subscriptions.

I'm not sure why we'd want to pass the token into .subscribe though rather than .subscribe returning a {source, token} pair.

jhusain commented 7 years ago

@benjamingr both accepting a cancellation token and returning a cancellation token source would work. however accepting a cancellation token is consistent with the approach being advocated for other asynchronous functions. If we use the approach in the proposal, subscribing to an observable looks pretty much the same as executing another asynchronous function (accepts a cancellation token as last parameter). I favor consistency here given both approaches work.

benjamingr commented 7 years ago

Both options work, I implemented my own flavor of very specific observables and I export the CancellationTokenSource in an out parameter so I'm used to that.

It sounds just fine to pass the token inside - when I wrote my own code I needed a cancellation token source to implement all of the combinators - so I exported it as a return value (well, out parameter).

An observable implementation will need to maintain a source internally and then cancel that when the passed token is cancelled - but that's just implementation detail.

staltz commented 7 years ago

As a library author, I cannot yet think of a case where this new API would break. The biggest concern is asynchronous cancellation resolution making combinators like switchMap with synchronous outer emissions a problem, but as you said there's a workaround for that.

As long as libraries can wrap the CT-based API and provide a traditional Subscription-returning API and keep this abstraction from leaking, I'm 👍 for this change. Just want to get the proposal forward.

However:

Internally each Observable creates its own CT, registers all of its cleanup behavior to run on its CT’s cancellation, then links the internal CT with the one passed to the subscribe method. The Observable runs its cleanup behavior by canceling its own cancellation token whenever the subscription process finishes.

I might not have understood this fully, but could the Observable become stateful with the existence of this internal (and shared) CT? We need to preserve cold Observable behavior by default.

jhusain commented 7 years ago

@staltz Note that the external cancellation token is accepted on subscription. Furthermore the internal token is created on subscription. Therefore the Observable itself doesn't need to be stateful. All side-effects can still be deferred to subscription. Does that answer your question? Or have I misunderstood?

staltz commented 7 years ago

Furthermore the internal token is created on subscription.

Right, that's the piece of info I missed. In that case I can't see Observable being stateful.

RangerMauve commented 7 years ago

Any comments on how this would work with libraries that overload subscribe to take callbacks directly? I guess they would have the cancellation token as a fourth argument and the first three for callbacks.

jhusain commented 7 years ago

@RangerMauve I expect most consumers of Observable will use forEach, which is pretty ergonomic:

// inside of async function
try {
  const completion = await observable.forEach(x => ... );
catch(e) {
  // handle error
}
// outside async function
observable.forEach(x => ...).then(completeFn, errorFn);

Observable.subscribe is more useful for library authors than consumers. The primary difference between forEach and subscribe is that subscribe allows for synchronous delivery of error and completion notifications. This is necessary to write certain combinators, but generally doesn't provide much value to consumers. I expect subscribe will only be used by consumers in very rare cases. For those rare cases, I think ES2015 method syntax is sufficiently terse:

observable.subscribe({
  next(v) {   },
  else(e) {  },
  complete() {  }
}, cancelToken);
zenparsing commented 7 years ago

Thanks @jhusain for the detailed write-up.

First, let me point out that interop with async functions and cancel tokens can be achieved with the simple addition of a forEach method:

forEach(callback, cancelToken) : Promise

End users wanting a high level of interoperability can use forEach exclusively without loss of expressivity. Given that, there is no requirement to remove the subscription type (at least from the point of view of interoperability).

With the proposed changes, it is clear that we have not eliminated the subscription object at all; we create one, disguised as a cancel token, on every call to subscribe.

Async cleanup

I am very concerned about the ergonomics and footgun potential of async cleanup execution. Consider a simple example:

Element.prototype.on = function(type, options) {
  return new Observable((observer, token) => {
    function handler(event) {
      if (!token.requested) {
        observer.next(event);
      }
    }
    this.addEventListener(type, handler, options);
    token.promise.then(() => this.removeEventListener(type, handler, options));
};

Notice that since the cleanup function runs asynchronously, we have to include an explicit synchronous check against the "requested" property in order to ascertain whether the subscription is actually still active. Of course, observer.next will internally check the subscription status, so in some cases it might be fine to leave out this check. But generally speaking I believe the user will tend to assume that as soon as completion or cancellation occurs, the cleanup function will run. As a user of the Observable constructor, I find the async nature of cleanup troubling.

(Also, notice how bizarre it is to test whether a subscription has successfully completed by checking a cancel token's "requested" property!)

Symmetry between subscribe and forEach

If we are going to have a "tiered" API, where "subscribe" is provides the core functionality for advanced users and "forEach" provides basic registration capabilities for normal end-use, then making the two tiers more similar to each other actually increases the potential confusion about which API one should use in any given situation.

Cross-dependencies between proposals

This design creates a dependency on the cancellation token proposal. Cross-proposal dependencies significantly increase the risk for both proposals. If Observables with cancellation tokens gain traction, then the cancel token proposal may be prematurely constrained. And obviously, a dependency on cancel tokens means that any changes to cancel tokens will have a negative churn effect on Observables.

Lack of user-testing and feedback

After a long design process, we ended up confirming the basic Rx design, a design which has been around for decade (or whatever). At this point, we've received lots of user testing in the form of RxJS5. Changing a core part of the API at this point represents a significant increase of risk.

Basically...

The interoperability requirements can be completely satisfied by the addition of a forEach method which accepts a cancel token. I have yet to see any significant advantages to changing the signature of subscribe. On the other hand, there are significant downsides: the introduction of latency into cleanup execution, increased API confusion between subscribe and forEach, and risks arising from cross-proposal dependencies and lack of user feedback.

Perhaps I've missed something though. What are the arguments in support of these changes that offset those downsides?

jhusain commented 7 years ago

Thanks @zenparsing for highlighting these issues. Let me address them one-by-one:

Async cleanup

You provided the following code example as evidence that async cleanup is unergonomic/hazardous:

Element.prototype.on = function(type, options) {
  return new Observable((observer, token) => {
    function handler(event) {
      if (!token.requested) {
        observer.next(event);
      }
    }
    this.addEventListener(type, handler, options);
    token.promise.then(() => this.removeEventListener(type, handler, options));
};

There is never a need for an explicit requested check because the Observer makes the check internally and doesn't forward on a next() call after the cancel token has already been cancelled. Under the circumstances it's not clear to me how async cleanup negatively impacts ergonomics or is otherwise a footgun. Can you provide an example that demonstrates this?

Symmetry between subscribe and forEach

I'd agree making subscribe and forEach more similar may cause confusion. This confusion has to be balanced against potential developer confusion caused by adding both Subscription and CancelToken to the language. My opinion is that given we already have two subscribe methods, an API change to one which makes it more similar to the other only marginally increases confusion. On the other hand I think adding two new and similar entities to the JS standard library has the potential to more substantially increase confusion. I think @domenic has expressed this view, so I'll ask that he chime in if he agrees.

Cross-dependencies between proposals

No question that this design couples the CT and Observable proposals. Members of the committee have requested we explore coupling these proposals to ensure these various async types in the language work well together. One downside of this is that it slows progress, and changes to one can destabilize the other. However there is the counter argument that when language features are likely to be used together, the proposals should be coupled in the interest of ensuring they work well together. I think both arguments are valid, but I'm not sure how compelling the former will be to the committee based on the feedback we got in Munich.

Lack of user-testing and feedback

I agree that Observable has been around for a long time, but it's worth pointing out that there are differences in the API and semantics across various implementations. Bacon uses only hot observables, RxJS5 supports both hot and cold, RxJava pushes the Subscription to the Observer rather than returns it, and so on. There are many ways to skin this cat.

I agree that it will take work to feel confident that these changes do not preclude the writing of certain combinators or allow key invariants to be violated. I agree we'll need to do due diligence here. Given that Subscription semantics are a strict subset of Cancellation Token Source, I'm optimistic that we will find that this change will be banine.

Conclusion

I agree that interoperability requirements can be completely satisfied by the addition of a forEach method which accepts a cancel token. I think committee members who argue for unification make a compelling point re API simplicity.

There is a very high bar for new types in the JS standard library. If moving to CT's causes real problems for Observable, I think it's incumbent on advocates for Subscriptions to demonstrate that. One hand I would be happy if someone could demonstrate that Subscription were absolutely necessary, because it would allow us to decouple the proposals. At the same time I would also be disappointed, because it would mean that we needed to make the JS standard library more complex.

zenparsing commented 7 years ago

Under the circumstances it's not clear to me how async cleanup negatively impacts ergonomics or is otherwise a footgun. Can you provide an example that demonstrates this?

I'll try to come up with a real-ish example.

Members of the committee have requested we explore coupling these proposals to ensure these various async types in the language work well together.

The committee was very wrong about a great many things in Munich, not the least of which was demanding that a proposal declare itself as a dependency of another, less-developed proposal.

I agree that Observable has been around for a long time, but it's worth pointing out that there are differences in the API and semantics across various implementations.

Do any of the implementations have async cleanup semantics? I'm no expert, so I'd be interested to hear.

At the same time I would also be disappointed, because it would mean that we needed to make the JS standard library more complex.

The standard library should be as complex as it needs to be. By analogy, no one "needs" Set, since you can just use a Map instead. But there is a semantic niche that is filled. By the same token, Subscription fulfills a semantic niche. We shouldn't have to contort a cancel token in order to represent a subscription. We should just have Subscription.

I would love for other committee members to weigh in here. Anyone?

jhusain commented 7 years ago

Pinging @domenic for comment.

jhusain commented 7 years ago

Do any of the implementations have async cleanup semantics? I'm no expert, so I'd be interested to hear.

Not to my knowledge. I think it's fair to say we are treading new ground. The standard library should be as complex as it needs to be. By analogy, no one "needs" Set, since you can just use a Map instead. But there is a semantic niche that is filled. By the same token, Subscription fulfills a semantic niche. We shouldn't have to contort a cancel token in order to represent a subscription. We should just have Subscription.

I think this is a fair point, and a reasonable analogy. I would love for other committee members to weigh in here. Anyone?

I know @deantribble had some concerns about asynchronous cancellation. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

ljharb commented 7 years ago

^ ping @dtribble

dtribble commented 7 years ago

Agh! All this conversation while I was buried in a deadline. I'll look this week!

benlesh commented 7 years ago

Just to chime in, I find myself in full agreement with @zenparsing. While I think that including handling for Cancel Token is important, I feel like adding it to subscribe, which is the bread-and-butter way to use observables, is an ugly ergonomics hit. I think that RxJS can work around that by simply returning a Subscription if a cancel token isn't provided, but it would be done in deviation from the standard with a polymorphic subscribe method. While that's doable, I'm not sure it's a good thing.

domenic commented 7 years ago

Pinging @domenic for comment.

I'm a bit fan of the proposal in the OP, and think it's necessary for observables to proceed as a part of the language that meshes with the rest of it. To address some specific concerns:

  • Symmetry between subscribe and forEach
  • Cross-dependencies between proposals

This is intentional. The langauge needs to evolve as a whole. The champion model of independently-staged specs does not absolve us of the responsibility of working out cross-cutting concerns; that's an explicit part of the process, in fact, in stage 1. Each proposal, cancel tokens and observables, needs to evolve to work with the other.

It's claimed that having subscribe and forEach be similar is confusing, but this argument seems wrong to me. Reusing concepts makes things less confusing, not more. What's confusing is introducing two different ways of accomplishing cancelation; a new, observable-only subscription type, and a more general cross-ecosystem cancel token type. That's exactly the failure mode of independently-developed proposals that we were asked to avoid in Munich.

Lack of user-testing and feedback

It's true that there exist user-space libraries for observables. If we wanted to just use those, however, we'd simply do that: use them. No need to bake them into the language. The point of standardization here is to work on how observables can fit with the language and host environment ecosystems, and one of the crucial requirements expressed in Munich is having a single cancelation primitive.

Indeed, the committee (and web spec editors I've talked to) are also uncomfortable having two async completion options in the ecosystem (complete/error vs. promises), and expressed that it would have been nice to consolidate those as part of this harmonization and standardization process. But @jhusain gave a reasonably compelling example of something that could not be accomplished that way (would be good to get that written up in a doc in this repo, BTW), so reluctantly, we moved forward with both.

This still makes people uncomfortable, however, and if the observable community is not interested this kind of harmonization effort unless it is on something that already has years of user-testing and feedback, it certainly would defuse some tension to just let users continue using their user-space libraries, and not introduce a second async completion type into the language and platform. (I would certainly be disappointed, given the work I'm doing this week to pitch observables as an extension to the web's EventTarget. But it would be easier.)

benlesh commented 7 years ago

It's true that there exist user-space libraries for observables. If we wanted to just use those, however, we'd simply do that: use them. No need to bake them into the language. The point of standardization here is to work on how observables can fit with the language and host environment ecosystems, and one of the crucial requirements expressed in Munich is having a single cancelation primitive.

Isn't it a little telling that there are no user-space solutions (in any async library I'm aware of) that use Cancellation Tokens?

Promises were added to the language because they provided benefit. We could just be using user-space Promise libraries (many people still are), but they provide enough benefit that they were added to the language. And rightfully so.

Observables, in the current RxJS5/zen-observable form, are growing in popularity and use an are increasingly being adopted by communities like Angular 2, React and others. I'm always shocked and pleased to learn about the large efforts at Facebook or Slack that I knew nothing about that are using RxJS 5.

Where is the user-land pull for Cancellation Tokens in their current form?

To be clear... I really want a single cancellation type that works for everyone, but cancelling a promise and cancellation in an Observable seem to be at odds in a few places. Something is going to have to give. I'd argue that Observables are more valuable to the language than cancellation tokens. The tokens seem more like a late response to the need for cancelation that promises didn't provided OOTB.

benlesh commented 7 years ago

I just re-read my previous comment... it reads as a little aggressive. Sincere apologies for that. If I think of a way to rephrase it, I will. I feel like I'm asking hard questions to ask without sounding negative though. haha

<3

domenic commented 7 years ago

No, I think these are good hard questions. But I think they lead to the also-hard question: if user-land and standards-land have different goals, as I outlined and you emphasized, do we need to be working on this in standards-land?

benlesh commented 7 years ago

if user-land and standards-land have different goals

It seems weird to me that standards wouldn't align with user-land use cases though. I think the same forces that would drive standards to adopt Promises, Cancellation Tokens, Generators, et al, would drive them to adopt Observables as well.

benjamingr commented 7 years ago

Promises were added to the web platform so we could base web APIs on them. The same story is true for cancellation tokens so far. These are mostly platform demands.

Observables are also good for the platform - we should make sure they interop well with other mechanisms.

I agree with Ben and Kevin's objections though. Perhaps Rx could adopt cancellation tokens on a branch and we could try it out?

jhusain commented 7 years ago

I'm working on a lightweight implementation that I will check into the repository. I will feel pretty confident we can tackle the big combinators (flatMap, zip, etc). Aside from the issue of asynchronous clean up,

Providing we find that this change does not limit the expressive power of Observable, I am very much in favor of pursuing the standardization track. I've long thought that the web could benefit from Observables, that they are I think improvement on Event Target. Nothing about the proposed API change impacts any of that.

For the record, I think Observable is a valuable addition to the web platform and I would not want to see the standardization process hamstrung by an API change that will have limited impact on consumers. The standardization process is based on consensus. And any consensus-based process there will be give-and-take and concessions. I think .

zenparsing commented 7 years ago

one of the crucial requirements expressed in Munich is having a single cancelation primitive.

Subscriptions aren't a cancellation primitive though. They provide a cancellation capability, for sure, but they conceptually represent an "open pipe" between the producer and consumer. They are actually irreducible. We know this because all the implementations of observable on top of cancel tokens that we've come up with have necessitated a new cancel token per subscribe call in order to represent that pipe. Is a cancel token a good way to represent a pipe?

it would have been nice to consolidate [subscribe and forEach] as part of this harmonization and standardization process. But @jhusain gave a reasonably compelling example of something that could not be accomplished that way

Right, so the experiment to unify these concepts under forEach did not work. We have, instead, an async primitive (Observable/Observer/Subscription) that can't be reduced to Promises + cancel tokens. I find that to be an incredibly interesting discovery and we can embrace it.

Given that we are unable to reduce the Observer completion type to promises, how does the OP proposal (which has Observer) help diffuse the objection to having two async completion primitives?

jhusain commented 7 years ago

Issue accidentally closed due to butterfingers.

There are two separate issues here:

  1. Does this change limit the expressive power of Observable? This will really come down to the impact of asynchronous clean up, because Subscriptions are otherwise a semantic subset of Cancellation Token Sources.
  2. Does this change negatively impact ergonomics so much so that we would be better off leaving Observable in user land?

If this change limits the expressiveness of Observable, then it is a dealbreaker for me as far as standardization is concerned. However if we can't demonstrate that the proposed change limits expressiveness, then I think it would be a mistake to abandon the standardization process over this proposal. It's important to remember that we are discussing a change to an API we expect only a small fraction of developers to use.

I believe Observable is a valuable addition to the web platform and we should continue to work towards its standardization together. In addition to replacing EventTarget with something much more composable and ergonomic, the current proposal will allow developers to coordinate event flows inside of their async functions. The results are pretty compelling as you can see in these slides (https://docs.google.com/presentation/d/18KkpDm0Z-lGnUFxcK_ZJwSKCSalnBqjhGN8W--PyT88). Given how well it meshes with popular features like async/await, I'm more eager than ever to get Observable into every JavaScript developer's hands.

The standardization process is based on consensus. Many proposals which have made concessions during the standardization process are now delivering real value to web developers. That's where I want to see Observable in a year.

jhusain commented 7 years ago

@zenparsing looks like I was editing my post while you were submitting yours. Didn't mean to ignore it. Do you agree with my assessment that the core issue is whether CT's can replace Subscriptions without impacting the expressiveness of Observable? If they can, it's strictly accurate to state that Observable/Observer/Subscription can be reduced to Observable/Observer/CT. I think this question is orthogonal to the question of whether we need both forEach and subscribe. The Observer used by subscribe is necessary because we need synchronous notification of complete/error.

@domenic can we assume your opposition to the original Subscription–based proposal is the position of the Chrome team as a whole?

domenic commented 7 years ago

Yes.

benlesh commented 7 years ago

@jhusain with your implementation have you experimented with take or takeUntil? I'd like to see how CancellationTokens do with this over synchronous ranges, flatMaps, etc. It seems like we hit a lot of edge cases with take.

jhusain commented 7 years ago

That's a great suggestion. I can try take() and I'll make sure to try all the combinators against sync streams.

JH

On Sep 21, 2016, at 11:12 AM, Ben Lesh notifications@github.com wrote:

@jhusain with your implementation have you experimented with take or takeUntil? I'd like to see how CancellationTokens do with this over synchronous ranges, flatMaps, etc. It seems like we hit a lot of edge cases with take.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

TylorS commented 7 years ago

Just to clarify, these views are my own, and not related to any of the projects I'm involved in.

can we assume your opposition to the original Subscription–based proposal is the position of the Chrome team as a whole

This question worries me as user of the web. I have trouble swallowing that a single team of people can have sway over the standardization process of the language. V8 is great (the best IMO), but not necessarily the representation of the users of ECMAScript as a whole.


I've definitely been in favor getting Observable as a language primitive since the beginning. I'm a long-time contributor to most.js, one of the leading user-space implementations of Observables. I have been involved in writing xstream, another user-land implementation of Observables. I'm a long-time contributor to Cycle.js a framework entirely based around Observables. I use Observables every single day for work (RxJS), for almost all of my side-projects, and pretty much anywhere that I can, they're powerful. The picture that I hope I'm painting is that I'm a real-life user of Observables, who will eventually be affected by the standardization here in some form or another, as I've not been an active participant of this proposal.

I really do resonate with blesh's comment. I'm worried that some of the recent changes and especially the idea of using cancellation tokens is straying too far from the work that has already been done. There are user-land implementations that are already solving real-life issues for people. It feels as though we're pushing too hard to have a solution rather than having the right solution.

Cancellation Tokens on their own are totally unproven within any major framework or library I'm aware of. I feel a bit uncomfortable basing Observables on it, and it should be of concern when the first question from some people are "How can I abstract this away?". That should be a big sign that something here is either too complex, or has poor ergonomics.

Overall, I just don't want to see down the line see that we're stuck with an Observable primitive that is awkward in usage, hard to implement compatibility with existing libraries, or slow. In regards to performance, Most.js' Observable type and architecture looks virtually nothing like the proposal, entirely based on performance. Here the subscriberFn, and composition of combinators is almost always going to require scope capturing, no?

function map(f, observable) {
  return new Observable((observer, token) => {
    observable.subscribe({
      next: x => observer.next(f(x)),
      else: x => observer.else(x),
      complete: () => observer.complete()
    }, token)
  })
}

I hope this example is correct, it's based on my current understanding of the proposal. My concern here around performance is that we are always required to create a closure in order to provide user-land combinators. Closures are a great language feature, and I use them frequently, but they also introduce some overheads. Anyways, I digress from performance issues as that's not what this issue is about (I'll open one if that seems reasonable).


Besides all that I hope I have an answer to this question

Do any of the implementations have async cleanup semantics?

Most.js 1.x currently provides the ability to return a Promise to encapsulate logic to tear down a Observable. I was not present when the decision was made to add support for that, but my understanding is it was desired to be able to release resources asynchronously if there were ever the case that it was requirement. However, it's come to our attention recently that it's never been needed in real-life, so it's been a topic to remove such feature from most.js.

I don't believe there is any precendent of an asynchronous dispose in ES? Two common examples would be EventTarget.removeEventListener(..), and XMLHTTPRequest.abort() which are both synchronous.

I'd also like to ask 2 questions of my own

Either the "catch" or "else" methods can be present on the Observer, but not both

Why one would want or need both of these methods? In the original post, it's not immediately clear to me why one would need both, or why only 1 of them could be present on an Observer.

Library authors can wrap a method which returns a subscription and adapt it to the fn accepted by the Observable constructor.

Does anyone see any issues with this for a library that does not attempt to use an primitive that uses a constructor like the proposal offers?

jhusain commented 7 years ago

Comments below:

JH

On Sep 25, 2016, at 9:09 AM, Tylor Steinberger notifications@github.com wrote:

Just to clarify, these views are my own, and not related to any of the projects I'm involved in.

can we assume your opposition to the original Subscription–based proposal is the position of the Chrome team as a whole

This question worries me as user of the web. I have trouble swallowing that a single team of people can have sway over the standardization process of the language.

All members of the committee get to sway the standardization process by adding their input. Chrome is not special. They are exercising their right to object. The consensus model puts a high bar on new JS features. This can be frustrating, but it can also be positive. V8 is great (the best IMO), but not necessarily the representation of the users of ECMAScript as a whole.

I've definitely been in favor getting Observable as a language primitive since the beginning. I'm a long-time contributor to most.js, one of the leading user-space implementations of Observables. I have been involved in writing xstream, another user-land implementation of Observables. I'm a long-time contributor to Cycle.js a framework entirely based around Observables. I use Observables every single day for work (RxJS), for almost all of my side-projects, and pretty much anywhere that I can, they're powerful. The picture that I hope I'm painting is that I'm a real-life user of Observables, who will eventually be affected by the standardization here in some form or another, as I've not been an active participant of this proposal.

Welcome. We welcome your perspective. I really do resonate with blesh's comment. I'm worried that some of the recent changes and especially the idea of using cancellation tokens is straying too far from the work that has already been done. There are user-land implementations that are already solving real-life issues for people. It feels as though we're pushing too hard to have a solution rather than having the right solution.

If you feel this is the wrong solution it would be great to see some examples of why. In particular, it would be really helpful to see some evidence that CT's impact either expressiveness or ergonomics. This is an area where implementers like yourself can deliver a lot of value. Most of us are more familiar with Subscription-based implementations, and it's understandable that we would be concerned about change. I think it's important for us to maintain an open mind and consider whether Subscriptions are not just one possible implementation, and whether they are really core to the value that Observable provides.

Provided we don't compromise the expressiveness of Observable, I think that it's very valuable to continue pursuing the standardization process. Let me elaborate on some of the reasons why:

Cancellation Tokens on their own are totally unproven within any major framework or library I'm aware of. I feel a bit uncomfortable basing Observables on it, and it should be of concern when the first question from some people are "How can I abstract this away?". That should be a big sign that something here is either too complex, or has poor ergonomics.

Gut feelings are very valuable in the design process. When something feels wrong, I tend to trust my gut as an individual developer. When working on a committee in a collaborative process, we have to do more work to substantiate claims. TC-39 is influenced by use cases. I want to reiterate that if we feel like CT has poor ergonomics, or is excessively complex, we should be able to demonstrate that convincingly with real world use cases. Thus far, I have not been able to do this and would appreciate your help. Overall, I just don't want to see down the line see that we're stuck with an Observable primitive that is awkward in usage, hard to implement compatibility with existing libraries, or slow.

Would love to see some examples of awkward usages as I mentioned earlier. As far as compatibility with existing libraries, you'll have to explain what you mean. A Most.js Observable is not usable by Rx without adaptation. Likewise a CT-based Observable will need to be adapted in order to be used by either Most or Rx.

Adaptation is a reasonable solution to this problem, one already used by Promise libraries. Can you demonstrate that the proposed CT-based Observable cannot be adapted to the one used by Most or Rx?

In regards to performance, Most.js' Observable type and architecture looks virtually nothing like the proposal, entirely based on performance. Here the subscriberFn, and composition of combinators is almost always going to require scope capturing, no?

function map(f, observable) { return new Observable((observer, token) => { observable.subscribe({ next: x => observer.next(f(x)), else: x => observer.else(x), complete: () => observer.complete() }, token) }) } I hope this example is correct, it's based on my current understanding of the proposal. My concern here around performance is that we are always required to create a closure in order to provide user-land combinators. Closures are a great language feature, and I use them frequently, but they also introduce some overheads. Anyways, I digress from performance issues as that's not what this issue is about (I'll open one if that seems reasonable).

You are correct. Scope capturing will be necessary.

First let me share that I too am concerned about performance. Rx avoids closures because Netflix observed the same performance issues with closures that you have. It's important to reemphasize that standardizing Observables gives browser vendors the capability to apply optimizations that may not be possible in userland libraries.

History has demonstrated that when browser vendors are given enough time and incentive, they can make some very impressive optimizations. It's worth pointing out that the code you wrote above could be rewritten with bind. Partly motivated by optimizing Promises, V-8 has recently dramatically improved the performance of bind. Getting on the standardization train means that Observable can benefit from the same year-over-year performance improvements that the rest of the JavaScript language enjoys.

Besides all that I hope I have an answer to this question

Do any of the implementations have async cleanup semantics?

Most.js 1.x currently provides the ability to return a Promise to encapsulate logic to tear down a Observable. I was not present when the decision was made to add support for that, but my understanding is it was desired to be able to release resources asynchronously if there were ever the case that it was requirement. However, it's come to our attention recently that it's never been needed in real-life, so it's been a topic to remove such feature from most.js.

I don't believe there is any precendent of an asynchronous dispose in ES? Two common examples would be EventTarget.removeEventListener(..), and XMLHTTPRequest.abort() which are both synchronous.

As far as I know there is no precedent for asynchronous teardown logic in Observable. However I have not yet been able to demonstrate that synchronous teardown is necessary. Once again, it would be great to see an example that demonstrates this. I'd also like to ask 2 questions of my own

Either the "catch" or "else" methods can be present on the Observer, but not both

Why one would want or need both of these methods? In the original post, it's not immediately clear to me why one would need both, or why only 1 of them could be present on an Observer.

I'll refer you to the cancellation token proposal (https://github.com/tc39/proposal-cancelable-promises) which explains why we need both a "catch" and "else." These methods mirror those proposed for addition to Promise. The vast majority of users will simply use "else" for both Promises and Observables. Library authors can wrap a method which returns a subscription and adapt it to the fn accepted by the Observable constructor.

Does anyone see any issues with this for a library that does not attempt to use an primitive that uses a constructor like the proposal offers?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jhusain commented 7 years ago

Adding some links re: V8 bind optimizations.

https://codereview.chromium.org/2044113002 http://v8project.blogspot.com/2016/06/release-52.html

benlesh commented 7 years ago

I have not yet been able to demonstrate that synchronous teardown is necessary

Can't DOM events be listened to, triggered, and the listener removed totally synchronously?

jhusain commented 7 years ago

That is correct. However checking the "reason" field on the cancel token before notifying maintains the invariant that no notification can be sent to the Observer after cancellation. In other words, if asynchronous teardown is not observable, it is arguably not an issue. If we can demonstrate a case in which lingering side effects which remain immediately after unsubscription cause problems, that would be useful. Are there cases in which it would be too expensive to leave scarce resources around, even for another tick?

I discovered an issue with proposed implementation strategy yesterday while implementing a CancelToken polyfill. The "reason" field is synchronously available on CancelTokens. However CancelTokens use Promises to notify interested parties of Cancellation. This means that if you have a dependency graph of CancelTokens, cancellation will propagate across the graph asynchronously. In other words, if I compose together two CancelTokens using CancelToken.race, and one of the input tokens gets canceled, the composed token will not get its "reason" field set until the next tick.

Observable is forced to compose together Cancel Tokens to get Subscription–like behavior, but that doesn't work unless cancellation propagates synchronously. As soon as two Cancel Tokens are composed together, cancellation is notified asynchronously, and it's possible to deliver values to Observers before the cancellation has propagated through the CancelToken graph. This problem does not exist with Subscriptions, because "unsubscribe" propagates synchronously.

Asynchronous cancellation propagation is not only a problem for Observable. Asynchronous notification of cancellation could lead normal asynchronous functions to execute unnecessary and expensive side effects because cancellation does not become visible until the next tick. Given this issue, it's hard for me to see how asynchronous notification will work for CT's.

I will post an issue on the CancelToken repo about this and link to it here.

JH

On Sep 28, 2016, at 12:49 AM, Ben Lesh notifications@github.com wrote:

I have not yet been able to demonstrate that synchronous teardown is necessary

Can't DOM events be listened to, triggered, and the listener removed totally synchronously?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

zenparsing commented 7 years ago

Asynchronous cancellation propagation is not only a problem for Observable. Asynchronous notification of cancellation could lead normal asynchronous functions to execute unnecessary and expensive side effects because cancellation does not become visible until the next tick. Given this issue, it's hard for me to see how asynchronous notification will work for CT's.

Yep! And what would the API for synchronous notification look like? Well, fortunately, we've already got it right here in this proposal!

benjamingr commented 7 years ago

As soon as two Cancel Tokens are composed together, cancellation is notified asynchronously

Really? Why?

MarkBennett commented 7 years ago

@benjamingr the CancellationTokens use Promises to notify of cancellation. Because of the way Promises work, they always resolve on the next tick of the event loop. This means the cancellation notification will always happen on the next tick and won't be synchronous.

Update: realized I might not have answered your question. Not sure why composing would make them asynchronous. Using promises should do it.

benjamingr commented 7 years ago

Thanks for the effort - I'm mainly interested why it works with one token and not with several - partly because the CancelToken spec is still in development and the implication might drive the design of CancelToken.race etc.

jhusain commented 7 years ago

Unfortunately I don't think this problem can be resolved while Cancel Tokens use Promises. They only option would be for a "reason" getter on RaceCancelToken (a hypothetical subclass of Cancel Token produced by "race") to synchronously check all of its constituent CancelToken's "reason" fields. This would be unacceptable for performance reasons: "reason" must be O(1).

On Sep 28, 2016, at 7:45 AM, Benjamin Gruenbaum notifications@github.com wrote:

Thanks for the effort - I'm mainly interested why it works with one token and not with several - partly because the CancelToken spec is still in development and the implication might drive the design of CancelToken.race etc.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

benjamingr commented 7 years ago

@jhusain I don't understand, CancelTokens currently have a synchronous inspection API and an asynchronous one. We could just spec .race to set the .reason on the cancel token when we create the aggregated cancel token - at which point we have to iterate the tokens passed in anyway (like in Rx composite subsciptions).

To make a stand - in general I find the CancelToken centric design risky and subscriptions a proven approach (although I'm still hoping @headinthebox shows up and explains why he said it's a good idea in that one thread). I'd love to be convinced why aggregation with cancel tokens breaks this.

jhusain commented 7 years ago

Comments below.

On Sep 28, 2016, at 8:21 AM, Benjamin Gruenbaum notifications@github.com wrote:

@jhusain I don't understand, CancelTokens currently have a synchronous inspection API and an asynchronous one. We could just spec .race to set the .reason on the cancel token when we create the aggregated cancel token - at which point we have to iterate the tokens passed in anyway (like in Rx composite subsciptions).

This only works at the moment in which race is called. Imagine for a moment that none of the cancel tokens passed to the race method have been canceled when "race" is invoked. However within the same tick, anyone of the cancel tokens previously passed to race may be canceled. Unfortunately the composite CancelToken will not get its "reason" field set until the next tick, which means that expensive work may be started within the current tick. To make a stand - in general I find the CancelToken centric design risky and subscriptions a proven approach (although I'm still hoping @headinthebox shows up and explains why he said it's a good idea in that one thread). I'd love to be convinced why aggregation with cancel tokens breaks this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

domenic commented 7 years ago

I have a writeup of the problem and a solution sketch/PR for cancel tokens ready. I'll post it.

domenic commented 7 years ago

https://github.com/tc39/proposal-cancelable-promises/issues/57, spec patch incoming today.

benlesh commented 7 years ago

Is it really the design that we always need to leak the cancel function out of the CancelToken constructor if we want to allow it to be used like a subscription?

jhusain commented 7 years ago

(Some edits made due to early post)

@blesh No. You can just use the CancelToken.source() API which returns you both the cancel function and the token.

Here is an explanation of the "Revealing Constructor Pattern" (https://blog.domenic.me/the-revealing-constructor-pattern/). Seemed pretty strange to me at first, but seems to be well reasoned. Not really appropriately described as a "leak". No implementation details are exposed here.

On Oct 2, 2016, at 9:43 PM, Ben Lesh notifications@github.com wrote:

Is it really the design that we always need to leak the cancel function out of the CancelToken constructor if we want to allow it to be used like a subscription?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jhusain commented 7 years ago

I have completed both an implementation of Observable on cancel tokens and a document that explains that implementation and the decisions involved the step-by-step:

Can Observable be built on CancelTokens? https://github.com/jhusain/proposal-observable

Implementation https://github.com/jhusain/proposal-observable/blob/master/src/Observable.js

I have yet to update the spec text and the tests, but I am in the process of doing so. Might be a good time to start discussing the relative merits of the approaches.

RangerMauve commented 7 years ago

With this change, is cancellation now async? If somebody calls cancel(), does that mean that any events that happen before the next tick are still sent through? If there's a chain of observables, does that mean that cancellation can take several ticks before it propagates to the source? What are Tasks, are they something that was just made for this example, or is this going to be part of the spec?

jhusain commented 7 years ago

Many side-effects of cancellation could yes be async. Tasks are an internal implementation detail. The only public APIs are Observable and Cancel Token.

On Tue, Dec 6, 2016 at 12:26 PM, RangerMauve notifications@github.com wrote:

With this change, is cancellation now async? If somebody calls cancel(), does that mean that any events that happen before the next tick are still sent through? If there's a chain of observables, does that mean that cancellation can take several ticks before it propagates to the source? What are Tasks, are they something that was just made for this example, or is this going to be part of the spec?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-observable/issues/112#issuecomment-265262668, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcfryuo0FWiq5pmlbTQv2B3kOcLbBv3ks5rFcTrgaJpZM4J_6iU .

RangerMauve commented 7 years ago

The similarities to Promises while being synchronous is a little bit confusing. Though I suppose having similar method signatures is intentional. It would be useful to support a non-promise method to subscribe to cancellations for cases where people would prefer to react synchronously. For instance, if somebody wanted to cancel Observable.of(hugearrayhere) midway.

RangerMauve commented 7 years ago

Though I suppose that checking for the reason property every time you want to pass anything down to an observer works, too. Just seems a little gross to be polling like that rather than reacting right away.