- From: Luca Casonato <notifications@github.com>
- Date: Mon, 26 Sep 2022 04:20:09 -0700
- To: whatwg/fetch <fetch@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/fetch/pull/1493/review/1120032568@github.com>
@lucacasonato approved this pull request. I agree with the motivation for this change. I do have a concern with this implementation though, namely that this effectively sets the default value for `duplex` to `"half"` (even if the user did not specify a value). This is incompatible with Deno, because our default value has always been `"full"` there (even though this is not explicitly exposed on a `.duplex` member on the `Request` object. I think we'll just have to swallow the "implementation will vary from spec" pill here though, as we implemented full duplex streaming before standardization. The user observable change here will be: ``` // Chrome const req = new Request("Hello World"); req.duplex; // "half" // Deno const req = new Request("Hello World"); req.duplex; // "full" ``` This is not much worse than the status quo though, as the actual behaviour would be different between Deno and other browsers regardless of this PR. This PR just explicitly exposes this difference between the default duplex streaming behaviours in a string value that the user can see. Also of note: this does not close the door to updating the default for `duplex` being `"full"` when a user specifies a `ReadableStream` body. LGTM -- Reply to this email directly or view it on GitHub: https://github.com/whatwg/fetch/pull/1493#pullrequestreview-1120032568 You are receiving this because you are subscribed to this thread. Message ID: <whatwg/fetch/pull/1493/review/1120032568@github.com>
Received on Monday, 26 September 2022 11:20:21 UTC