Re: [whatwg/fetch] Editorial: Add prose about constructing a request (PR #1585)

@jyasskin approved this pull request.

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.

> @@ -1508,6 +1508,9 @@ these steps:
 
 <h4 id=requests>Requests</h4>
 
+<p class=note>This section documents how requests work in detail. To get started with populating a
+request, see <a href="fetch-elsewhere-request">populating a request</a>.

Unless the editors are intentionally avoiding the Bikeshed feature for this,

```suggestion
request, see [[#fetch-elsewhere-request]].
```

will check that the section exists, and automatically follow any title changes.

> @@ -8385,6 +8388,69 @@ correctly. This section aims to give some advice.
 
 <p class=XXX>This is a work in progress.
 
+<h3 id=fetch-elsewhere-request>Populating a request</h3>
+
+<p>The first step in <a for=/>fetching</a> is to create a <a for=/>request</a>, and populate its
+multiple associated parameters.

"multiple associated parameters" sounds scary. I think a request is formally a struct, even though we haven't updated its `<dfn>` yet, so maybe
```suggestion
<a for=struct>items</a>.
```

> @@ -8385,6 +8388,69 @@ correctly. This section aims to give some advice.
 
 <p class=XXX>This is a work in progress.
 
+<h3 id=fetch-elsewhere-request>Populating a request</h3>
+
+<p>The first step in <a for=/>fetching</a> is to create a <a for=/>request</a>, and populate its
+multiple associated parameters.
+
+<p>Start by setting the <a for=/>request</a>'s <a for=request>URL</a> and <a for=request>method</a>,
+as defined by HTTP. If your `<code>POST</code>` or `<code>PUT</code>` <a for=/>request</a> needs a
+body, you can assign or stream it to <a for=/>request</a>'s <a for=request>body</a>. [[HTTP]]
+
+<p>Set your <a for=/>request</a>'s <a for=request>client</a> to the
+<a>environment settings object</a> you're operating in. Web-exposed APIs usually operate on
+<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

```suggestion
<a>relevant settings object</a> you can use. For example, a <a for=/>request</a> associated with a DOM
```

> +<a>environment settings object</a> you're operating in. Web-exposed APIs usually operate on
+<a>platform objects</a> (e.g. a <cite>Web IDL</cite> based API), which have a

The "(e.g. ...)" seems out of place here. Would it work to just remove it, or I'm I just missing what you were trying to accomplish with it?

> @@ -8385,6 +8388,69 @@ correctly. This section aims to give some advice.
 
 <p class=XXX>This is a work in progress.
 
+<h3 id=fetch-elsewhere-request>Populating a request</h3>
+
+<p>The first step in <a for=/>fetching</a> is to create a <a for=/>request</a>, and populate its
+multiple associated parameters.
+
+<p>Start by setting the <a for=/>request</a>'s <a for=request>URL</a> and <a for=request>method</a>,
+as defined by HTTP. If your `<code>POST</code>` or `<code>PUT</code>` <a for=/>request</a> needs a
+body, you can assign or stream it to <a for=/>request</a>'s <a for=request>body</a>. [[HTTP]]
+
+<p>Set your <a for=/>request</a>'s <a for=request>client</a> to the
+<a>environment settings object</a> you're operating in. Web-exposed APIs usually operate on
+<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]]

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 <a>environment settings object</a>, 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...

> +
+<p>The first step in <a for=/>fetching</a> is to create a <a for=/>request</a>, and populate its
+multiple associated parameters.
+
+<p>Start by setting the <a for=/>request</a>'s <a for=request>URL</a> and <a for=request>method</a>,
+as defined by HTTP. If your `<code>POST</code>` or `<code>PUT</code>` <a for=/>request</a> needs a
+body, you can assign or stream it to <a for=/>request</a>'s <a for=request>body</a>. [[HTTP]]
+
+<p>Set your <a for=/>request</a>'s <a for=request>client</a> to the
+<a>environment settings object</a> you're operating in. Web-exposed APIs usually operate on
+<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]]
+
+<p>Choose your <a for=/>request</a>'s <a for=request>destination</a>.

```suggestion
<p>Choose your <a for=/>request</a>'s <a for=request>destination</a> using the guidance in the <a>destination table</a>.
```
To put the TL;DR right up front.

> +as defined by HTTP. If your `<code>POST</code>` or `<code>PUT</code>` <a for=/>request</a> needs a
+body, you can assign or stream it to <a for=/>request</a>'s <a for=request>body</a>. [[HTTP]]
+
+<p>Set your <a for=/>request</a>'s <a for=request>client</a> to the
+<a>environment settings object</a> you're operating in. Web-exposed APIs usually operate on
+<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]]
+
+<p>Choose your <a for=/>request</a>'s <a for=request>destination</a>.
+<a for=request>destinations</a> affect <cite>Content Security Policy</cite> and have other
+implications such as the `<code>sec-fetch-dest</code>` header, so they are much more than
+informative metadata. If a new feature requires a <a for=request>destination</a> that's not in the
+<a>destination table</a>, please file an issue in the
+<a href=https://github.com/whatwg/fetch>Fetch Github repository</a> to discuss. [[!CSP]]

```suggestion
<a href=https://github.com/whatwg/fetch/issues>Fetch Github repository</a> to discuss. [[!CSP]]
```

> +"<code>same-origin</code>". As a general rule, modern web-exposed features that fetch cross-origin
+resources should use "<code>cors</code>" and decide on the <a for=request>credentials mode</a> on a
+case by case basis. For example, font loading uses "<code>same-origin</code>" (credentials are only

I think:
```suggestion
"<code>same-origin</code>". Otherwise, new web-exposed features should almost always set their <a for=request>mode</a> to
"<code>cors</code>".

<p>Next, pick the <a for=request>credentials mode</a> depending on the details of your feature.
For example, font loading uses "<code>same-origin</code>" (credentials are only
```

With the original arrangement, I stumbled over whether font loading was setting *mode* or *credentials mode* to "`same-origin`".

Oh, and then move the "If your feature is not web-exposed" bit back up to the *mode* paragraph.

> +<p>Choose your <a for=/>request</a>'s <a for=request>destination</a>.
+<a for=request>destinations</a> affect <cite>Content Security Policy</cite> and have other
+implications such as the `<code>sec-fetch-dest</code>` header, so they are much more than
+informative metadata. If a new feature requires a <a for=request>destination</a> that's not in the
+<a>destination table</a>, please file an issue in the
+<a href=https://github.com/whatwg/fetch>Fetch Github repository</a> to discuss. [[!CSP]]
+
+<p>Think through the way you intend to handle cross-origin resources. Some features may only work in
+the <a>same origin</a>, in which case set your <a for=/>request</a> <a for=request>mode</a> to
+"<code>same-origin</code>". As a general rule, modern web-exposed features that fetch cross-origin
+resources should use "<code>cors</code>" and decide on the <a for=request>credentials mode</a> on a
+case by case basis. For example, font loading uses "<code>same-origin</code>" (credentials are only
+sent in same-origin requests), video posters use "<code>include</code>" (credentials are always
+sent), and many features make this configurable in the API. If your feature is not web-exposed, or
+you think there is another reason for it to fetch cross-origin resources without CORS, discuss this
+with your security team and ping contributors of the

To avoid implying that only employees of big companies can contribute to the web, maybe:
```suggestion
with a security team and ping contributors of the
```

This still assumes that everyone knows how to find a security team to talk to, which won't always be right...

> +informative metadata. If a new feature requires a <a for=request>destination</a> that's not in the
+<a>destination table</a>, please file an issue in the
+<a href=https://github.com/whatwg/fetch>Fetch Github repository</a> to discuss. [[!CSP]]
+
+<p>Think through the way you intend to handle cross-origin resources. Some features may only work in
+the <a>same origin</a>, in which case set your <a for=/>request</a> <a for=request>mode</a> to
+"<code>same-origin</code>". As a general rule, modern web-exposed features that fetch cross-origin
+resources should use "<code>cors</code>" and decide on the <a for=request>credentials mode</a> on a
+case by case basis. For example, font loading uses "<code>same-origin</code>" (credentials are only
+sent in same-origin requests), video posters use "<code>include</code>" (credentials are always
+sent), and many features make this configurable in the API. If your feature is not web-exposed, or
+you think there is another reason for it to fetch cross-origin resources without CORS, discuss this
+with your security team and ping contributors of the
+<a href=https://github.com/whatwg/fetch>Fetch Standard</a>.
+
+<p>Figure out if your fetch needs to be reported to <cite>Resource Timing</cite>, and with which

I wanted to suggest
```suggestion
<p>Figure out if your fetch needs to be reported to [[RESOURCE-TIMING inline]], and with which
```
to get a nice link to the spec, but it gives some ugly extra "Level 2" text as part of the full spec title. (https://github.com/tabatkins/bikeshed/issues/2441) Up to you whether you think it works better anyway.

> + <dd><p>Together with <cite>Content Security Policy</cite>, ensures that only elements that provide
+ this predetermined string can fetch this resource.

I think this is
```suggestion
 <dd><p>If the site's <cite>Content Security Policy</cite> specifies a <a grammar>nonce-source</a>
 for this <a for=/>request</a>'s <a for=request>destination</a>, any request without a
 [[CSP#match-nonce-to-source-list|matching]] value in this field will be blocked.
```

> +<p>Figure out if your fetch needs to be reported to <cite>Resource Timing</cite>, and with which
+<a for=request>initiator type</a>. By passing an <a for=request>initiator type</a> to the

```suggestion
<p>Figure out if your fetch needs to be reported to <cite>Resource Timing</cite>. If so, set
the <a for=/>request</a>'s <a for=request>initiator type</a> to the value you want to appear
in <code>{{PerformanceResourceTiming}}.{{PerformanceResourceTiming/initiatorType}}</code>.
By passing an <a for=request>initiator type</a> to the
```
(The choice of `{{}}` or `<a idl>` actually affects the styling here.)

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1585#pullrequestreview-1236362097
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1585/review/1236362097@github.com>