- From: Domenic Denicola <notifications@github.com>
- Date: Wed, 10 Feb 2021 12:30:33 -0800
- To: whatwg/fetch <fetch@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/fetch/pull/1165/review/587988874@github.com>
@domenic approved this pull request. Overall looks good! I'd like to work on a PR for HTML to use these in the fetching scripts section to see what that looks like, although I don't want to block too long. I'll check out the XHR PR as well... > Requests no longer have a synchronous flag. Blocking a thread is now up to the caller. This seems kind of inconvenient. What does it buy us? What would it look like? I guess I should go check out the XHR PR... Maybe you could work on a PR to HTML to update some of the synchronous fetches, to see what it looks like there? > -<p>To <dfn>queue a fetch task</dfn> on <a for=/>request</a> -<var>request</var> to <var>run an operation</var>, run these steps: + <dt><dfn for="fetch params">process request body</dfn> (default null) + <dt><dfn for="fetch params">process request end-of-body</dfn> (default null) + <dt><dfn for="fetch params">process response</dfn> (default null) + <dt><dfn for="fetch params">process response end-of-body</dfn> (default null) It'd be good to state what these algorithms take. Although probably saying it in the definition of "fetch" is more important than saying it in the struct. Streams does this using a separate sentence; see https://streams.spec.whatwg.org/#other-specs-rs-create > + <dt><dfn for="fetch params">process request body</dfn> (default null) + <dt><dfn for="fetch params">process request end-of-body</dfn> (default null) + <dt><dfn for="fetch params">process response</dfn> (default null) + <dt><dfn for="fetch params">process response end-of-body</dfn> (default null) + <dd>Null or an algorithm. + + <dt><dfn for="fetch params">global</dfn> (default null) + <dd>Null or a <a for=/>global object</a>. + + <dt><dfn for="fetch params">algorithm queue</dfn> (default null) + <dd>Null or a <a for=/>parallel queue</a>. +</dl> + +<p>To <dfn>queue a fetch task</dfn>, given an algorithm <var>algorithm</var>, null or a +<a for=/>global object</a> <var>global</var>, and null or a <a for=/>parallel queue</a> +<var>parallelQueue</var>, run these steps: It seems like _global_ and _parallelQueue_ are mutually exclusive. Instead maybe have a single _destination_ which is either a global object or a parallel queue? A bit unusual from a spec-language-as-a-programming language perspective, but I think it'd be nice for spec writers and readers. > - <p class="note no-backref">This step blocks until <var>bs</var> is fully transmitted. + <li><p>Let <var>algorithm</var> be this step: perform the <a>transmit-request-body loop</a> algorithm -> continueAlgorithm? > - <li><p>Set <var>request</var>'s <a for=request>body</a> to <var>body</var>. - </ol> + <li><p>If <var>request</var>'s <a for=request>client</a> is non-null, then set <var>global</var> to + <var>request</var>'s <a for=request>client</a>'s + <a for="environment settings object">global object</a>. Can we assert now that either global or algorithmQueue is non-null? > @@ -160,35 +160,40 @@ lt="authentication entry">authentication entries</a> (for HTTP authentication). <hr> -<p><a>Tasks</a> that are -<a lt="queue a task">queued</a> by this standard are annotated as one -of: +<p>A <dfn>fetch params</dfn> is a <a for=/>struct</a> used in <a for=/>fetching</a>. It has the Maybe expand on its use a little bit to explain that it's used internally in this standard? Based purely on the name, one might assume it's something callers of fetch need to know about. > + <dt><dfn for="fetch params">process request body</dfn> (default null) + <dt><dfn for="fetch params">process request end-of-body</dfn> (default null) + <dt><dfn for="fetch params">process response</dfn> (default null) + <dt><dfn for="fetch params">process response end-of-body</dfn> (default null) + <dd>Null or an algorithm. + + <dt><dfn for="fetch params">global</dfn> (default null) + <dd>Null or a <a for=/>global object</a>. + + <dt><dfn for="fetch params">algorithm queue</dfn> (default null) + <dd>Null or a <a for=/>parallel queue</a>. +</dl> + +<p>To <dfn>queue a fetch task</dfn>, given an algorithm <var>algorithm</var>, null or a +<a for=/>global object</a> <var>global</var>, and null or a <a for=/>parallel queue</a> +<var>parallelQueue</var>, run these steps: Hmm, I see that doesn't match your usage elsewhere, where you pass things which could be null (but the local algorithm doesn't know). Probably things are good as-is, although I wonder if instead you want to make the unification I suggest inside the fetch params struct? (I don't know if that works yet, I'll keep reading...) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/whatwg/fetch/pull/1165#pullrequestreview-587988874
Received on Wednesday, 10 February 2021 20:30:48 UTC