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

Dropping old IEs #162

Open
wants to merge 10 commits into
base: v3development
Choose a base branch
from
7 changes: 7 additions & 0 deletions MIGRATING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Migrating from v2 to v3

## JSON body
If you used the `json` option to pass request body, now you need to set `body` to the object you want to send, and `json: true`

## Old IEs
If you need to support IE8 or IE9, stay on v2.
25 changes: 5 additions & 20 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ For webpack, add a [resolve.alias](http://webpack.github.io/docs/configuration.h
}
```

Browser support: IE8+ and everything else.
Browser support: IE10+ and everything else.

IE8 and IE9 support was dropped as of v3.0.0 but you can still use v2 in IEs just fine. See v2 documentation for details.

## Example

Expand All @@ -46,7 +48,6 @@ xhr({

```js
type XhrOptions = String | {
useXDR: Boolean?,
sync: Boolean?,
uri: String,
url: String,
Expand All @@ -64,9 +65,7 @@ type XhrOptions = String | {
}
xhr := (XhrOptions, Callback<Response>) => Request
```
the returned object is either an [`XMLHttpRequest`][3] instance
or an [`XDomainRequest`][4] instance (if on IE8/IE9 &&
`options.useXDR` is set to `true`)
the returned object is an [`XMLHttpRequest`][3] instance

Your callback will be called once with the arguments
( [`Error`][5], `response` , `body` ) where the response is an object:
Expand All @@ -83,8 +82,6 @@ Your callback will be called once with the arguments
- `body`: HTTP response body - [`XMLHttpRequest.response`][6], [`XMLHttpRequest.responseText`][7] or
[`XMLHttpRequest.responseXML`][8] depending on the request type.
- `rawRequest`: Original [`XMLHttpRequest`][3] instance
or [`XDomainRequest`][4] instance (if on IE8/IE9 &&
`options.useXDR` is set to `true`)
- `headers`: A collection of headers where keys are header names converted to lowercase


Expand Down Expand Up @@ -130,14 +127,6 @@ xhr.del('/delete-me', { headers: { my: 'auth' } }, function (err, resp) {
Specify the method the [`XMLHttpRequest`][3] should be opened
with. Passed to [`XMLHttpRequest.open`][2]. Defaults to "GET"

### `options.useXDR`

Specify whether this is a cross origin (CORS) request for IE<10.
Switches IE to use [`XDomainRequest`][4] instead of `XMLHttpRequest`.
Ignored in other browsers.

Note that headers cannot be set on an XDomainRequest instance.

### `options.sync`

Specify whether this is a synchrounous request. Note that when
Expand Down Expand Up @@ -192,7 +181,7 @@ A function being called right before the `send` method of the `XMLHttpRequest` o

### `options.xhr`

Pass an `XMLHttpRequest` object (or something that acts like one) to use instead of constructing a new one using the `XMLHttpRequest` or `XDomainRequest` constructors. Useful for testing.
Pass an `XMLHttpRequest` object (or something that acts like one) to use instead of constructing a new one using the `XMLHttpRequest` constructor. Useful for testing.

### `options.qs`

Expand All @@ -216,8 +205,6 @@ For more, see [Query string support](#query-string-support).
`options.json:true` with `options.body` for convenience - then `xhr` will do the serialization and set content-type accordingly.
- Where's stream API? `.pipe()` etc.
- Not implemented. You can't reasonably have that in the browser.
- Why can't I send `"true"` as body by passing it as `options.json` anymore?
- Accepting `true` as a value was a bug. Despite what `JSON.stringify` does, the string `"true"` is not valid JSON. If you're sending booleans as JSON, please consider wrapping them in an object or array to save yourself from more trouble in the future. To bring back the old behavior, hardcode `options.json` to `true` and set `options.body` to your boolean value.
- How do I add an `onprogress` listener?
- use `beforeSend` function for non-standard things that are browser specific. In this case:
```js
Expand All @@ -242,7 +229,6 @@ or you can override the constructors used to create requests at the module level

```js
xhr.XMLHttpRequest = MockXMLHttpRequest
xhr.XDomainRequest = MockXDomainRequest
```

## Query string support
Expand Down Expand Up @@ -275,7 +261,6 @@ xhr.get('/foo', {
[1]: http://xhr.spec.whatwg.org/#the-send()-method
[2]: http://xhr.spec.whatwg.org/#the-open()-method
[3]: http://xhr.spec.whatwg.org/#interface-xmlhttprequest
[4]: http://msdn.microsoft.com/en-us/library/ie/cc288060(v=vs.85).aspx
[5]: http://es5.github.com/#x15.11
[6]: http://xhr.spec.whatwg.org/#the-response-attribute
[7]: http://xhr.spec.whatwg.org/#the-responsetext-attribute
Expand Down
68 changes: 16 additions & 52 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,16 @@ var xtend = require("xtend")

module.exports = createXHR
createXHR.XMLHttpRequest = window.XMLHttpRequest || noop
createXHR.XDomainRequest = "withCredentials" in (new createXHR.XMLHttpRequest()) ? createXHR.XMLHttpRequest : window.XDomainRequest
createXHR.qsSerialize = null // Define this as a function to support the `qs` option

forEachArray(["get", "put", "post", "patch", "head", "delete"], function(method) {
"get,put,post,patch,head,delete".split(",").map(function(method) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make a string and split instead of just creating an array? ["get","put","post","patch","head","delete"].map()

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It saves 6 characters :)
I'm generally avoiding using my old golfing tricks in real work, but this one seemed readable.

If it raises eyebrows, contrary to what I assumed, I sure have to change it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either are probably fine, although I am more used to seeing the array syntax.

If we want to prioritize file size to this extent, then I think there are other low-hanging fruit that would bring even bigger wins; i.e.; an "xhr.modern.js" file that would have zero polyfills (and just use Object.assign); that would save a bit more than 6 characters.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmeas an ES6 implementation would be generally a lot smaller, but then the only reason to use this instead of .fetch would be to get an xhr that you can abort. Not sure if that's enough value proposition to get any adoption.
Support for not-so-old browsers is the key here.

Anyway, xhr.modern or xhr.es6 is a good idea to introduce next, once we get v3 shipped.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@djake Seeing you both default to array syntax, I think I'll undo this golfing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@djake Seeing you both default to array syntax, I think I'll undo this golfing

👍

then the only reason to use this instead of .fetch would be to get an xhr that you can abort.

For the record, this is precisely why I use this lib. Promises provide a subset of functionality compared to regular XHR's, yet are simple to make from this callback-style API, so I'm surprised anyone chooses to use fetch!

createXHR[method === "delete" ? "del" : method] = function(uri, options, callback) {
options = initParams(uri, options, callback)
options.method = method.toUpperCase()
return _createXHR(options)
}
})

function forEachArray(array, iterator) {
for (var i = 0; i < array.length; i++) {
iterator(array[i])
}
}

function isEmpty(obj){
for(var i in obj){
if(obj.hasOwnProperty(i)) return false
}
return true
}

function initParams(uri, options, callback) {
var params = uri

Expand All @@ -39,6 +25,7 @@ function initParams(uri, options, callback) {
params = {uri:uri}
}
} else {
// xtend creates a shallow copy of options
params = xtend(options, {uri: uri})
}

Expand All @@ -53,7 +40,7 @@ function createXHR(uri, options, callback) {

function _createXHR(options) {
if(typeof options.callback === "undefined"){
throw new Error("callback argument missing")
throw Error("callback argument missing")
}

var called = false
Expand Down Expand Up @@ -92,7 +79,7 @@ function _createXHR(options) {
function errorFunc(evt) {
clearTimeout(timeoutTimer)
if(!(evt instanceof Error)){
evt = new Error("" + (evt || "Unknown XMLHttpRequest Error") )
evt = Error("" + (evt || "Unknown XMLHttpRequest Error") )
}
evt.statusCode = 0
return callback(evt, failureResponse)
Expand All @@ -103,12 +90,7 @@ function _createXHR(options) {
if (aborted) return
var status
clearTimeout(timeoutTimer)
if(options.useXDR && xhr.status===undefined) {
//IE8 CORS GET successful response doesn't have a status field, but body is fine
status = 200
} else {
status = (xhr.status === 1223 ? 204 : xhr.status)
}
status = xhr.status
var response = failureResponse
var err = null

Expand All @@ -121,29 +103,19 @@ function _createXHR(options) {
url: uri,
rawRequest: xhr
}
if(xhr.getAllResponseHeaders){ //remember xhr can in fact be XDR for CORS in IE
response.headers = parseHeaders(xhr.getAllResponseHeaders())
}
response.headers = parseHeaders(xhr.getAllResponseHeaders())
} else {
err = new Error("Internal XMLHttpRequest Error")
err = Error("Internal XMLHttpRequest Error")
}
return callback(err, response, response.body)
}

var xhr = options.xhr || null

if (!xhr) {
if (options.cors || options.useXDR) {
xhr = new createXHR.XDomainRequest()
}else{
xhr = new createXHR.XMLHttpRequest()
}
}
var xhr = options.xhr || (new createXHR.XMLHttpRequest())

var qsStringifyDefined = isFunction(createXHR.qsSerialize);

if (options.qs && !qsStringifyDefined) {
throw new Error("To use the 'qs' option, first define an 'xhr.qsSerialize' function.")
throw Error("To use the 'qs' option, first define an 'xhr.qsSerialize' function.")
}

var qs = options.qs && qsStringifyDefined ? '?' + createXHR.qsSerialize(options.qs) : ''
Expand All @@ -166,22 +138,18 @@ function _createXHR(options) {
rawRequest: xhr
}

if ("json" in options && options.json !== false) {
if (options.json) {
isJson = true
headers["accept"] || headers["Accept"] || (headers["Accept"] = "application/json") //Don't override existing accept header declared by user
if (method !== "GET" && method !== "HEAD") {
headers["content-type"] || headers["Content-Type"] || (headers["Content-Type"] = "application/json") //Don't override existing accept header declared by user
body = JSON.stringify(options.json === true ? body : options.json)
body = JSON.stringify(body)
}
}

xhr.onreadystatechange = readystatechange
xhr.onload = loadFunc
xhr.onerror = errorFunc
// IE9 must have onprogress be set to a unique function.
xhr.onprogress = function () {
// IE must die
}
xhr.onabort = function(){
aborted = true;
}
Expand All @@ -197,22 +165,18 @@ function _createXHR(options) {
if (!sync && options.timeout > 0 ) {
timeoutTimer = setTimeout(function(){
if (aborted) return
aborted = true//IE9 may still call readystatechange
aborted = true //browser may still call readystatechange
xhr.abort("timeout")
var e = new Error("XMLHttpRequest timeout")
var e = Error("XMLHttpRequest timeout")
e.code = "ETIMEDOUT"
errorFunc(e)
}, options.timeout )
}

if (xhr.setRequestHeader) {
for(key in headers){
if(headers.hasOwnProperty(key)){
xhr.setRequestHeader(key, headers[key])
}
for(key in headers){
if(headers.hasOwnProperty(key)){
xhr.setRequestHeader(key, headers[key])
}
} else if (options.headers && !isEmpty(options.headers)) {
throw new Error("Headers cannot be set on an XDomainRequest object")
}

if ("responseType" in options) {
Expand Down
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,14 @@
"email": "[email protected]"
},
"dependencies": {
"browserify": "^13.3.0",
"global": "~4.3.0",
"is-function": "^1.0.1",
"parse-headers": "^2.0.0",
"xtend": "^4.0.0"
},
"devDependencies": {
"browser-run": "3.5.0",
"browserify": "^13.1.1",
"for-each": "^0.3.2",
"browserify": "^13.3.0",
"pre-commit": "1.2.2",
"tap-spec": "^4.0.2",
"tape": "^4.0.0"
Expand Down
Loading