whatwg / dom

DOM Standard
https://dom.spec.whatwg.org/
Other
1.58k stars 295 forks source link

Improving ergonomics of events with Observable #544

Open benlesh opened 6 years ago

benlesh commented 6 years ago

Observable has been at stage 1 in the TC-39 for over a year now. Under the circumstances we are considering standardizing Observable in the WHATWG. We believe that standardizing Observable in the WHATWG may have the following advantages:

The goal of this thread is to gauge implementer interest in Observable. Observable can offer the following benefits to web developers:

  1. First-class objects representing composable repeated events, similar to how promises represent one-time events
  2. Ergonomic unsubscription that plays well with AbortSignal/AbortController
  3. Good integration with promises and async/await

Integrating Observable into the DOM

We propose that the "on" method on EventTarget should return an Observable.

partial interface EventTarget {
  Observable on(DOMString type, optional AddEventListenerOptions options);
};

[Constructor(/* details elided */)]
interface Observable {
  AbortController subscribe(Function next, optional Function complete, optional Function error);
  AbortController subscribe(Observer observer); // TODO this overload is not quite valid
  Promise<void> forEach(Function callback, optional AbortSignal signal);

  Observable takeUntil(Observable stopNotifier);
  Promise<any> first(optional AbortSignal signal);

  Observable filter(Function callback);
  Observable map(Function callback);
  // rest of Array methods
  // - Observable-returning: filter, map, slice?
  // - Promise-returning: every, find, findIndex?, includes, indexOf?, some, reduce
};

dictionary Observer { Function next; Function error; Function complete; };

The on method becomes a "better addEventListener", in that it returns an Observable, which has a few benefits:

// filtering and mapping:
element.on("click").
    filter(e => e.target.matches(".foo")).
    map(e => ({x: e.clientX, y: e.clientY })).
    subscribe(handleClickAtPoint);

// conversion to promises for one-time events
document.on("DOMContentLoaded").first().then(e => …);

// ergonomic unsubscription via AbortControllers
const controller = element.on("input").subscribe(e => …);
controller.abort();

// or automatic/declarative unsubscription via the takeUntil method:
element.on("mousemove").
    takeUntil(document.on("mouseup")).
    subscribe(etc => …);

// since reduce and other terminators return promises, they also play
// well with async functions:
await element.on("mousemove").
    takeUntil(element.on("mouseup")).
    reduce((e, soFar) => …);

We were hoping to get a sense from the whatwg/dom community: what do you think of this? We have interest from Chrome; are other browsers interested?

If there's interest, we're happy to work on fleshing this out into a fuller proposal. What would be the next steps for that?

annevk commented 6 years ago

Thanks @benlesh for starting this. There've been mumblings about a better event API for years and with Chrome's interest this might well be a good direction to go in.

Apart from browser implementers, I'd also be interested to hear from @jasnell @TimothyGu to get some perspective from Node.js.

Another thing that would be interesting to know is what the various frameworks and libraries do here and whether this would make their job easier.

From what I remember discussing this before one problem with this API is that it does not work with preventDefault(). So if you frequently need to override an action (e.g., link clicks), you can't use this API.

cc @smaug---- @cdumez @travisleithead @ajklein

davidkpiano commented 6 years ago

From what I remember discussing this before one problem with this API is that it does not work with preventDefault(). So if you frequently need to override an action (e.g., link clicks), you can't use this API.

Couldn't this be covered with EventListenerOptions options?

element.on('click', { preventDefault: true })
  .filter(/* ... */)
  // etc.
gsans commented 6 years ago

Cool! I propose to call them DOMservables

matthewp commented 6 years ago

@annevk Can you explain the preventDefault() problem in more detail? From @benlesh's examples, I would think you could call it in any of the filter() or map() callbacks. Is there a reason why you could not?

domenic commented 6 years ago

I think @annevk is remembering an async-ified version, perhaps based on async iterators. But observables are much closer to (really, isomorphic to) the EventTarget model already, so don't have this problem. In particular, they can call their next callback in the same turn as the event was triggered, so e.preventDefault() will work fine.

It really is just A Better addEventListener (TM). ^_^

annevk commented 6 years ago

(It seems my concern indeed only applies to promise returning methods, such as first().)

