Editorial: Add prose about constructing a request #1585
Conversation
|
@annevk I'm sure some things here can be made more accurate but I thought I'd take a first stab as I feel this is needed... |
There was a problem hiding this comment.
Here are my thoughts about how this should work, but you should prioritize opinions from people who've had trouble calling into fetch, like @tabatkins and @npm1.
fetch.bs
Outdated
| that existed before the CORS mechanism was introduced. To use "<code>no-cors</code>" in a new | ||
| feature, please discuss on the <a href=https://github.com/whatwg/fetch>Fetch Github repository</a>. |
There was a problem hiding this comment.
This should link somewhere more precise than the top level of the repository. @annevk/@domenic, do you want Issues for future proposals to use no-cors, or would it make more sense to enable Github Discussions for it?
fetch.bs
Outdated
| @@ -8380,6 +8380,56 @@ correctly. This section aims to give some advice. | |||
|
|
|||
| <p class=XXX>This is a work in progress. | |||
|
|
|||
| <h3 id=fetch-elsewhere-request>Constructing a request</h3> | |||
There was a problem hiding this comment.
We should link to here from https://fetch.spec.whatwg.org/#requests.
fetch.bs
Outdated
| multiple associated parameters. | ||
|
|
||
| <p>A <a for=/>request</a> oftenmost has a <a for=request>client</a>, which is the | ||
| <a>environment settings object</a> that initiatated the request, and the receiver of the different |
There was a problem hiding this comment.
I'm not sure it's always obvious which settings object "initiated the request". I think we could say that if a request is caused by a call into a platform object, it's the "relevant settings object" for that object. What other situations are there?
- The request is on behalf of the UA itself, for which we can ask people to see if Add unsafe-no-cors mode #1533 does what they need?
- The request is on behalf of an origin, but detached from any particular window, in which case maybe we want to check if that's really a good behavior for the web, and they should discuss it in this repo (see below for a question about how to organize such discussions)?
- Anything else? I haven't trawled through uses of fetch to collect more cases, but there probably are some.
fetch.bs
Outdated
| <a>environment settings object</a> that initiatated the request, and the receiver of the different | ||
| <a for=/>response</a> callbacks. It then expects a <a for=request>URL</a>, and an HTTP |
There was a problem hiding this comment.
I think we can skip the mention of response callbacks here and instead mention it in the "Invoking fetch" section.
fetch.bs
Outdated
| <p>Requests accept an optional <a for=request>header list</a> parameter, custom HTTP headers to be | ||
| sent alongside the <a for=/>request</a>. Note that sending custom headers may have implications, | ||
| such as requiring a <a>CORS-preflight fetch</a>, so handle with care. |
There was a problem hiding this comment.
If we write this like If you need to send custom HTTP headers in your request, put them in the [=request/header list=] parameter. then we tell callers what to do , which should be easier for them to follow than if we just state facts about request objects.
fetch.bs
Outdated
| <p>A <a for=/>request</a> accepts a few security-related parameters that are often configured, such | ||
| as <a for=request>integrity metadata</a>, a string hash of the actual data that verifies that the | ||
| received resource matches an expectation, a <a for=request>cryptographic nonce metadata</a> and | ||
| <a for=request>parser metadata</a>, which allows more fine-grained security via | ||
| <cite>Content Security Policy</cite>, and <a for=request>referrer policy</a>, which allows the | ||
| <a for=/>request</a> to use a different <a for=/>referrer policy</a> from the one provided by the | ||
| <a for=request>client</a>. [[!CSP]] [[!REFERRER]] |
There was a problem hiding this comment.
I'd give each of these a paragraph or an item in a <dl class="switch">. Like
If you need to verify the received data hashes to a particular value, set [=request/integrity metadata=].
fetch.bs
Outdated
| @@ -8380,6 +8380,56 @@ correctly. This section aims to give some advice. | |||
|
|
|||
| <p class=XXX>This is a work in progress. | |||
|
|
|||
| <h3 id=fetch-elsewhere-request>Constructing a request</h3> | |||
There was a problem hiding this comment.
A high-level comment for the whole section, which I've elaborated on in several of the specific comments:
I think we want this section to be instructions to the authors of calling specifications -- a how-to guide. It might make sense to also improve the descriptive text in https://fetch.spec.whatwg.org/#requests, but that's not the primary hole this section needs to fill.
There was a problem hiding this comment.
Agree that it would be good to format this more as a how-to, and in particular have more bullet lists (and mandatory parameters first and common optional parameters after). A hard part about using fetch is how to invoke it and continue processing the response after it's done. Based on the title of the section it would be out of scope but wanted to point out the gap there as well. There are not many good examples of using fetch in a nontrivial way where the result is 'awaited' by the algorithm, which then does something with the response.
There was a problem hiding this comment.
I focused on requests first because the callback part is already kinda covered in the subsequent bit. Is something missing in that section?
There was a problem hiding this comment.
I think that subsequent section could use an example of how to use it. I think a lot of people expect to be able to say:
- In parallel:
- fetch the request.
- Wait for the fetch to finish.
- Use the response.
And with the setup of the processResponse algorithms, that just doesn't work. You have to do something more like
- Fetch the request, with the following algorithms as arguments:
- processResponse
- Steps
- processResponseConsumeBody
- Other Steps
It might also be helpful to rewrite that section to map from a spec's goals about how it'll handle the response, to the set of algorithm arguments it ought to pass.
But +100 for doing all that in a separate PR. Writing this section is a great first step.
There was a problem hiding this comment.
Gotcha, this makes sense. I will get to improving the callback section once we finish with the request section.
I also intend to add a section about the different cross-origin protections (CORS, TAO, CORB, CSP) with a short guide on which to use for what purpose.
fetch.bs
Outdated
| <a for=/>response</a> callbacks. It then expects a <a for=request>URL</a>, and an HTTP | ||
| <a for=request>method</a>, `<code>GET</code>` by default. `<code>POST</code>` and | ||
| `<code>PUT</code>` requests also accept a <a for=request>body</a>, which is sent together with the | ||
| request. |
There was a problem hiding this comment.
These probably belong first:
Start by setting the HTTP [=request/method=] and target [=request/URL=] as described in [[HTTP]]. If your
POSTorPUTrequest needs a body, you can assign or stream it to [=request/body=].
fetch.bs
Outdated
| <p>A <a for=/>request</a> also accepts a few metadata parameters, the important one being | ||
| <a for=request>destination</a>. <a for=request>destinations</a> affect |
There was a problem hiding this comment.
Perhaps:
Set your request's [=request/destination=] according to how your specification is going to use the response. If you need a new destination, ...
This paragraph should link to the note that's under https://fetch.spec.whatwg.org/#request-destination-script-like that describes what each of the destinations and initiator types are for.
I think people are required to set their destination, in which case, this paragraph should appear before the optional parameters.
fetch.bs
Outdated
| <a href=https://github.com/whatwg/fetch>Fetch Github repository</a>. An aditional metadata | ||
| paramater is <a for=request>initiator type</a>, which specifies that the <a for=/>fetching</a> |
There was a problem hiding this comment.
Split this paragraph so people notice that they also need to set the initiator type. Do they always need to set it, or just if they want their resource to participate in [[resource-timing]]? Is the note under https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-initiatortype the best description of how to set this?
|
Also, thanks a bunch for starting this section! |
fetch.bs
Outdated
| @@ -8380,6 +8380,56 @@ correctly. This section aims to give some advice. | |||
|
|
|||
| <p class=XXX>This is a work in progress. | |||
|
|
|||
| <h3 id=fetch-elsewhere-request>Constructing a request</h3> | |||
There was a problem hiding this comment.
Agree that it would be good to format this more as a how-to, and in particular have more bullet lists (and mandatory parameters first and common optional parameters after). A hard part about using fetch is how to invoke it and continue processing the response after it's done. Based on the title of the section it would be out of scope but wanted to point out the gap there as well. There are not many good examples of using fetch in a nontrivial way where the result is 'awaited' by the algorithm, which then does something with the response.
There was a problem hiding this comment.
Thanks! I think @annevk and @domenic should decide how they want this section to direct feature authors to start discussions with them. Do they want to enable Github Discussions here, use Issues, or something else? Beyond that, all of my comments here are optional, and I don't expect you to wait for me once the editors approve the PR.
fetch.bs
Outdated
| <a>platform objects</a> (e.g. a <cite>Web IDL</cite> based API), which have a | ||
| <a>relevant settings object</a>. For example, a <a for=/>request</a> associated with a DOM | ||
| <a for=/>element</a> would set the <a for=/>request</a>'s <a for=request>client</a> to the element's | ||
| <a>node document</a>'s <a>relevant settings object</a>. [[WEBIDL]] |
There was a problem hiding this comment.
I might just have more contact with unusual APIs than most people, but I feel like "there's no obvious client" comes up fairly often. Could we say "If your request isn't associated with a particular environment settings object, please start a discussion with this repository's maintainers?" Ideally, we'd be able to link to a forum on which to start that discussion...
There was a problem hiding this comment.
Can you give examples outside of FedCM? Is this mostly about UA-initiated fetches that are not web-exposed in themselves?
There was a problem hiding this comment.
There was a problem hiding this comment.
I believe these are all web-exposed:
Seems like the fetch itself is not web exposed?
This is a JS API with an IDL interface, which has a relevant settings object.
Seems like this ends up also doing something with a JS observer, which has a relevant settings object
There was a problem hiding this comment.
For the first, I might be understanding something different by "web-exposed": the result doesn't get reported to Javascript running in a web page, but developers building websites will have their designs broken if it changes.
For the second, the relevant settings object's window may have closed by the time the fetch is sent, which means it can't use that as its client. Maybe the right answer is to start a Service Worker to exist while the fetch is running, so that can be the client, but the feature's designers need someone to tell them that, and help work through the tradeoffs.
For the third, the JS observer isn't really important, but there is a global available in the algorithm's caller, so the use of null is just a bug. But the developers thought they didn't have one, maybe because the algorithm originally ran after the originating client closed. They should have a way to ask what to do, so someone knowledgeable can find them the right client.
There was a problem hiding this comment.
Totally agree that documentation for this is lacking. Let's fix it step by step.
btw client is not mandatory. Instead of a client you can populate the request's origin and policy container, and respond to callbacks on a parallel queue (some anonymous new thread). For some of these features maybe that's the right approach and we should address this in this documentation.
|
@annevk want to have a go at reviewing this? |
|
Thanks @domenic, I addressed your notes but seems like builds are failing (CSSWG API is down?) |
Any other comments? |
This intends to be a summary of informative notes in the #Requests section, into a concise "starter" guide to populating a request, which is usually the tricky bit of using Fetch. Requested by authors of other specs, see for example: w3ctag/design-principles#238 w3c-fedid/FedCM#320
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Jeffrey Yasskin <[email protected]>
Co-authored-by: Domenic Denicola <[email protected]>
Co-authored-by: Domenic Denicola <[email protected]>
Co-authored-by: Domenic Denicola <[email protected]>
Co-authored-by: Domenic Denicola <[email protected]>
Co-authored-by: Domenic Denicola <[email protected]>
Co-authored-by: Domenic Denicola <[email protected]>
All fixed... Another review pass? |
|
Ping? @domenic |
|
``
No worries! @annevk would you merge please? (or have your own review pass if you want :)) |
This intends to be a summary of informative notes in the #Requests section, into a concise "starter" guide to populating a request, which is usually the tricky bit of using Fetch.
Requested by authors of other specs, see for example: w3ctag/design-principles#238 w3c-fedid/FedCM#320
Preview | Diff