Why is the default type 'js'? #70

Closed
connor opened this Issue Feb 5, 2012 · 9 comments

Projects

None yet

7 participants

@connor
Collaborator
connor commented Feb 5, 2012

in setType (link), if the developer doesn't set a type when using reqwest, it defaults to js. I'm curious as to why it's that, instead of html or json?

The reason I ask is this tripped me up today and I kept getting an Unexpected token : error until I specified the type I wanted to return was json. Just curious why it's js in particular.

h5's.

@rvagg
Collaborator
rvagg commented Feb 5, 2012

I've been caught by that a few times myself but my guess would be that 'js' makes sense because in the early days of XHR we were mostly using it to return executable scripts, like the early version of Google Suggest, until we got clever with JSON et. al. (or maybe that was just me?).

I tend to always set 'Content-Type' (and often 'Accept') manually, perhaps we should sniff them if they exist to make a better guess at the type? We even have an explicit contentType option in there now too that could be inspected.

@chadselph

I actually think this to be a bug. The expected behavior would be for setType to consult the Content-Type of the response.

Furthermore, suppose you're returning a payload that isn't in one the authors of the library thought of (anything that isn't json, js, html, or xml). In this case, it will default to js, do an eval on it and fail!

The work around seems to be explicitly setting a type parameter in the request.

@enyo
enyo commented May 20, 2012

I agree that reqwest should consult the Content-type of the response!

@mrbanzai

I actually implemented the Content-type check in a fork, and was planning on submitting a PR as soon as I could get tests written for the use case.

@smcmurray

Even worse, any 'js' response is automatically eval-ed. This seems like a terrible default behavior even if the content is real js. If the content is not actually js, it leads to script syntax errors that can be hard to trace because they happen asynchronously.

@ded
Owner
ded commented Feb 24, 2014

fwiw, here's what jQuery docs do with $.ajax({dataType: ... })

(default: Intelligent Guess (xml, json, script, or html))

jQuery can convert a dataType from what it received in the Content-Type header to what you require. For example, if you want a text response to be treated as XML, use "text xml" for the dataType. You can also make a JSONP request, have it received as text, and interpreted by jQuery as XML: "jsonp text xml." Similarly, a shorthand string such as "jsonp xml" will first attempt to convert from jsonp to xml, and, failing that, convert from jsonp to text, and then from text to xml.

@ded
Owner
ded commented Feb 24, 2014

ok all, I'll have a change for this and create a minor version update. this basically removes our old setType function which makes a guess at the URL. Instead we will inspect request.getResponseHeader('Content-Type')

@ded ded self-assigned this Feb 24, 2014
@ded ded added a commit that closed this issue Feb 24, 2014
@ded infers type based on Content-Type header.
closes #70
2b751d2
@ded ded closed this in 2b751d2 Feb 24, 2014
@connor
Collaborator
connor commented Feb 24, 2014

😍

@smcmurray

thank you @ded. I suppose I should open a different issue for the automatic eval of js content.

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