appsforartists commented 6 years ago

@benlesh Can you speak more to why the subscribe(observer) signature isn't valid? That's how all the observable implementations I've seen currently work, but they aren't written in C or Rust.

domenic commented 6 years ago

In web APIs, per the rules of Web IDL, it's not possible to distinguish between a function and a dictionary. (Since functions can have properties too.) So it's just disallowed in all web specs currently. Figuring out how or whether to allow both o.subscribe(fn) and o.subscribe({ next }) is the TODO.

To be clear, the tricky case is

function fn() { console.log("1"); }
fn.next = () => { console.log("2") };

o.subscribe(fn);

Which does this call? Sure, we could make a decision one way or another, but so far in web APIs the decision has been to just disallow this case from ever occurring by not allowing specs that have such overloads, So whatever we do here will need to be a bit novel.

This is all a relatively minor point though, IMO. Perhaps we should move it to https://github.com/heycam/webidl/issues.

benlesh commented 6 years ago

Another thing that would be interesting to know is what the various frameworks and libraries do here and whether this would make their job easier.

I can't speak for frameworks, directly. Perhaps @IgorMinar or @mhevery can jump in for that, but for RxJS's part, whenever anyone goes to build an app using only RxJS and the DOM, one of the most common things they need from RxJS is fromEvent, which this would completely replace. I would also definitely love to see an RxJS that was simply a collection of operators built on top of a native Observable we didn't always have to ship.

appsforartists commented 6 years ago

Thanks for the clarification.

It's interesting to me that TC39 and WHATWG both have the ability to add JS APIs, but with different constraints. The TC39 proposal decides what to do based on if the first param is callable. If the TC39 proposal was stage 4, the browsers would be implementing that behavior, right? (Or maybe the TC39 proposal was supposed to be in WebIDL too and violated this. I hadn't heard about that, but I'm not a TC39 member either).

keithamus commented 6 years ago

FWIW in practice the DOM already makes the distinction of function-vs-object in the case of addEventListener:

const handle = e => console.log('main!', e)
handle.handleEvent = e => console.log('property!', e)
document.body.addEventListener('click', handle)

// Logs with `main!`

(Not suggesting WebIDL can't make the distinction, just pointing out there is a precedent here)

jhusain commented 6 years ago

@annevk concerns about the ability to call preventDefault() when using Promise returning methods are valid. Mutation use cases could be addressed with a do method which allows side effects to be interleaved.

button.on(“click”).do(e => e.preventDefault()).first()

This method is included in most userland Observable implementations.

benlesh commented 6 years ago

@jhusain it could also be handled with map, although it would drive some purists crazy:

button.on('click').map(e => (e.preventDefault(), e)).first()
appsforartists commented 6 years ago

The biggest thing I see missing from this proposal is an official way to use userland operators. @benlesh, do you think pipe is ready to be included?

To the question about frameworks/libraries, as the author of a userland implementation, there are three core types that I'm interested in:

If Observable is standardized without the Subjects, userland libraries will still ship them. Still, any standardization is good for the whole ecosystem - it means operator authors know what interface they must implement to be interoperable with the ecosystem.

Furthermore, if Observable is standardized, I expect it will become common knowledge among web authors (just like promises, generators, etc.) That will make it easier for frameworks to depend on it without having to worry about intimidating their users with a steep learning curve - understanding observables becomes part of the job.

jhusain commented 6 years ago

@benlesh Using map would be less ergonomic for this use case, because developers would be obligated to also return the function argument.

button.on(“click”).map(e => {
  e.preventDefaut():
  return e;
}).first()

The do method would allow preventDefault() to be included in a function expression, and would consequently be more terse. Given how common this use case may be, the do method may be justified.

johanneslumpe commented 6 years ago

@appsforartists Nitpick, but a Subject should not retain its current value. Rather than having a Memoryless subject, I think it should be the other way around: having a Subject that can retain its latest value, like BehaviorSubject in rxjs. Adding functionality on top of the common base, instead of removing it.

appsforartists commented 6 years ago

Not to bikeshed, but the name of that method seems to keep flipping between do and tap. I don't know the reasoning for the changes, but I suspect tap is easier for users to disambiguate from do blocks and do expressions.

appsforartists commented 6 years ago

@johanneslumpe Agree that the information architecture is a bit wonky. I think I originally learned Subject has a value, so I think of the other as MemorylessSubject, but that doesn't mean we should standardize that way.

emilio-martinez commented 6 years ago

@jhusain If I may, I believe you're nitpicking a bit too much on that point.

First, the benefits and ergonomics of having the listener removed due to the first() operator are much greater than the possible ergonomics lost on preventDefault(). In fact, it's the ergonomics for adding+removing event listeners which would make this API so rich.

Second, calling preventDefault() would probably not happen quite as you mention in your example. I believe it would be more like

button.on('click').first().map(e =>
  e.preventDefault();
  ... do more stuff with event
})
benlesh commented 6 years ago

