-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
update fetch guide #34278
update fetch guide #34278
Conversation
Preview URLs (comment last updated: 2024-06-28 14:52:55) |
Here are all issues concerning this page:
Maybe check if this PR happens to fix any of them, or if they are easy enough to be addressed in this PR, or if they aren't really worth fixing? |
Thanks for this! As discussed in that issue, I'm open to this but there are some wrinkles and questions, so not sure if we want to do it now. The main bit of this is addressed. The rest seems to be about potentially splitting this guide into "basic fetch" and separate guides for more detailed things. Again I can see the value of that and if that's the way people want to go with this PR I'm happy to. But we'd need to think of what the other things would be. Maybe:
Added 47767e7 in this PR which should address that. I wonder if this indicates that we should have a separate "Making POST requests" guide, to provide a home for this and for the "upload a file" and upload forms" examples. I'm not at all familiar with the streams API but:
fetch(`/receive?channel=${channel}`).then(async res => {
const reader = res.body.pipeThrough(new TextDecoderStream()).getReader();
while (true) {
const { done, value } = await reader.read();
if (done) return;
output.append(value);
}
}); ...from https://glitch.com/edit/#!/fetch-request-stream?path=static%2Fmain.js%3A50%3A0 ? Added 404963d in this PR which should address that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
So for instance, can we have a streaming example like this: const url = "https://www.gutenberg.org/cache/epub/34225/pg34225.txt";
async function fetchAsStream(url) {
try {
const response = await fetch(url);
if (!response.ok) {
throw new Error(`Response status: ${response.status}`);
}
const reader = response.body.pipeThrough(new TextDecoderStream());
for await (const value of reader) {
console.log(value);
}
} catch (e) {
console.error(e);
}
} I've tried this in Chrome with network throttling, and it works nicely to deliver chunks of text as they come in (compared with the I think this is a lot clearer than the "read line by line" example, and shows the point of streaming responses quite clearly. WDYT? |
This commit: fbf188b adds the simpler streaming example, and also fixes #10086 by piping the response body through a UTF-8 decoder. I've confirmed (using https://www.gutenberg.org/cache/epub/30774/pg30774.txt) that:
|
Co-authored-by: Joshua Chen <[email protected]>
In c441596 I deleted https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Basic_concepts, because it only talked about "guard", which is an internal spec concept, and whose effect is, with this PR, now documented in the main "using fetch" guide. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving a verbal approval; if you don't get it merged in a week we can merge it
Co-authored-by: Joshua Chen <[email protected]>
(@sideshowbarker bumping this up your inbox) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me — the technical content of the prose and examples. Only found a couple minor proofreading fixes.
Co-authored-by: sideshowbarker <[email protected]>
Co-authored-by: sideshowbarker <[email protected]>
Thank you for the reviews! |
* upstream/main: (58 commits) Update arrow function documentation to clarify naming and assignment (mdn#34501) update fetch guide (mdn#34278) Replace alert in Learn/JavaScript/First_steps/Variables (mdn#34487) Replace alert in MDN/Writing_guidelines/Page_structures/Live_samples (mdn#34479) Fix typo (mdn#34486) Remove SVG color-profile attribute (mdn#34482) Remove SVG enable-background attribute (mdn#34483) Remove SVG kerning attribute (mdn#34475) Updated the description of `targetOrigin` to specify the intended re… (mdn#34114) Mention CSWH in WebSocket server guide (mdn#34411) Add note to CSP sandbox saying allow-top-navigation is redundant (mdn#34415) Mention navigator.languages may be truncated and Accept-Language may have fallback (mdn#34418) Remove IDB output "example", preferring live example (mdn#34464) Mention that pinch-zoom are also wheel events (mdn#34468) Mention that flex-basis is floored at min-content (mdn#34469) More content to Global object glossary (mdn#34471) Fix IDB cursor prev direction description (mdn#34463) Remove all line number references to inline code examples (mdn#34459) Remove link to notification example (mdn#34412) Replaces HTML entity glossary links/mentions with char reference (mdn#34391) ...
I liked the example https://web.archive.org/web/20240612035627/https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#supplying_request_options because it was a complete structure, and the other examples in the old page had examples for specific use cases where one could learn from the examples, not simply atomic pieces of the Fetch API. (I used this old example recently and found it helpful, then I noticed this documentation change). Would it be possible to add the Supplying Request Options example back? |
Of course, but I'd like to understand better what you mean by "a complete structure". The example at the start of the new page is also "complete" in that it runs, as is the "making a POST request" example and most of the other examples in the page. On the other hand https://web.archive.org/web/20240612035627/https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#supplying_request_options is not complete in the sense of showing all the options, it only shows some of them, and I'm not sure what's the basis for the choices it makes about which ones to show. Perhaps we could update the example to show all the options? I also think the old example is odd in that it passes options which are the same as the default, which makes me think why would someone do that. |
-1 to add it back, or to have any example that has every option configured. If you want to read up on all possible options, read the reference instead of the guide, which shows you how to use the API. You don't use the API by configuring every option; you change what you need. |
This is a rewrite of the "Using Fetch" guide": https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch.
The main impetus here was to have a place to talk about bodies being streams, so we can document the exceptions that get thrown when a body is "locked or disturbed" (#13208 (comment)). But unfortunately the structure of this document didn't lend itself to just adding something about that, so I have restructured and rewritten it pretty much completely.
The old doc reads like a collection of facts about fetch in no particular order. In this doc I have three main parts:
I have thrown out various things:
FormData
docs (https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest_API/Using_FormData_Objects#sending_files_using_a_formdata_object , but perhaps we should have better linking to that?)Other than that I've tried to keep anything in there that seemed useful, although I've reorganized it a lot.
As well as content about streaming responses, I have added quite a bit on cross-origin requests.
I have not described all the options. Specifically I've not described the following:
I feel like describing all of them would make this doc a real slog to read, so I tried describing "the main ones" but that's quite subjective.
I'd love feedback about whether this is a generally good "shape" of doc to have here and whether the scope is OK.
Also I this this doc needs careful review by people with really good Fetch knowledge, since some of it is tricky, and I don't know it very well.
In doing this I realize there's definite scope for more Fetch guides, like maybe separate ones of redirects, streaming, CORS, "using fetch securely", ... but I've not attempted that here. I'd welcome feedback on whether I should attempt that in this PR, especially given some of the issues in #34278 (comment)...