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

Request should allow WritableStream as body #378

Closed
jimmywarting opened this issue Aug 29, 2016 · 8 comments
Closed

Request should allow WritableStream as body #378

jimmywarting opened this issue Aug 29, 2016 · 8 comments

Comments

@jimmywarting
Copy link

jimmywarting commented Aug 29, 2016

Now we can get a readable stream from the response, which is cool! But i think it needs a complementary WriteableStream for the request as well. Imagine being able to post more then what can fit into the memory or were you simply just don't have everything you need to post.

let ws = new WritableStream
let encoder = new TextEncoder
let writer = ws.getWriter()

fetch('api/upload', {
    method: 'post',
    body: ws
})

let uint8array = encoder.encode('chunk')

writeStream.write(uint8array)
writeStream.close()

But i guess the WritableStream has to be implemented first ^^

@annevk
Copy link
Member

annevk commented Aug 29, 2016

Duplicate of #88. I think we might offer ReadableStream-based uploads first. It's a little unclear how WritableStream can work still.

@annevk annevk closed this as completed Aug 29, 2016
@domenic
Copy link
Member

domenic commented Aug 29, 2016

Yeah, we're actively working on this, although as you can see from #88 and other issues at https://github.com/yutakahirano/fetch-with-streams/issues, there are some complexities :-/.

Also, FYI writable streams are getting much closer to finalized :).

@jimmywarting
Copy link
Author

jimmywarting commented Oct 18, 2016

Emh, just why is WritableStream unclear? Feels weird to have a ReadableStream based upload When you are writing to a stream that are sending stuff.

I'm trying to think of this like node's http server request

http.createServer((req, res) => {
  // here you have both a readable (req) from which you read what has been written by the client
  // and a writeable (res) that you write back to the client
})

Should be similar but the other way around, You write to the server and read from the response

@domenic
Copy link
Member

domenic commented Oct 18, 2016

The problem essentially is that Requests, as specified in Fetch, are timeless objects that can be carted around and used. They aren't tied to a specific actual fetch() call (i.e. a specific actual request). This means that associating a writable stream with each Request gets strange, as you have to then hook up the writable stream to a specific "actual writable stream" when the fetch() happens.

The design we settled on was instead that you could have a Request with a body that is a readable stream, which gets carted around along with the timeless Request object. Then, fetch() takes care of doing an invisible request.body.pipeTo(writableStreamRepresentingThisFetch) behind the scenes, once we've figured out what that writableStreamRepresentingThisFetch is. But writableStreamRepresentingThisFetch is never exposed to the user, by the nature of the timeless-Request model.

@wanderview
Copy link
Member

And if you want to you can create pipe (identity transform, etc) to give yourself a WritableStream with a ReadableStream for the Response. This is explicitly opt'ing in to the buffering that this approach would require instead of the browser implicitly sticking a bunch of buffering in the path magically.

@jimmywarting
Copy link
Author

I think i would use the identity transform trick...

But it's true that in most cases you send non-streamable object and often you have everything before making a ajax call so those need to be readable so just passing a readable is okey i guess.

But for something like JSZip it would have been nice if the lib could just write directly to the writableStreamRepresentingThisFetch where you zip a bunch of stuff and uploads them.

@ioquatix
Copy link

ioquatix commented Oct 4, 2018

I may also comment on the other issue, but this is exactly the design I came up with in async-http: A Body::Writable is just a Body::Readable with an internal queue of chunks.

https://github.com/socketry/async-http/blob/master/lib/async/http/body/writable.rb

I think the design works well.

@an0ndev
Copy link

an0ndev commented May 25, 2020

Just as an aside, the feature is still broken. See here for the commit and associated discussion: #425

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

No branches or pull requests

6 participants