fastmail / overture

Overture is a powerful JS library for building really slick web applications, with performance at, or surpassing, native apps.
MIT License
725 stars 25 forks source link

Support non-string XHR response with responseType #22

Closed chris-morgan closed 7 years ago

chris-morgan commented 7 years ago

(Note: per our discussions I have assumed a browser support baseline of IE 11, though we haven’t yet merged the WeakMap code which will actively break IE<11. For this particular feature, IE 10 will still work, but IE 9 won’t work at all any more. It would be a small patch to this to make IE<10 just use responseText because response won’t be there; that would take care of '' (text) and json', though 'arraybuffer', 'blob' and the rest still won’t work.)


Our browser support baseline all supports XMLHttpRequest.responseType, so let’s allow using it. (Most notably, so that you can get images as blobs rather than UTF-8 decoded strings, which makes for lots of U+FFFD REPLACEMENT CHARACTER substitutions.)

'json' as a responseType was added later and is lacking, notably, in all versions of IE (specifically IE 11 which we still support), but it’s useful and easy to polyfill so I’ve polyfilled it.

There is one specific recommended change that arises out of this: if you were using HttpRequest for JSON (e.g. JSON.parse( event.data ) in the io:success handler) you should set responseType to 'json' on the HttpRequest. Then event.data will be a JSON object, or null if it’s not valid JSON. (This implies that if you need to access the text in the case of its not being JSON you can’t use responseType.)

Review note on XHR#getResponse concerning the removal of error handling: the error handling was there because “Internet Explorer may throw an error if you try to read the responseText before it is in readyState 4.” I have confirmed that this is not the case in IE 11 for responseText or response (which is now being used instead).

neilj commented 7 years ago

Merged.