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

Uploading: allow blobs, or just BufferSource? #53

Closed
domenic opened this issue Aug 12, 2015 · 13 comments
Closed

Uploading: allow blobs, or just BufferSource? #53

domenic opened this issue Aug 12, 2015 · 13 comments

Comments

@domenic
Copy link
Contributor

domenic commented Aug 12, 2015

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
Copy link

annevk commented Aug 13, 2015

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

@wanderview
Copy link

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
Copy link

annevk commented Aug 13, 2015

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

@wanderview
Copy link

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
Copy link
Contributor Author

domenic commented Aug 13, 2015

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
Copy link

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
Copy link
Contributor Author

domenic commented Aug 13, 2015

Agreed. I thought we were talking about uploading here.

@wanderview
Copy link

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
Copy link

annevk commented Aug 13, 2015

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
Copy link
Owner

+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
Copy link
Contributor Author

domenic commented Aug 19, 2015

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

@annevk
Copy link

annevk commented Aug 19, 2015

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

@domenic
Copy link
Contributor Author

domenic commented Aug 2, 2016

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

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

No branches or pull requests

4 participants