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

doc: add undici notes #39057

Closed
wants to merge 5 commits into from
Closed

doc: add undici notes #39057

wants to merge 5 commits into from

Conversation

ronag
Copy link
Member

@ronag ronag commented Jun 17, 2021

We discussed in #38533 that once undici@4 is released we would in some fashion update the documentation to encourage users to use undici instead of the core http client.

I've tried to identify where in the documentation we might want to add a note and also updated the few examples I could find that use the core client to instead use undici.

I think I need a little help here in terms of what would be the best way to achieve this "encouragement", e.g. where in the documentation and what is an appropriate phrasing. I don't think we want to deprecate the core client just yet. We consider undici stable per se. However, due limited usage in the ecosystem I would still consider it experimental in the context of Node core.

Refs: #38533

@ronag ronag added the http Issues or PRs related to the http subsystem. label Jun 17, 2021
@ronag
Copy link
Member Author

ronag commented Jun 17, 2021

@nodejs/http @nodejs/documentation

@github-actions github-actions bot added the doc Issues and PRs related to the documentations. label Jun 17, 2021
@ronag
Copy link
Member Author

ronag commented Jun 17, 2021

Also pinging @nodejs/tsc for visibility.

Copy link
Contributor

@mscdex mscdex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't make any sense to me. Why would we want to recommend a module that won't have the same maintenance/release/LTS/etc. guarantees as node core? I can't imagine many people will want to install an external module in order to keep http-related functionality in their projects (yes, I know this is merely changing the documentation at this point but why bother if it's never going to advance beyond that?).

As a major core module of node (IMO), I think it makes more sense to improve the http module instead. Perhaps it would help to gather a list of the top nn issues end users have with the existing implementation and work on fixing those.

@jasnell
Copy link
Member

jasnell commented Jun 17, 2021

I think it makes more sense to improve the http module instead.

While I know a decision hasn't yet been made whether undici will become part of core, "improving the http module" is exactly what undici is doing. That's why it was brought into the nodejs project in the first place.

@jasnell
Copy link
Member

jasnell commented Jun 17, 2021

@ronag ... at this point, I think it's best not the change the existing documentation points and examples. What would be better, I think, is adding a new section to the end of the http.md documentation that introduces undici as an alternative, with clear caveats that it is experimental and being worked on as a potential replacement for the existing http client API.

@ronag
Copy link
Member Author

ronag commented Jun 17, 2021

I think it makes more sense to improve the http module instead.

While I know a decision hasn't yet been made whether undici will become part of core, "improving the http module" is exactly what undici is doing. That's why it was brought into the nodejs project in the first place.

Second this. IMO improving the existing code is a walk we have tried and reached a dead end. The existing http client code is even hindering improvements to the http server code since they are interlinked.

@ronag
Copy link
Member Author

ronag commented Jun 17, 2021

clear caveats that it is experimental and being worked on as a potential replacement for the existing http client API.

I guess our goal here is to encourage users to try out undici so we can get more users so that we can eventually pull it into core. I'm not sure adding a note in the bottom of the documentation does much in this regard?

@mhdawson
Copy link
Member

@ronag , what I thought would be the next step based on discussions (but I may have misunderstood) was more along the line of what @jasnell suggested above but I'd also support adding a reference to it near the start of the existing http doc that highlights the work and points to the new section.

@ronag
Copy link
Member Author

ronag commented Jun 17, 2021

@ronag , what I thought would be the next step based on discussions (but I may have misunderstood) was more along the line of what @jasnell suggested above but I'd also support adding a reference to it near the start of the existing http doc that highlights the work and points to the new section.

I’m good with that.

@mscdex
Copy link
Contributor

mscdex commented Jun 17, 2021

"improving the http module" is exactly what undici is doing

The same could be said about lots of modules on npm, but the difference here seems to be that we would be discontinuing work on the built-in http module somewhere down the road and then (runtime) deprecating it at some point. That is what I disagree with the most here.

Additionally, reading through the linked issue I got the feeling that moving undici into core was not desired (at least by the maintainers of that module).

@jasnell
Copy link
Member

jasnell commented Jun 17, 2021

@mscdex:

but the difference here seems to be that we would be discontinuing work on the built-in http module somewhere down the road and then (runtime) deprecating it at some point. That is what I disagree with the most here.

I don't believe anyone has suggested either (discontinuing work or deprecating the existing module) would ever be the case. I imagine it would be a situation similar to the legacy URL parser vs. the WHATWG URL parser. They can exist in parallel with no intent to ever deprecate.

@ronag
Copy link
Member Author

ronag commented Jun 17, 2021

@mscdex:

but the difference here seems to be that we would be discontinuing work on the built-in http module somewhere down the road and then (runtime) deprecating it at some point. That is what I disagree with the most here.

I don't believe anyone has suggested either (discontinuing work or deprecating the existing module) would ever be the case. I imagine it would be a situation similar to the legacy URL parser vs. the WHATWG URL parser. They can exist in parallel with no intent to ever deprecate.

I have suggested deprecation. Or at least brought it up for discussion.

@jasnell
Copy link
Member

jasnell commented Jun 17, 2021

@ronag:

I have suggested deprecation.

