-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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: improved fetch
docs
#57296
base: main
Are you sure you want to change the base?
doc: improved fetch
docs
#57296
Conversation
cc @nodejs/undici |
I am personally a -1: the basic example has bad practices (not consuming the body if (!res.ok)); Dispatcher/Agent aren't exposed in node (so why document them here?); FormData, Response, Request, and Headers are already documented. |
@KhafraDev I took the basic example from Node.js 18 release notes so I thought it was ok, but it can be improved.
As commented here and answered here:
Moreover, there is also 3K views question on Stackoverflow.
Yes, but these classes are relevant to |
Thanks @lifeisfoo for the PR and @KhafraDev for the review! Jumping here because the discussion in this PR piqued my curiosity.
Curious, why it's a bad practice and what would be the good practice? |
Setting up a custom Agent is quite a common use case and we can't just redirect to the Undici documentation, because as you said, Dispatcher/Agent aren't exposed in node. What would be your suggestion? Shall we suggest to use |
The expanded Incidentally, I created this PR after struggling to work with a SOCKS proxy in Node.js, where the dispatcher override function is the key to using it.
From my point of view, documentation should aim to save developers hours by giving them all the information they need without having to look at the source code or perform searches on other websites. |
Updated
fetch
documentation:undici
undici
versionRefs: