Re: [whatwg/fetch] Introduce "override fetch". (PR #1840)

@annevk commented on this pull request.

Thanks for writing this up. I'd like to double check internally this is the kind of flexibility we are okay with, but generally this looks good to me. Based on that I left a number of editorial comments. I didn't spot anything wrong.

Would @bvandersloot-mozilla be a good person to okay this for Mozilla?

> +<p>To <dfn id=concept-override-fetch>override fetch</dfn>, given a <var>type</var> (which is either
+"<code>scheme fetch</code>" or "<code>http fetch</code>", a <a for=/>fetch params</a>
+<var>fetchParams</var>, and an optional boolean <var>makeCORSPreflight</var> (default false):

```suggestion
<p>To <dfn id=concept-override-fetch>override fetch</dfn>, given "<code>scheme-fetch</code>" or
"<code>http-fetch</code>" <var>type</var>, <a for=/>fetch params</a> <var>fetchParams</var>, and an
optional boolean <var>makeCORSPreflight</var> (default false):
```
This would be more consistent with how we deal with types elsewhere. And since these appear like enum values we should follow enum naming conventions. (Note that this requires changes elsewhere.)

> + <li><p>Let <var>response</var> be the result of executing <a>Potentially override response for a
+ request</a> on <var>request</var>.

```suggestion
 <li><p>Let <var>response</var> be the result of executing
 <a>potentially override response for a request</a> on <var>request</var>.
```

> @@ -4989,6 +4992,96 @@ steps:
 </div>
 
 
+<h3 id=override-fetch>Override fetch</h3>
+
+<div algorithm>
+<p>To <dfn id=concept-override-fetch>override fetch</dfn>, given a <var>type</var> (which is either
+"<code>scheme fetch</code>" or "<code>http fetch</code>", a <a for=/>fetch params</a>
+<var>fetchParams</var>, and an optional boolean <var>makeCORSPreflight</var> (default false):
+
+<ol>
+ <li><p>Let <var>request</var> be <var>fetchParams</var>' <a for="fetch params">request</a>.
+
+ <li><p>Let <var>response</var> be the result of executing <a>Potentially override response for a
+ request</a> on <var>request</var>.
+
+ <li><p>If <var>response</var> is not null, return <var>response</var>.

```suggestion
 <li><p>If <var>response</var> is non-null, then return <var>response</var>.
```

> +<h3 id=override-fetch>Override fetch</h3>
+
+<div algorithm>
+<p>To <dfn id=concept-override-fetch>override fetch</dfn>, given a <var>type</var> (which is either
+"<code>scheme fetch</code>" or "<code>http fetch</code>", a <a for=/>fetch params</a>
+<var>fetchParams</var>, and an optional boolean <var>makeCORSPreflight</var> (default false):
+
+<ol>
+ <li><p>Let <var>request</var> be <var>fetchParams</var>' <a for="fetch params">request</a>.
+
+ <li><p>Let <var>response</var> be the result of executing <a>Potentially override response for a
+ request</a> on <var>request</var>.
+
+ <li><p>If <var>response</var> is not null, return <var>response</var>.
+
+ <li><p>Switch on <var>type</var> and run the associated step:

```suggestion
 <li>
  <p>Switch on <var>type</var> and run the associated step:
```

> +   <dd>
+    <p>Set <var>response</var> be the result of running <a>scheme fetch</a> given
+    <var>fetchParams</var>.

```suggestion
   <dd><p>Set <var>response</var> be the result of running <a>scheme fetch</a> given
   <var>fetchParams</var>.
```
Same below as `<p>` is the only child.

> +   <dd>
+    <p>Set <var>response</var> be the result of running <a>scheme fetch</a> given
+    <var>fetchParams</var>.
+
+   <dt>"<code>HTTP fetch</code>"
+   <dd>
+    <p>Set <var>response</var> be the result of running <a>HTTP fetch</a> given
+    <var>fetchParams</var> and <var>makeCORSPreflight</var>.
+  </dl>
+
+ <li><p>Return <var>response</var>.
+</ol>
+</div>
+
+<div algorithm>
+<p>The <dfn id=concept-potentially-override-response>potentially override response for a

No need for an ID.

> +   <li><p>If <var>request</var>'s <a for=request>current url</a>'s <a for=url>path</a> is
+   « "<code>widget.js</code>" », then:

```suggestion
   <li><p>If <var>request</var>'s <a for=request>current url</a>'s <a for=url>path</a> is
   « "<code>widget.js</code>" »:
```

> +       <dt><a for=response>type</a>
+       <dd>"<code>cors</code>"
+
+       <dt><a for=response>status</a>
+       <dd>200</dd>
+
+       <dt>...
+       <dd>...
+
+       <dt><a for=response>body</a>
+       <dd>The result of getting <var>body</var> <a>as a body</a>.
+      </dl>
+     </ol>
+
+    <li><p>Return a <a>network error</a>.
+   </ol>

```suggestion
   </ol>

```

> +       <dd>200</dd>
+
+       <dt>...
+       <dd>...
+
+       <dt><a for=response>body</a>
+       <dd>The result of getting <var>body</var> <a>as a body</a>.
+      </dl>
+     </ol>
+
+    <li><p>Return a <a>network error</a>.
+   </ol>
+  <li><p>Return null.
+ </ol>
+</div>
+</div>

```suggestion

</div>
```

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

Message ID: <whatwg/fetch/pull/1840/review/3051414298@github.com>