Ah, ok. Well, for that I would suggest using the new Legacy status and leaving it at that. If folks want to work on it, great.

@ronag
Copy link
Member Author

ronag commented Jun 17, 2021

@ronag:

I have suggested deprecation.

Ah, ok. Well, for that I would suggest using the new Legacy status and leaving it at that. If folks want to work on it, great.

Ah yes, it was legacy status we discussed. Sorry.

@mscdex
Copy link
Contributor

mscdex commented Jun 17, 2021

I don't believe anyone has suggested either (discontinuing work or deprecating the existing module) would ever be the case. I imagine it would be a situation similar to the legacy URL parser vs. the WHATWG URL parser. They can exist in parallel with no intent to ever deprecate.

In the linked issue @ronag commented:

Part of the reason for this is that some of the people that usually engage with http client issues is @mcollina, @dnlup or myself. Whom I believe are mostly focusing on nodejs/undici, which we feel is a better alternative and the future, and have limited interest in putting further effort into the node core http client.

So basically if we started pushing people towards undici when http (core) contributors are focusing more of their time and effort on that module, I don't see how http will ever see improvements/fixes/etc.? At that point http will become stagnant and I don't think that's a path we should be taking.

I believe the comparison to URL parsing illustrates exactly my point: how much effort has there been since marking url.parse(), etc. as "legacy" in improving/fixing things with that API? Anecdotally I would say significantly less compared to the WHATWG implementation. However, at least in the case of URL parsing, both the legacy and the WHATWG implementation live in node core, so nobody has to go and npm install .... something extra.

I still would greatly prefer that we focus on the top issues people have with the http module and go from there. It's fine if there are third-party modules that improve upon node core, but if we can solve some of the major pain points of the built-in modules I think we should try and make a serious effort to do so instead of just giving up and redirecting people to an external module.

@StarpTech
Copy link

StarpTech commented Jun 17, 2021

As a major core module of node (IMO), I think it makes more sense to improve the http module instead. Perhaps it would help to gather a list of the top nn issues end users have with the existing implementation and work on fixing those.

but the difference here seems to be that we would be discontinuing work on the built-in http module somewhere down the road and then (runtime) deprecating it at some point. That is what I disagree with the most here.

I second this. Nobody wants to invest in software that is fragmented at its core. We are talking about one of the most used and basic features in Node.js. How should we motivate people to work on a core module when the issues are already solved with an external module by the same Node.js team?

@mcollina sorry to quote you but this reminds me of your tweet https://twitter.com/matteocollina/status/1400845805074911243

A lot of folks consider Node.js a product. It’s not. It’s an OSS community that maintains a JS runtime. If you want something to happen, come and contribute.

Then let us handle it as such. Node.js needs a more aggressive releasing strategy to allow such innovations and refactorings in his own house. Not sure how else should we keep with the time ....Deno 🦕 ...

I prefer to not deprecate/mark the "HTTP" module when undici is handled as a regular npm package. This is my personal opinion, as an outsider.

@jasnell
Copy link
Member

jasnell commented Jun 17, 2021

So basically if we started pushing people towards undici when http (core) contributors are focusing more of their time and effort on that module, I don't see how http will ever see improvements/fixes/etc.? At that point http will become stagnant and I don't think that's a path we should be taking...
I still would greatly prefer that we focus on the top issues people have with the http module and go from there. It's fine if there are third-party modules that improve upon node core, but if we can solve some of the major pain points of the built-in modules I think we should try and make a serious effort to do so instead of just giving up and redirecting people to an external module.

Again, what I'd like to see ultimately is undici get merged into core.

As with everything, node.js core contributors put their time and energy where they see the most value. Nothing is stopping anyone from jumping in to help maintain the existing module if they wish to do so.

@jasnell
Copy link
Member

jasnell commented Jun 17, 2021

@StarpTech:

I prefer to not deprecate/mark the "HTTP" module

To be clear, deprecating the existing HTTP module is not on the table.

@ronag
Copy link
Member Author

ronag commented Jun 17, 2021

At that point http will become stagnant and I don't think that's a path we should be taking.

IMHO, it's already stagnant. We have reported issues that no one is looking into resolving.

@StarpTech
Copy link

Nothing is stopping anyone from jumping in to help maintain the existing module if they wish to do so.

Basically, I can rewrite the http module and release in the next major Node.js version?

@jasnell
Copy link
Member

jasnell commented Jun 18, 2021

Basically, I can rewrite the http module and release in the next major Node.js version?

The conversation here is derailing sharply from this PR and should likely be moved into a discussion.

@ronag: Specifically for this PR, I'll go back to my original feedback... let's limit the mention to undici (for now) to a single section in the http.md or https.md doc, with a simple example and explanation of why it's being developed and leave all of the existing examples and docs unmodified.

@ronag
Copy link
Member Author

ronag commented Jun 18, 2021

I’ll update the PR accordingly.

@tniessen
Copy link
Member

Ah, ok. Well, for that I would suggest using the new Legacy status and leaving it at that. If folks want to work on it, great.

This came up in a TSC meeting a while ago and the primary concern was that undici is not in core and not mature enough to be used as a replacement in production. Marking http (and https, presumably) as legacy implies that we have a better and stable alternative that provides all features of these modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants