diff --git a/docs/change-log.md b/docs/change-log.md index c59ea84c..39348f54 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -115,6 +115,7 @@ - render: correct `contenteditable` check to also check for `contentEditable` property name ([#2450](https://github.com/MithrilJS/mithril.js/pull/2450) [@isiahmeadows](https://github.com/isiahmeadows)) - docs: clarify valid key usage ([#2452](https://github.com/MithrilJS/mithril.js/pull/2452) [@isiahmeadows](https://github.com/isiahmeadows)) - route: don't pollute globals ([#2453](https://github.com/MithrilJS/mithril.js/pull/2453) [@isiahmeadows](https://github.com/isiahmeadows)) +- request: track xhr replacements correctly ([#2455](https://github.com/MithrilJS/mithril.js/pull/2455) [@isiahmeadows](https://github.com/isiahmeadows)) --- diff --git a/docs/index.md b/docs/index.md index d1a98478..8589f517 100644 --- a/docs/index.md +++ b/docs/index.md @@ -18,7 +18,7 @@ It's small (< 10kb gzip), fast and provides routing and XHR utilities out of the
Download size
- Mithril (9.5kb) + Mithril (9.6kb)
Vue + Vue-Router + Vuex + fetch (40kb)
diff --git a/docs/request.md b/docs/request.md index 44d8d28b..4d454147 100644 --- a/docs/request.md +++ b/docs/request.md @@ -54,7 +54,7 @@ Argument | Type | Required | Descr `options.withCredentials` | `Boolean` | No | Whether to send cookies to 3rd party domains. Defaults to `false` `options.timeout` | `Number` | No | The amount of milliseconds a request can take before automatically being [terminated](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/timeout). Defaults to `undefined`. `options.responseType` | `String` | No | The expected [type](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType) of the response. Defaults to `""` if `extract` is defined, `"json"` if missing. If `responseType: "json"`, it internally performs `JSON.parse(responseText)`. -`options.config` | `xhr = Function(xhr)` | No | Exposes the underlying XMLHttpRequest object for low-level configuration. Defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function). +`options.config` | `xhr = Function(xhr)` | No | Exposes the underlying XMLHttpRequest object for low-level configuration and optional replacement (by returning a new XHR). `options.headers` | `Object` | No | Headers to append to the request before sending it (applied right before `options.config`). `options.type` | `any = Function(any)` | No | A constructor to be applied to each object in the response. Defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function). `options.serialize` | `string = Function(any)` | No | A serialization method to be applied to `body`. Defaults to `JSON.stringify`, or if `options.body` is an instance of [`FormData`](https://developer.mozilla.org/en/docs/Web/API/FormData), defaults to the [identity function](https://en.wikipedia.org/wiki/Identity_function) (i.e. `function(value) {return value}`). diff --git a/request/request.js b/request/request.js index b0faaaa7..48f52f39 100644 --- a/request/request.js +++ b/request/request.js @@ -79,13 +79,13 @@ module.exports = function($window, Promise) { var assumeJSON = (args.serialize == null || args.serialize === JSON.serialize) && !(body instanceof $window.FormData) var responseType = args.responseType || (typeof args.extract === "function" ? "" : "json") - var xhr = new $window.XMLHttpRequest(), - aborted = false, - _abort = xhr.abort + var xhr = new $window.XMLHttpRequest(), aborted = false + var original = xhr, replacedAbort + var abort = xhr.abort - xhr.abort = function abort() { + xhr.abort = function() { aborted = true - _abort.call(xhr) + abort.call(this) } xhr.open(method, url, args.async !== false, typeof args.user === "string" ? args.user : undefined, typeof args.password === "string" ? args.password : undefined) @@ -106,47 +106,45 @@ module.exports = function($window, Promise) { } } - if (typeof args.config === "function") xhr = args.config(xhr, args) || xhr - - xhr.onreadystatechange = function() { + xhr.onreadystatechange = function(ev) { // Don't throw errors on xhr.abort(). - if(aborted) return + if (aborted) return - if (xhr.readyState === 4) { + if (ev.target.readyState === 4) { try { - var success = (xhr.status >= 200 && xhr.status < 300) || xhr.status === 304 || (/^file:\/\//i).test(url) + var success = (ev.target.status >= 200 && ev.target.status < 300) || ev.target.status === 304 || (/^file:\/\//i).test(url) // When the response type isn't "" or "text", // `xhr.responseText` is the wrong thing to use. // Browsers do the right thing and throw here, and we // should honor that and do the right thing by // preferring `xhr.response` where possible/practical. - var response = xhr.response, message + var response = ev.target.response, message if (responseType === "json") { // For IE and Edge, which don't implement // `responseType: "json"`. - if (!xhr.responseType && typeof args.extract !== "function") response = JSON.parse(xhr.responseText) + if (!ev.target.responseType && typeof args.extract !== "function") response = JSON.parse(ev.target.responseText) } else if (!responseType || responseType === "text") { // Only use this default if it's text. If a parsed // document is needed on old IE and friends (all // unsupported), the user should use a custom // `config` instead. They're already using this at // their own risk. - if (response == null) response = xhr.responseText + if (response == null) response = ev.target.responseText } if (typeof args.extract === "function") { - response = args.extract(xhr, args) + response = args.extract(ev.target, args) success = true } else if (typeof args.deserialize === "function") { response = args.deserialize(response) } if (success) resolve(response) else { - try { message = xhr.responseText } + try { message = ev.target.responseText } catch (e) { message = response } var error = new Error(message) - error.code = xhr.status + error.code = ev.target.status error.response = response reject(error) } @@ -157,6 +155,19 @@ module.exports = function($window, Promise) { } } + if (typeof args.config === "function") { + xhr = args.config(xhr, args, url) || xhr + + // Propagate the `abort` to any replacement XHR as well. + if (xhr !== original) { + replacedAbort = xhr.abort + xhr.abort = function() { + aborted = true + replacedAbort.call(this) + } + } + } + if (body == null) xhr.send() else if (typeof args.serialize === "function") xhr.send(args.serialize(body)) else if (body instanceof $window.FormData) xhr.send(body) diff --git a/request/tests/test-request.js b/request/tests/test-request.js index 2cc6b5c8..969de68b 100644 --- a/request/tests/test-request.js +++ b/request/tests/test-request.js @@ -455,19 +455,15 @@ o.spec("request", function() { var failed = false var resolved = false function handleAbort(xhr) { - var onreadystatechange = xhr.onreadystatechange // probably not set yet - var testonreadystatechange = function() { - onreadystatechange.call(xhr) + var onreadystatechange = xhr.onreadystatechange + xhr.onreadystatechange = function() { + onreadystatechange.call(xhr, {target: xhr}) setTimeout(function() { // allow promises to (not) resolve first o(failed).equals(false) o(resolved).equals(false) done() }, 0) } - Object.defineProperty(xhr, "onreadystatechange", { - set: function(val) { onreadystatechange = val }, - get: function() { return testonreadystatechange } - }) xhr.abort() } request({method: "GET", url: "/item", config: handleAbort}).catch(function() { @@ -477,6 +473,40 @@ o.spec("request", function() { resolved = true }) }) + o("doesn't fail on replaced abort", function(done) { + mock.$defineRoutes({ + "GET /item": function() { + return {status: 200, responseText: JSON.stringify({a: 1})} + } + }) + + var failed = false + var resolved = false + var abortSpy = o.spy() + var replacement + function handleAbort(xhr) { + var onreadystatechange = xhr.onreadystatechange + xhr.onreadystatechange = function() { + onreadystatechange.call(xhr, {target: xhr}) + setTimeout(function() { // allow promises to (not) resolve first + o(failed).equals(false) + o(resolved).equals(false) + done() + }, 0) + } + return replacement = { + send: xhr.send.bind(xhr), + abort: abortSpy, + } + } + request({method: "GET", url: "/item", config: handleAbort}).then(function() { + resolved = true + }, function() { + failed = true + }) + replacement.abort() + o(abortSpy.callCount).equals(1) + }) o("doesn't fail on file:// status 0", function(done) { mock.$defineRoutes({ "GET /item": function() { @@ -521,18 +551,58 @@ o.spec("request", function() { } }) }) - /*o("data maintains after interpolate", function() { + o("params unmodified after interpolate", function() { mock.$defineRoutes({ - "PUT /items/:x": function() { - return {status: 200, responseText: ""} + "PUT /items/1": function() { + return {status: 200, responseText: "[]"} } }) - var data = {x: 1, y: 2} - var dataCopy = Object.assign({}, data); - request({method: "PUT", url: "/items/:x", data}) + var params = {x: 1, y: 2} + var p = request({method: "PUT", url: "/items/:x", params: params}) - o(data).deepEquals(dataCopy) - })*/ + o(params).deepEquals({x: 1, y: 2}) + + return p + }) + o("can return replacement from config", function() { + mock.$defineRoutes({ + "GET /a": function() { + return {status: 200, responseText: "[]"} + } + }) + var result + return request({ + url: "/a", + config: function(xhr) { + return result = { + send: o.spy(xhr.send.bind(xhr)), + } + }, + }) + .then(function () { + o(result.send.callCount).equals(1) + }) + }) + o("can abort from replacement", function() { + mock.$defineRoutes({ + "GET /a": function() { + return {status: 200, responseText: "[]"} + } + }) + var result + + request({ + url: "/a", + config: function(xhr) { + return result = { + send: o.spy(xhr.send.bind(xhr)), + abort: o.spy(), + } + }, + }) + + result.abort() + }) }) o.spec("failure", function() { o("rejects on server error", function(done) { diff --git a/test-utils/tests/test-xhrMock.js b/test-utils/tests/test-xhrMock.js index 8c64a308..410811b8 100644 --- a/test-utils/tests/test-xhrMock.js +++ b/test-utils/tests/test-xhrMock.js @@ -61,6 +61,23 @@ o.spec("xhrMock", function() { } xhr.send("a=b") }) + o("passes event to onreadystatechange", function(done) { + $window.$defineRoutes({ + "GET /item": function(request) { + o(request.url).equals("/item") + return {status: 200, responseText: "test"} + } + }) + var xhr = new $window.XMLHttpRequest() + xhr.open("GET", "/item") + xhr.onreadystatechange = function(ev) { + o(ev.target).equals(xhr) + if (xhr.readyState === 4) { + done() + } + } + xhr.send() + }) o("handles routing error", function(done) { var xhr = new $window.XMLHttpRequest() xhr.open("GET", "/nonexistent") diff --git a/test-utils/xhrMock.js b/test-utils/xhrMock.js index b4383218..55c74606 100644 --- a/test-utils/xhrMock.js +++ b/test-utils/xhrMock.js @@ -72,7 +72,7 @@ module.exports = function() { self.readyState = 4 if (args.async === true) { callAsync(function() { - if (typeof self.onreadystatechange === "function") self.onreadystatechange() + if (typeof self.onreadystatechange === "function") self.onreadystatechange({target: self}) }) } }