Add a .text() accessor to FetchResponse #2922
Conversation
…out converting it to JSON first. (I.e., extending FetchResponse to cover one additional method from the Response / Body API.)
| @@ -387,6 +387,15 @@ class FetchResponse { | |||
| } | |||
|
|
|||
| /** | |||
| * Drains the response and returns a promise that resolves with the response | |||
| * text. | |||
| * @returns {!Promise.<string>} | |||
There was a problem hiding this comment.
qq, is it @returns or @return? https://developers.google.com/closure/compiler/docs/js-for-compiler#tags says it should be @return
There was a problem hiding this comment.
Oops. My editor templates betrayed me. Fixed.
|
Please add a test. |
| */ | ||
| text() { | ||
| return this.drainText_(); | ||
| } |
There was a problem hiding this comment.
if we need drainText_ exposed, i think we should just make it public instead of this re-aliasing. but i don't mind this either way.
/cc @cramforce
There was a problem hiding this comment.
actually, looking through the full code again, this is probably better. (adding this new public text() and keeping the private drainText_ method)
There was a problem hiding this comment.
Yes, text is part of the fetch API.
|
PTAL |
| const response = new FetchResponse(mockXhr); | ||
| return response.text().then(result => { | ||
| expect(result).to.equal('this is some test text'); | ||
| expect(response.text, 'should throw').to.throw(Error); |
There was a problem hiding this comment.
This will throw for an unrelated reason, you'll need to #bind here.
There was a problem hiding this comment.
Oh, good catch. I didn't realize that. Thanks! Fixed.
Add .text() accessor to FetchResponse to allow retrieving text without converting it to JSON first. (I.e., extending FetchResponse to cover one additional method from the Response / Body API.)