WICG / observable

Observable API proposal
https://wicg.github.io/observable/
Other
563 stars 13 forks source link

Feedback from me with a Node perspective #30

Closed benjamingr closed 1 week ago

benjamingr commented 1 year ago

Hey, Ben sent me this proposal, we're likely happy to adopt it in Node though I'll raise it internally with the project for feedback. I'm in the TSC now so I need to be more careful with accidentally sounding like I speak for the whole project which I don't.

In terms of API I think it's important to consider compatibility with https://github.com/tc39/proposal-iterator-helpers and https://github.com/tc39/proposal-explicit-resource-management .

Regarding the "history" section, I don't think TC39 was ever opposed to the proposal they just had concerns though WHATWG may be a better standards body for it given the deep web platform integration (e.g AbortSignal support would be hard through TC39).

Regarding "prior art" - I think stuff like MobX/Vue.reactive etc are the same thing but it's a hard case to make @headinthebox makes it (here).

Regarding concerns with promises - I'm wondering if being able to preventDefault/stopPropagation is actually important for these promise returning events. I've seen this concern raised many times when discussing streams in Node and in practice we shipped the iterator-helpers proposal and streams are otherwise mostly synchronous and no one noticed or complained in practice so from my PoV it's not a problem in practice (and neither is firehose to be fair).

benjamingr commented 1 year ago

(Personally I am excited and happy you picked this up and are working on this, I was an advocate of the older effort)

benlesh commented 1 year ago

In terms of API I think it's important to consider compatibility with https://github.com/tc39/proposal-iterator-helpers and https://github.com/tc39/proposal-explicit-resource-management .

Full agreement.

To that end, I had told @domfarolino that we should try to align the API with the iterator helpers. I don't think we've explicitly discussed disposable/resource management... however, I'm amenable to a lot of things here, and I think it's a good add. In RxJS land, we're going to add Symbol.dispose to Subscription, which will basically just alias unsubscribe.

ljharb commented 1 year ago

fwiw i think this utterly belongs in TC39 and nowhere else, and if there's now browser interest, that feels like it would help it feel motivated - so i hope you bring it back to TC39.

benlesh commented 1 year ago

fwiw i think this utterly belongs in TC39 and nowhere else

Years ago when I wrote the WHATWG issue, discussing with @domenic, he believed it did not belong in the TC39. And that was the general feedback that put things on hold way back in 2015, IIRC. I don't know if @jhusain still monitors his github (I hope he doesn't, because he's living the dream right now), but maybe he recalls.

ljharb commented 1 year ago

That was not my sense being in the room at those times, but either way the makeup of the committee and the state of the language is quite different than it was 8 years ago, so I strongly urge you to reattempt it.

benjamingr commented 1 year ago

I can see the case that like EventTarget, Observable which serves a similar purpose belongs in the same place. Like with DOMPromise/DOMFuture - I don't see a problem with standardizing in both bodies so both WHATWG and TC39 APIs can build on top of Observable.

From Node.js's point of view - I don't really see why it would matter in what standards body it belongs but I'm happy to ask around in WinterCG.

domfarolino commented 1 week ago

Thanks a lot for the feedback @benjamingr! I think given https://github.com/wintercg/proposal-common-minimum-api/issues/72 and https://github.com/nodejs/standards-positions/issues/1, we can probably close this issue now. Also, the remaining points of feedback here I believe are captured by other issues that have been filed against this proposal in the intervening time.

Specifically, see https://github.com/WICG/observable/issues/150 for talks of resource management and disposability. And see https://github.com/WICG/observable/issues/148 for discussion of unsubscribe() if we decided that the "synchronous firehose" problem is not significant enough to justify the clunkiness of forcing users into AbortControllers & AbortSignals for unsubscription.