-
Notifications
You must be signed in to change notification settings - Fork 367
Add a textStream() method to the Body mixin #1862
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8032,6 +8032,7 @@ interface mixin Body { | |
[NewObject] Promise<FormData> formData(); | ||
[NewObject] Promise<any> json(); | ||
[NewObject] Promise<USVString> text(); | ||
[NewObject] ReadableStream textStream(); | ||
};</pre> | ||
|
||
<p class=note>Formats you would not want a network layer to be dependent upon, such as | ||
|
@@ -8073,6 +8074,9 @@ due course. | |
|
||
<dt><code><var>requestOrResponse</var> . <a method for=Body>text</a>()</code> | ||
<dd><p>Returns a promise fulfilled with <var>requestOrResponse</var>'s body as string. | ||
|
||
<dt><code><var>requestOrResponse</var> . <a method for=Body>textStream</a>()</code> | ||
<dd><p>Returns a {{ReadableStream}} with <var>requestOrResponse</var>'s body as string chunks. | ||
</dl> | ||
|
||
<hr> | ||
|
@@ -8246,6 +8250,20 @@ of running <a for=Body>consume body</a> with <a>this</a> and <a>parse JSON from | |
of running <a for=Body>consume body</a> with <a>this</a> and <a>UTF-8 decode</a>. | ||
</div> | ||
|
||
<div algorithm> | ||
<p>The <dfn method for=Body><code>textStream()</code></dfn> method steps are:</p> | ||
|
||
<ol> | ||
<li><p>If <span>this</span>'s <a for=Body>body</a> is null, throw a {{TypeError}}. | ||
|
||
<li><p>Let <var>stream</var> be <a>this</a>'s <a for=Body>body</a>'s <a for=body>stream</a>. | ||
|
||
<li><p>Let <var>decoder</var> be a new {{TextDecoderStream}} object. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and the next step would need improving if there's interest in this method. The constructor steps for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MattiasBuelens or @ricea might have thoughts, but ideally we don't use public API for internal algorithms. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably expose an algorithm to create a |
||
|
||
<li><p>Return the result of piping <var>stream</var> through <var>decoder</var>. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here "pipe through" would need to link to something. I'm not sure if there are other APIs that internally do something similar that I could copy? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should be able to use the pipe through algorithm exposed for other specifications. |
||
</ol> | ||
</div> | ||
|
||
|
||
<h3 id=request-class>Request class</h3> | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for the null case it'd be nicer to return an empty stream, similar to what
text()
does.Another thing that's not entirely clear to me is whether piping ends up making the underlying stream unusable so that you can't invoke
text()
aftertextStream()
for instance. (That's a property we want.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I didn't realize what
text()
does ifthis.body
is null. I agree an empty stream would be better, code that doesn't consider this case would likely just work.I thought this should behave like when you try to use both
blob()
andbytes()
, or indeed call one of those twice. That throws a TypeError in the first step of https://fetch.spec.whatwg.org/#concept-body-consume-body.Since it's not possible to call
text()
twice, should it really be possible to calltext()
aftertextStream()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not be possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, that should be easy to achieve, even though the current spec text doesn't make clear what happens.