@jhusain I completely agree. I was just demonstrating that if there was a concern over method proliferation, it's possible with existing proposed methods. (And I know about the return requirement, notice the sly use of the comma in my example)

benlesh commented 6 years ago

Not to bikeshed, but the name of that method seems to keep flipping between do and tap.

@appsforartists, that's an RxJS implementation thing. Unrelated to this issue.

TimothyGu commented 6 years ago

For Node.js to accept (and implement) this, the on() method name must be changed, given that the Node.js EventEmitter class has on() as an alias for addEventListener(). People who program both for Node.js and for the web will just get more confused between the different behaviors on different platforms.

benlesh commented 6 years ago

@appsforartists this proposal is really meant to meet needs around events in the DOM, and happens to ship with a nice, powerful primitive. We should keep it to that and not over-complicate it.

domenic commented 6 years ago

@TimothyGu Node could switch on the number of parameters provided to the on() method. That is, assuming Node wants to do direct integration into their EventEmitter like the web platform does.

hrajchert commented 6 years ago

Congrats all for bringing this proposal 🎉. I just want to address a point that I think the current implementation of RxJs is missing.

Currently, both Promises and Observables can only be typed on the value, not on the error. And I know talking about types (TypeScript or flow) is a strange thing to do in an Observable proposal, but the underlying reason is a subtle one, and the behaviour of how Observables handle different error situations is paramount.

The problem, as stated here, arrise when handling functions that throws errors, for example

Observable
  .of(1)
  .map(_ => {throw 'ups'})
  .fork(
       x => x,
       err => err // The error will be the string 'ups', but the type checker can only type it as any
  );

We can't avoid to handle these types of error as either your functions or functions that you call may throw, but on the other hand this type of error handling goes against of how we should do it functionally. It would be better to use a chainable method like .switchMap and Observable.throw or in the case of promises .then and Promise.reject.

For that reason, both Promises and Observables can only be typed on error as any and sadly it is the correct type. Luckly I think there are at least two possible solutions, one that is relevant to the Observable proposal.

One solution would be to try catch all methods that may throw and wrap the possible error into a class that extends from Error, for example

class UncaughtError extends Error {
}

which would make the following example possible

Observable
  .of(1)
  .map(_ => {throw 'ups'})
  .switchMap(_ => Observable.throw(new DatabaseError())
  .fork(
       x => x,
       err => err // The error type would be UncaughtError | DatabaseError
  );

Note that UncaughtError is always a posibility both if you have a function that throws or not but DatabaseError could be infered from the types of Observable.throw and switchMap.

Very recently I created a Promise like library called Task (WIP) that takes this into account and allow us to type both success and error cases. So far the results where more than satisfying.

The other solution I can think of would be if TypeScript or flow implements typed exceptions, but I don't think is the path their plans.

I hope you take this situations under consideration and thanks for the great work

Cheers!

TimothyGu commented 6 years ago

Node could switch on the number of parameters provided to the on() method.

The IDL proposed in the OP allows a second options-style parameter.

But I'd also like to point out jQuery's on() as evidence for that the name simply has too much historical burden to it.

keithamus commented 6 years ago

At the risk starting the bikeshedding wars around the API; would .observe() be a reasonable name for this?

benlesh commented 6 years ago

At the risk starting the bikeshedding wars around the API; would .observe() be a reasonable name for this?

@keithamus Certainly! But there's already a huge amount of industry momentum around subscribe as the primary API here. There's also some interesting nuance around what this method does. In all cases, it does observe, but in many cases it not only observes, but it also sets up the production of values. In this particular use case (EventTarget), it's only really observing, so the name "observe" makes total sense, but we would be sacrificing a name that makes a little more sense for other use cases of the Observable type as a primitive.

TimothyGu commented 6 years ago

To be clear, anything other than on would be fine with me. I’ll leave y’all to determine what’s best other than that :)

