yutakahirano / fetch-with-streams

Fetch API integrated with Streams.
66 stars 8 forks source link

Uploading: allow blobs, or just BufferSource? #53

Closed domenic closed 7 years ago

domenic commented 8 years ago

Would a ReadableStream of blobs be acceptable as a body? Or would it have to be one of easily-accessible data? I don't see any reason in principle to disallow blobs.

annevk commented 8 years ago

At that point you might as well allow https://fetch.spec.whatwg.org/#bodyinit.

wanderview commented 8 years ago

IMO the ReadableStream chunks should be required to be BufferSource since this what Response.body provides from fetch(). We shouldn't force consumers of Response.body to have check the type on every chunk.

annevk commented 8 years ago

If we're not doing any conversion, I tend to agree. But that would argue for only Uint8Array?

wanderview commented 8 years ago

Uint8Array or ArrayBuffer would work for me. But yea, whatever we decided fetch() returns.

I guess the trick is... we can't really enforce this. If someone provides a ReadableStream as a body we can't validate that it produces Uint8Array for every chunk.

This is why I thought maybe it would be nice to be able to set a ReadableStream.chunkType that enforces that every chunk enqueued is of the right type. This could be optional for streams in general, but required for Response.

domenic commented 8 years ago

I guess the trick is... we can't really enforce this. If someone provides a ReadableStream as a body we can't validate that it produces Uint8Array for every chunk.

Sure we can. Every chunk we read from the stream, we have a choice as to what we do with it. Do we error? Do we convert? Etc.

This is why I thought maybe it would be nice to be able to set a ReadableStream.chunkType that enforces that every chunk enqueued is of the right type. This could be optional for streams in general, but required for Response.

This seems like it would require inventing a way of enforcing types in JavaScript, which is a whole research project :P.

wanderview commented 8 years ago

I guess the trick is... we can't really enforce this. If someone provides a ReadableStream as a body we can't validate that it produces Uint8Array for every chunk.

Sure we can. Every chunk we read from the stream, we have a choice as to what we do with it. Do we error? Do we convert? Etc.

Who is "we"? If the Response is passed to a DOM API, then sure we can codify how conversions happen.

If content script both creates the Response and consumes it, though, we can't really do anything unless we are going to proxy the stream internally.

domenic commented 8 years ago

Agreed. I thought we were talking about uploading here.

wanderview commented 8 years ago

Ok, I guess that addresses the enforcement question I had. Sorry for my confusion.

I still think we should require Uint8Array or BufferSource for consistency. I think Uint8Array would be most correct, but it seems a bit punitive to force people to convert from other ArrayBufferView types.

annevk commented 8 years ago

If we are doing any kind of conversion, I don't see why we would constrain ourselves to just BufferSource or ArrayBufferView... However, if we only sometimes do conversion, and not when you directly invoke json() on the Request/Response, or when you observe a Request in a service worker, or something similar, that would seem bad to me.

yutakahirano commented 8 years ago

+1 for not allowing blobs, at least for a while. I'm not sure if it is strongly wanted, and we can loose the restriction later if necessary.

domenic commented 8 years ago

OK, sounds good. Let's maybe just stick with Uint8Array in the initial upload draft?

annevk commented 8 years ago

Yes, let's stick with the same as what we output then.

domenic commented 7 years ago

Let's close this. We seem to have decided on Uint8Array only for uploads, at least for now.