-
Notifications
You must be signed in to change notification settings - Fork 295
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
Improving ergonomics of events with Observable #544
Comments
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 |
Couldn't this be covered with element.on('click', { preventDefault: true })
.filter(/* ... */)
// etc. |
Cool! I propose to call them DOMservables |
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 It really is just A Better addEventListener (TM). ^_^ |
(It seems my concern indeed only applies to promise returning methods, such as |
@benlesh Can you speak more to why the |
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 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. |
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 |
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). |
FWIW in practice the DOM already makes the distinction of function-vs-object in the case of 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) |
@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. |
@jhusain it could also be handled with button.on('click').map(e => (e.preventDefault(), e)).first() |
The biggest thing I see missing from this proposal is an official way to use userland operators. @benlesh, do you think To the question about frameworks/libraries, as the author of a userland implementation, there are three core types that I'm interested in:
If Furthermore, if |
@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. |
@appsforartists Nitpick, but a |
Not to bikeshed, but the name of that method seems to keep flipping between |
@johanneslumpe Agree that the information architecture is a bit wonky. I think I originally learned |
@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
|
@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) |
@appsforartists, that's an RxJS implementation thing. Unrelated to this issue. |
For Node.js to accept (and implement) this, the |
@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. |
@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. |
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
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 For that reason, both Promises and Observables can only be typed on error as One solution would be to
which would make the following example possible
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 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! |
The IDL proposed in the OP allows a second But I'd also like to point out jQuery's |
At the risk starting the bikeshedding wars around the API; would |
@keithamus Certainly! But there's already a huge amount of industry momentum around |
To be clear, anything other than |
Observables would add ergonomics to EventTarget in two ways: having a standard interface that allows reusable abstractions, and adding pull semantics at the stream level. The issue there is that observables are just a stage-1 proposal, so they're not standard and won't be for years at best, and also that stream-level pull semantics are limited compared to value level. Async iterables are now a standard stream primitive in the language, and they offer value-level pull semantics using promise generation, which integrates nicely with async functions and other language constructs and is familiar to users. Converting the EventTarget push streams to iterables requires buffering, but there is a standard solution for that in Streams API, or even a simplified last-value cache could be used (which removes the risk of leaking with the tradeoff of having to be consumed serially). The workaround to use Instead of waiting for observables for years, async iteration can be supported now. It'd enable a pattern like this: for await (const event of element.addEventListener('click')) {
console.log(event);
} Calling |
See the OP - this is about speccing observables in DOM, not in TC39.
See #544 (comment) |
Of note: I think we could simplify this proposal quite a bit if we moved to something more like I'm referencing here: tc39/proposal-observable#201 |
For DOM it remains problematic I think that @pemrouz's https://github.com/tc39/proposal-emitter does not have that downside. |
What's stopping those methods returning an |
This might be a bit off-topic, but what about a stronger integration with DOM beyond events? From my perspective, What I mean is something like this: partial interface Node {
void bindTextContent(Observable<string> observable, optional AbortSignal signal);
};
partial interface Element {
void bindAttribute(string name, Observable<string> observable, optional AbortSignal signal);
void bindAttribute(string name, Observable<bool> observable, optional AbortSignal signal);
}; // the element will become a dynamic counter
element.bindTextContent(timer(0, 1000));
// the element will change class each second
element.bindAttribute('class',
interval(1000).pipe(
map(x => x % 2 == 0?'red':'blue')
)
);
// the element would disappear and reappear each second
// the `hidden` attribute basically gets removed when the observable value is `false`
element.bindAttribute('hidden', interval(1000).pipe(map(x => x % 2 ==0))); Of course this raises some further questions, most importantly when to subscribe and when to cancel that subscription. I guess (but am not sure) that automatically subscribing to bound And again, sorry if this is off-topic, but I assume that this issue is one of the main reasons for pushing forward standardization of |
Hey, I think it might be interesting to revisit this now that AddEventListenerOptions includes signal with a really smallish API. Maybe something like: partial interface EventTarget {
// since stuff in AddEventListenerOptions like `once` is relevant after subscribing it should probably live there?
// Also, since the name `on` was objected to 🤷
Observable observe(DOMString type);
};
[Constructor(/* details elided */)]
interface Observable {
// unsubscribing already works with AddEventListenerOptions
void subscribe(Function next, optional Function complete, optional Function error, optional AddEventListenerOptions);
Promise<void> forEach(Function callback, optional AddEventListenerOptions);
}; It might be even simpler to remove one of these (subscribe or forEach). In terms of Node.js - I don't see where/why Node would use this since our (I am personally a bit overextended - with the |
I'm not particularly convinced
Just to point out something, the only thing that can't be done synchronously is to receive a value. Streams (just like async functions) call basically everything synchronously, it's just that you can't receive the result until some future tick. e.g.:
The only real problem, and this is also shared by promises in general, is that if the promise is already resolved you can't abort it. Although as this is a problem for promises in general I don't think it's that great of an idea to create new primitives just to sidestep it in a few cases. Obviously there's also the whole const clickEvents = element.events("click", {
// just a small sync handler for enabling .preventDefault/.stopPropagation
syncHandler: (event) => { if (someCondition) event.preventDefault() },
});
for await (const event of clickEvents) {
// Do the complicated work here
} One particular advantage of using |
Hey everyone, I know this thread has been quiet for a while, but I've been looking at this proposal from an implementer perspective and I have a few questions, some of which stem from the fact that I think I only recently wrapped my head around the desired semantics here, so bear with me :)
|
@domfarolino So that's a LOT of questions, and I'll try to answer as many as I can. There are a couple of things worth noting here:
Observables as a primitiveObservables, at a basic level, are more primitive than Promises, Streams, or Async Iterables. They are, for lack of a better way to put it: "Specialized functions with some safety added". Observables could be in many shapes, and don't need to be confined to what you've seen above in this proposal. Consider that a straw man. If we wanted to build the ideal PRIMITIVE
Observables in the DOMIn the DOM, we don't necessarily need the whole primitive (although it would be the most useful thing to have). Instead what the DOM needs is the consumer side of the contract. To make this really powerful, the ideal API is to have the whole primitive though.
If you want to be able to accurately model what we can do with console.log('start');
eventTarget.addEventListener('whatever', console.log);
eventTarget.dispatchEvent(new Event('whatever'));
eventTarget.removeEventListener('whatever', console.log)
console.log('stop');
// synchronously logs:
// "start"
// [object Object]
// "stop"
I'm not surprised. I'm amenable to having the observable primitive be asynchronous... however, it's decidedly LESS primitive once you force scheduling:
However, if it IS synchronous, there are definitely some ergonomics gotchas when working with it. For example if you're trying to write an operator in RxJS that may terminate a source (like
No.
These things have been available for use for quite some time, pretty much since the dawn of Promises, and I haven't seen any real-world issues arise out of it. That said, promisifying observables is more of an ergonomics thing, since JavaScript has gone "all in" on Promise-related APIs (async/wait, for example). The entire world is used to the lack of cancellation in Promises. That said, I could take the promise features or leave them. But again, I've never seen promise conversion be the issue for observables. In fact, we're going to add
Think of it like this: @benlesh sees a pressing need to ship a real Observable primitive to the JavaScript community en masse, and I've tweaked my proposals several times over the years to try to reach an audience with the gatekeepers. Some facts:
I'm tired. I don't want to be the sole arbiter of the most popular Observable implementation. It's a primitive type. It should be in our platforms by any means necessary. We're shipping countless bytes over the wire to send these observables out to everyone, where if the actual primitive existed I believe more library implementors would use them. Especially if they performed well because they were native, and they had a better debugging story in dev tools (because they're native). I want RxJS to be a cute little library of helper methods over a native type that becomes slowly irrelevant as those features grow in the native platforms. (Thus completing the long-running statement "RxJS is lodash for events"). What would I find to be acceptable criteria for a good Observable type in the platform?See the requirements above. No "operators". Doesn't need any promise conversion stuff. That's all just "nice to have". This would be fine: const source = new Observable(subscriber => {
let n = 0;
const id = setInterval(() => subscriber.next(n++), 1000);
return () => clearInterval(id);
});
const ac = new AbortController()
const signal = ac.signal;
source.takeUntil(signal.on('abort')).subscribe(console.log); But for the sake of this proposal: The main thing it was tryingto do is make events more ergonomic. Like: eventTarget.on('click').takeUntil(signal.on('abort')).subscribe(() => console.log('clicked')) Ironically, @benjamingr , a LONG time supporter of RxJS may have killed any chance the community had at getting Observable on a platform when he added eventTarget.addEventListener('click', () => console.log('clicked'), { signal }); // This works today. The down side? The API we have above with div.on('mousedown').flatMap(downEvent => {
const divX = div.getBoundingClientRect().left;
const startX = downEvent.clientX;
return document.on('mousemove')
.map(moveEvent => divX + moveEvent.clientX - startX)
.takeUntil(document.on('mouseup'))
})
.subscribe(x => {
div.style.transform = `translate3d(${x}px, 0, 0)`;
}) It would be pretty cool to be able to do that without RxJS. The imperative version of that is easier to mess up in my opinion. And note that outside of This WHATWG proposal was painted as the best chance Observable ever had. My discussions with @domenic years ago signaled that he didn't believe it belonged in the ECMAScript standard (so presumably it's blocked there), and it was his determination that proposing it here was the best idea. I hope this answers all of your questions, @domfarolino, and I hope you found this helpful. Please feel free to ask more. You know where to find me. (My github notifications are a hot mess because of RxJS, and I'm not paid to work on anything in open source or on GitHub, so if you ping me here I might not see) |
Thanks so much for the very thorough response @benlesh. Some replies below:
Interesting. We of course could move incrementally, introducing Observable-returning APIs before making Observables themselves fully constructible by JS, however I do see value in providing the full API as a primitive up front. https://w3ctag.github.io/design-principles/#constructors recommends something similar, which is good news too.
I agree with this requirement, but it doesn't seem unique to user-constructed observables, right? For example, if unsubscribing from the platform-constructed Observable returned by
Thanks, the impact of being synchronous makes sense here, especially for events and cancelation. I agree that the sync semantics here are ideal and give the most flexibility.
Exactly, this is what I was asking about. Just wanted to make sure I understood the relationship between the two.
Can you clarify this (who exactly is tearing down here)? I understand that if complete()/error() is called on the Subscriber/wrapped Observer, we'd want to unregister "the subscription" from the event source for clean-up (it doesn't make sense to keep listening for events and having the "safe" RE promises: It's good to know that this hasn't been seen as fraught territory so far in the Observable-using community, though I do agree that for something like an Observables MVP, the Promise-ifying APIs could probably be initially left out. |
For developer ergonomics to improve, Observable has to ship with a complete operator library in my opinion. A standard
This might be a case of "worse is better" since it covers the large majority of use cases where no complex combination of event streams etc. is required. This is how the drag example above looks using div.addEventListener("mousedown", (downEvent) => {
const divX = div.getBoundingClientRect().left
const startX = downEvent.clientX
const abortController = new AbortController()
document.addEventListener("mousemove", (moveEvent) => {
const x = divX + moveEvent.clientX - startX
div.style.transform = `translate3d(${x}px, 0, 0)`
}, {signal: abortController.signal})
document.addEventListener("mouseup", () => abortController.abort(), {once: true})
}) Sure, the API is not as nice, but I would argue that it is easier to understand for most developers today and works in all modern browsers. The order of events is also preserved when reading the code (mousedown, mousemove, mouseup). There is a pitfall however: it is easy to create a memory leak by forgetting to add Once observables are part of the language, developers will expect common operators to be present. After advocating for |
For what it's worth I disagree with:
I acknowledge it covers one thing observables did better than EventTarget (and we absolutely should keep improving EventTarget), but it doesn't really compare I think? You can't The thing that has been blocking observables to my knowledge for the last few years is that they were not presented to the committee and concerns raised and research did not progress because no one is funding it. I don't expect you to spend another 1000 hours for free on this, but let's remember we can still have observables in JavaScript if someone funds the work to do it or does it. I think WhatWG's and TC39's concerns can be addressed to everyone's happiness. |
Hey everyone, just to follow up on this thread: @benlesh and I have been working on this recently over at https://github.com/domfarolino/observable where we have a more formal explainer for this proposal, so feel free to engage over there if you'd like! At some point with a few more details ironed out, I'd like to send an Intent-to-Prototype out for Chromium to express formal interest. |
@domfarolino as i recall, the main signal TC39 was missing was browser interest, so if Chrome is interested, it is definitely worth bringing back to TC39. |
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:
Integrating Observable into the DOM
We propose that the "on" method on EventTarget should return an Observable.
The
on
method becomes a "better addEventListener", in that it returns an Observable, which has a few benefits: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?
The text was updated successfully, but these errors were encountered: