Skip to content
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

Fetch request body as ReadableStream #622

Closed
ptrdom opened this issue Nov 1, 2021 · 4 comments · Fixed by #628
Closed

Fetch request body as ReadableStream #622

ptrdom opened this issue Nov 1, 2021 · 4 comments · Fixed by #628
Assignees
Milestone

Comments

@ptrdom
Copy link
Contributor

ptrdom commented Nov 1, 2021

According to the specs, Fetch should be able to handle ReadableStream as request body. Implementing this would likely result in some changes to ReadableStream and ReadableStreamController facades - ReadableStream needs a constructor that implements the underlyingSource, which allows interaction with ReadableStreamController, and ReadableStreamController should not have its own constructor, as it is constructed by ReadableStream during its construction.

If implementing this would be approved, I could give it a shot.

References:

@armanbilge
Copy link
Member

Thanks for this! 👍 to these changes, the goal is to match the spec so a contribution for this would be very welcome 😁

However, just so you are aware ... I recently implemented a Fetch-based client as well in http4s-dom and tried using a ReadableStream in the request body, but was dissapointed to learn that many browsers have not implemented this feature for requests. See:

https://caniuse.com/mdn-api_request_request_readablestream_request_body

@ptrdom
Copy link
Contributor Author

ptrdom commented Nov 1, 2021

I have just realized that also, while playing around with possible new version of the facade 😞. I guess it still makes sense to have the upgraded facade in case someone might find some use for natively available streaming implementation and to prepare for eventual browser support for ReadableStream in Fetch request body.

I will start working on the PR then.

@armanbilge
Copy link
Member

Thanks! The enhancements for ReadableStream in general are a good idea, and even though browsers have not implemented it, there are other environments that have, such as Cloudflare Workers (serverless):
https://developers.cloudflare.com/workers/runtime-apis/request#requestinit

@armanbilge armanbilge added this to the v2.1.0 milestone Nov 16, 2021
@armanbilge armanbilge linked a pull request Nov 17, 2021 that will close this issue
@armanbilge
Copy link
Member

Good news, it seems that Chrome will finally be supporting this:
https://developer.chrome.com/articles/fetch-streaming-requests/

Hopefully other browsers will follow suit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants