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

Revise default content-type in $.fetch() #208

Open
friday opened this issue Nov 1, 2017 · 3 comments
Open

Revise default content-type in $.fetch() #208

friday opened this issue Nov 1, 2017 · 3 comments

Comments

@friday
Copy link
Contributor

friday commented Nov 1, 2017

The data argument in $.fetch() corresponds to the body argument in XMLHttpRequest.send(), which as per the MDN docs can be:

  • A Document
  • A BodyInit, which means either Blob (and hence also File), BufferSource, FormData, URLSearchParams, ReadableStream, or USVString

Depending on what the body argument is, the body needs to be serialized in different ways (which is handled by the browser), and the content-type header is needed for the recipient (likely a back-end server) to understand how to decode it (among other things). Browsers set content-type based on the body, so it should only be necessary to override that if the data is ambiguous or if the browser fails to detect the type for some other reason.

I only know of these cases where that could happen:

  • Document can be one of many types like "text/html" or "application/xml".
  • String will be assumed to be "text/plain". This is reasonable, since that's what it is. However, the more common use case seems to be pre-serialized data, such as urlencoded params ("application/x-www-form-urlencoded") or stringified json ("application/json").
  • ReadableStream is very new and not well supported yet. I haven't tested it. It seems it can be used for arbitrary streamable data, so browsers will likely not be able to detect the type?

The other types are more straightforward:

Type Format Content-type
Blob/File Binary mime-type
BufferSource Binary? mime-type?
FormData Multipart multipart/form-data; + boundary
URLSearchParams urlencoded params application/x-www-form-urlencoded

BufferSource is an ArrayBuffer or typed arrays providing an ArrayBufferView. Unless I've misunderstood this, this make it binary too.

The types that should set content-type "application/x-www-form-urlencoded" are URLSearchParams (always) and strings (sometimes). In other cases I think setting the default to "application/x-www-form-urlencoded" is counter-intuitive.

So, my suggestion is (similar to PR #203) to only set "content-type" if the data is text. But additionally also URLSearchParams. In the latter case it shouldn't be necessary, but perhaps it is. For example if URLSearchParams is supported by a polyfill, or if it is supported, but not by the browser implementation of XHR (in these cases we might also need to convert it to a string though).

Compatibility

This could be breaking some minor quirky use cases:

  • If servers have been modified to expect malformed request headers
  • With binary types, which in turn are texts with urlencoded data. However, in the case that's somehow beneficial/reasonable, I still think respecting the mime-type is preferable (new Blob(['key1=val1&key2=val2'], {type: 'application/x-www-form-urlencoded'}) would work for example).
  • ReadableStream could possibly be used for urlencoded data? But I don't think that's a reasonable default.
@friday
Copy link
Contributor Author

friday commented Nov 1, 2017

Sorry for my stubbornness @LeaVerou! I probably should have posted this before the PR to explain the motivation. I've been using axios before and this is more in line with how they handle it (though they also serialize object literals and couple this logic with setting content-type).

@LeaVerou
Copy link
Owner

LeaVerou commented Nov 3, 2017

I would be happy to merge a PR on this since it doesn't break (significant) existing cases!

@joyously
Copy link

joyously commented Nov 4, 2017

Be sure to fix #200 first.

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

3 participants