domenic commented 6 years ago

I don't think we should change the name for Node.js concerns; Node.js already has differently-named APIs (e.g. addListener/removeListener vs. addEventListener/removeEventListener) and the objects passed would be different (Node.js arbitrary, DOM Event instances).

In the end, both jQuery objects and Node.js EventEmitters are different APIs, so can have different entry points without there being a problem. What's more interesting is whether they would make use of the Observable primitive itself, in one way or another.

YurySolovyov commented 6 years ago

Btw, any reasoning on why this should be a DOM API and not core language feature?

benlesh commented 6 years ago

@TimothyGu Also worth considering: Node and JQuery chose on because it happens to be a really good name for an event-related method. It's almost an argument for why it's the right name. It would be a shame if we didn't consider using it.

benlesh commented 6 years ago

@YurySolovyov there is a related proposal in the TC39 to add it as a feature to JavaScript: https://github.com/tc39/proposal-observable/ It's basically in the same shape as this, but this particular proposal is more about improving the DOM eventing API with a proven primitive that matches up with EventTarget well.

domenic commented 6 years ago

@YurySolovyov there's a lot of history here. This proposal has repeatedly failed to advance at TC39 (the committee in charge of JS standardization) precisely because it's not a great fit as a language feature. It doesn't expose any fundamentally new capabilities, it doesn't tie in to any syntax, and by staying at the language level it doesn't integrate well with popular platform APIs that would use it. As such there would be no advantage to putting it in the language, as opposed to just continuing to let people use libraries like RxJS. That's why it's been at stage 1 for over a year, unable to advance to stage 2 where "The committee expects the feature to be developed and eventually included in the standard ".

The committee's feedback was to do precisely what @benlesh has done, and propose working with the DOM community to create a feature worth shipping in engines because it ends up with deep integrations with the platform.

Note that being specified in the DOM spec doesn't prevent it from being used across the different JS-using ecosystems, like Node.js. As we've seen so far with URL, TextEncoder/TextDecoder, and the performance.* APIs, there are a lot of APIs specified outside the core language which are available across many different JS platforms. I would expect Observable to become one of these, and indeed you can see the beginnings of that discussion happening above :).

mgol commented 6 years ago

@domenic

This proposal has repeatedly failed to advance at TC39 (the committee in charge of JS standardization) precisely because it's not a great fit as a language feature. It doesn't expose any fundamentally new capabilities, it doesn't tie in to any syntax, and by staying at the language level it doesn't integrate well with popular platform APIs that would use it.

Out of curiosity - what was different in promises vs. observables that the former found their way in the core language then? Was async-await already in TC39 minds when promises were getting standardized?

domenic commented 6 years ago

Yep, that and module loading.

rauchg commented 6 years ago

It doesn't expose any fundamentally new capabilities, it doesn't tie in to any syntax

Is is out of the question that a native Observable type could be await-ed? I think one of the core advantages of Observable in the first place is that they are an effective superset of Promise.

appsforartists commented 6 years ago

