-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[fetch] Support streaming fetch requests. #25383
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
Conversation
1d93124 to
db936bb
Compare
|
Just adding a comment here to say I'd appreciate this change. I had just hacked up Fetch.js to also do this, and then found this PR while searching for issues (and this looks much better than my hack and slash). RE: overrideMimeType, it's effectively supported in that it doesn't matter - you don't need the XHR to reinterpret whatever mime type the server specified, you treat it as a big binary buffer regardless thanks to the readable stream. |
|
I've been meaning to come back to this. I was thinking of changing it so that the DOM fetch is only used when streaming is requested. That way I don't have to try and replicate all the old XHR behavior when streaming isn't enabled. |
fe93793 to
536b320
Compare
|
I've changed how it works from original PR, now when FETCH_STREAMING is enabled we'll only use the DOM fetch API when a streaming data is requested. This way we can worry less about fully polyfilling XHR and hopefully have less breakage. |
sbc100
left a comment
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.
Very nice to see this working out!
536b320 to
d84ebc7
Compare
Add a new `FETCH_BACKEND` setting that can either be 'xhr' or 'fetch' to use the corresponding DOM API. To keep the emscripten Fetch.js code the same I've implemented a polyfill of XMLHttpRequest using Fetch. To support streaming I wired up the fetch code to use the old onprogress code that old versions of Firefox supported. Most of the current API is supported with some notable exceptions: - synchronous requests - overriding the mime type I also changed a few of the tests to support both sync and async so I could test them with the fetch backend.
is only used for streaming requests otherwise it defaults to a regular XHR request.
e56a459 to
33d44c6
Compare
Add a new
FETCH_STREAMINGsetting that enables using the DOM fetch API to stream data when paired withEMSCRIPTEN_FETCH_STREAM_DATA. WhenEMSCRIPTEN_FETCH_STREAM_DATAis not used we will default to a regular XMLHttpRequest. To keep the emscripten Fetch.js code the same I've implemented a polyfill of XMLHttpRequest using Fetch. To support streaming I wired up the fetch code to use the old onprogress code that old versions of Firefox supported.Most of the current API is supported with some notable exceptions:
I also changed a few of the tests to support both sync and async so I could test them with the fetch backend.