Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedback from me with a Node perspective #30

Closed
benjamingr opened this issue Jul 28, 2023 · 7 comments
Closed

Feedback from me with a Node perspective #30

benjamingr opened this issue Jul 28, 2023 · 7 comments

Comments

@benjamingr
Copy link

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
Copy link
Author

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

@benlesh
Copy link
Collaborator

benlesh commented Jul 28, 2023

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
Copy link

ljharb commented Jul 28, 2023

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
Copy link
Collaborator

benlesh commented Jul 28, 2023

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
Copy link

ljharb commented Jul 28, 2023

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
Copy link
Author

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
Copy link
Collaborator

Thanks a lot for the feedback @benjamingr! I think given wintercg/proposal-minimum-common-api#72 and nodejs/standards-positions#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 #150 for talks of resource management and disposability. And see #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.

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

No branches or pull requests

4 participants