@rauchg There might be a better venue for that question (e.g. https://github.com/tc39/proposal-observable/), since I think this issue is trying to stay focused on implementor's perspectives on the EventTarget use case.

ljharb commented 6 years ago

Observable has been at stage 1 in the TC-39 for over a year now

It's worth noting that the only meeting @jhusain has even attempted to advance the proposal beyond stage 1 at TC39 in the last few years is, according to the agendas, May 2017. There was a non-advancement update in September 2016 regarding cancellation; prior to that, May 2016 and on 3 meeting agendas in 2015.

I don't think it's a fair characterization that "TC39 is going too slow"; the committee has only been given 6 chances in the entire multi-year history of the proposal to discuss it, only one of which asked for stage 2 (it's likely that stage advancement was brought up in more than one meeting, of course; I'm not checking the notes, only the agendas - but either way, it's still not been frequently brought up)

@jhusain are you planning on adding an agenda item in January to talk about Observables, and hopefully advance them?

dcleao commented 6 years ago

Observable would be as a core primitive as Promise is, and thus, imo, it should be defined at the language level. Please spend your strengths pushing TC39 to advance on it.

matthewp commented 6 years ago

I don't think the WHATWG should restrain itself from improving the ergonomics of the web platform just because TC39 might, theoretically, be interested in a feature at some unknown future time (despite not being that interested in it for several years).

What exactly is it about Observable that is a language feature that, for example, ReadableStream is not? I think it was absolutely the right choice to design that feature in WHATWG.


@benlesh is it your intention not to define an Observable constructor at this time? Or is that something you want to add immediately?

ljharb commented 6 years ago

@matthewp i'm not sure where "not being that interested in it" comes from; many of us are very interested in it. See https://github.com/whatwg/dom/issues/544#issuecomment-351561091 - TC39 simply hasn't been given many meeting opportunities to discuss it.

matthewp commented 6 years ago

I wish you had chosen to respond to the rest of my comment and not just the side-note that I put in parentheses. I wish I had omitted that as it obviously distracts from the point I was trying to make; that this specification doesn't appear to need language-level support and can be implemented to suit each platform's needs, the same way that ReadableStream is not a EMCA api.

ljharb commented 6 years ago

@matthewp Specifically, I think that the language needs a primitive for "multiple temporal values", in the way that an array/iterable is a primitive for "multiple scalar values". Specifically, the language needs an answer to the fourth quadrant in https://github.com/kriskowal/gtor#a-general-theory-of-reactivity ("plural" + "temporal"). I don't feel strongly that an Observable necessarily needs to be it; but one needs to exist, and it would be ideal if, like Promises, future web and node APIs were interoperable with it.

domenic commented 6 years ago

The language has async iterators for that quadrant; those make more sense at the language level, given their integration with async generators and for-await-of. Observables are much more of a library-level feature (including the DOM as a popular library here).

othermaciej commented 6 years ago

Definition of AbortController, for newbs like me who don't know: https://dom.spec.whatwg.org/#interface-abortcontroller

A few questions:

  1. Does the Promise return of first() mean that event dispatch to a client that registered with first() takes an extra trip around the event loop? If so, wouldn't that hurt performance? What is the benefit?
  2. Do the subscription methods that take an Observable potentially have the same issue as 1?
  3. Could someone provide an explanation of the semantics of takeUntil()? It is not clear to me from the name and example.
  4. Does the map/filter stuff really provide meaningful syntactic sugar? For example, take this case:
element.on("click").
    filter(e => e.target.matches(".foo")).
    map(e => ({x: e.clientX, y: e.clientY })).
    subscribe(handleClickAtPoint);

What if on() was just a very short synonym for addEventListener instead? Then you could write:

element.on("click", 
           e => if (e.target.matches(".foo")) {
                handleClickAtPoint({x: e.clientX, y: e.clientY}) 
           })

This seems equally clear, and likely would be more efficient, since only one JS function is called per dispatch instead of 3. What's the win?

It's not totally clear to me why the first version is better. If anything, a bigger win to ergonomics would be adding Point Event.clientPoint.

jhusain commented 6 years ago

@ljharb I think it is fair to characterize the Observable proposal as being stalled in the TC-39. Stage 2 has been blocked on three separate occasions. Here's a summarized timeline:

  1. November 2015: Members of committee object to stage 2 advancement on the basis that the AsyncIterator and Observable proposals should advance in lockstep to ensure they are harmonized.

AsyncIterator proposal subsequently reaches stage 2.

  1. May 2016: Champions present evidence that Observable and AsyncIterator each have different use cases, and can be adapted to each other. Committee acknowledges that Observable meets criteria for stage 2. However members of the committee object to stage 2 on the basis that Observable should be harmonized with CancelToken proposal introduced in the same meeting.

Over the next several months the Observable proposal is significantly reworked to incorporate CancelTokens as their cancellation primitive. However the CancelToken proposal is eventually withdrawn due in part to concerns that consensus could not be achieved in TC-39.

  1. May 2017: Original proposal is submitted for advancement to stage 2. Some committee members object based on concerns there has not been insufficient consultation with implementers.

It's worth noting that none of the recent objections to advancement had to do with the design of Observable, which is very mature. Today the champions of the Observable proposal are optimistic that we can demonstrate its value to implementers and web developers. However, even if we do that, Observable still has an uphill battle in TC-39 for other reasons:

  1. Observable does not require syntax, and consequently, committee members have questioned whether there is a compelling reason to standardize.
  2. Due to the failure of the TC39 CancelToken proposal, the web has a standardized cancellation primitive (AbortController) and JavaScript does not. At least one implementer has already expressed concerns about adding an additional cancellation primitive to the language and consequently to the web platform. This would present yet another hurdle to advancing the Observable proposal in TC39.

Standardizing Observable in WHATWG allows the champions to harmonize Observable with AbortController, and pursue a more ambitious proposal than we could reasonably hope to get through TC-39 in a timely fashion (see addition of Array methods). We are excited by the prospect of finally addressing the many well-known pain points with DOM event APIs.

It's important to note that the door remains open to standardize a subset of the Observable proposal in the future should the need arise in the language for a push stream primitive. Hopefully by that time Observable will have already demonstrated its value to implementers and developers alike.

--note: make a few edits for clarity

On Dec 13, 2017 3:41 PM, "Jordan Harband" notifications@github.com wrote:

Observable has been at stage 1 in the TC-39 for over a year now

It's worth noting that the only meeting @jhusain https://github.com/jhusain has even attempted to advance the proposal beyond stage 1 at TC39 in the last few years is, according to the agendas, May 2017 https://github.com/tc39/agendas/blob/902c6d30d3b054a027a1c33b8d5642b3e2632209/2017/05.md. There was a non-advancement update in September 2016 https://github.com/tc39/agendas/blob/69cb34bb9346b792bdd92dd5b33beb0c7e404430/2016/09.md regarding cancellation; prior to that, May 2016 https://github.com/tc39/agendas/blob/c18e7bf7d8f32d722230d57f6f621ef4e01085d4/2016/05.md and on 3 meeting agendas in 2015.

I don't think it's a fair characterization that "TC39 is going too slow"; the committee has only been given 6 chances in the entire multi-year history of the proposal to discuss it, only one of which asked for stage 2 (it's likely that stage advancement was brought up in more than one meeting, of course; I'm not checking the notes, only the agendas - but either way, it's still not been frequently brought up)

@jhusain https://github.com/jhusain are you planning on adding an agenda item in January to talk about Observables, and hopefully advance them?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/whatwg/dom/issues/544#issuecomment-351561091, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcfr_y8DAFsvh8uXuX7GjCn1MzvBGYsks5tAGC6gaJpZM4RADEv .

appsforartists commented 6 years ago

@othermaciej Thanks for the link to AbortController. To see an example of how it's used elsewhere, here's a blog post @jakearchibald wrote showing how abort() works with fetch().

Here are the docs for takeUntil. In short, it aborts the subscription when its argument emits a value. Something like this:


Observable.prototype.takeUntil = function (cancellation$) {
  return new Observable(
    (observer) => {
      const upstreamSubscription = this.subscribe(observer);
      const cancellationSubscription = cancellation$.subscribe(
        () => {
          upstreamSubscription.abort();
          cancellationSubscription.abort();
        }
      );
    }
  }
}

element.on('mousemove').takeUntil(element.on('mouseup')).subscribe(console.log);

would log all the mousemoves until the next emission from mouseup. Then, both the subscriptions to mousemove and to mouseup would be aborted.

@benlesh has written more about this approach. Note: in that article, abort is called unsubscribe.

othermaciej commented 6 years ago

@appsforartists thanks for answering my question 3. I can see how takeUntil is handy. It makes me wonder how subscription handlers can subscribe themselves conditionally otherwise. Do they need to be a closure capturing the AbortController return from subscribe?

Still interested in the answers to 1, 2 and 4.

gsans commented 6 years ago

Quick note for @appsforartists. Just want to point a common misunderstanding regarding takeUntil as you didn't add it to your code snippet.

It doesn't only unsubscribe but also completes the Observable. takeUntil will trigger the complete callback if present. I think is worth mentioning for accuracy.