From 7cbc15e7a2ec0da3c503d8715719f5406b775190 Mon Sep 17 00:00:00 2001 From: Isiah Meadows Date: Wed, 28 Nov 2018 20:10:46 -0500 Subject: [PATCH] Fix `m.request`/`m.jsonp` to not mutate arguments, simplify code (#2288) I basically recast it to remove 99% of the duplication. They're basically the same function mod how they fire their requests and append query parameters. --- docs/change-log.md | 1 + request/request.js | 205 +++++++++++++++++++----------------------- test-utils/xhrMock.js | 2 + 3 files changed, 97 insertions(+), 111 deletions(-) diff --git a/docs/change-log.md b/docs/change-log.md index 5ab2d88d..9f61c7cf 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -73,6 +73,7 @@ - render/core: avoid touching `Object.prototype.__proto__` setter with `key: "__proto__"` in certain situations ([#2251](https://github.com/MithrilJS/mithril.js/pull/2251)) - render/core: Vnodes stored in the dom node supplied to `m.render()` are now normalized [#2266](https://github.com/MithrilJS/mithril.js/pull/2266) - render/core: CSS vars can now be specified in `{style}` attributes [#2192](https://github.com/MithrilJS/mithril.js/pull/2192) +- request: don't modify params, call `extract`/`serialize`/`deserialize` with correct `this` value ([#2288](https://github.com/MithrilJS/mithril.js/pull/2288)) --- diff --git a/request/request.js b/request/request.js index d40f8d06..d5980332 100644 --- a/request/request.js +++ b/request/request.js @@ -2,85 +2,108 @@ var buildQueryString = require("../querystring/build") -var FILE_PROTOCOL_REGEX = new RegExp("^file://", "i") - module.exports = function($window, Promise) { var callbackCount = 0 - var oncompletion - function setCompletionCallback(callback) {oncompletion = callback} - function finalizer() { - var count = 0 - function complete() {if (--count === 0 && typeof oncompletion === "function") oncompletion()} - - return function finalize(promise) { - var then = promise.then - promise.then = function() { - count++ - var next = then.apply(promise, arguments) - next.then(complete, function(e) { - complete() - if (count === 0) throw e - }) - return finalize(next) + function makeRequest(factory) { + return function(url, args) { + if (typeof url !== "string") { args = url; url = url.url } + else if (args == null) args = {} + var promise = new Promise(function(resolve, reject) { + factory(url, args, function (data) { + if (typeof args.type === "function") { + if (Array.isArray(data)) { + for (var i = 0; i < data.length; i++) { + data[i] = new args.type(data[i]) + } + } + else data = new args.type(data) + } + resolve(data) + }, reject) + }) + if (args.background === true) return promise + var count = 0 + function complete() { + if (--count === 0 && typeof oncompletion === "function") oncompletion() + } + + return wrap(promise) + + function wrap(promise) { + var then = promise.then + promise.then = function() { + count++ + var next = then.apply(promise, arguments) + next.then(complete, function(e) { + complete() + if (count === 0) throw e + }) + return wrap(next) + } + return promise } - return promise } } - function normalize(args, extra) { - if (typeof args === "string") { - var url = args - args = extra || {} - if (args.url == null) args.url = url + + function hasHeader(args, name) { + for (var key in args.headers) { + if ({}.hasOwnProperty.call(args.headers, key) && name.test(key)) return true } - return args + return false } - function request(args, extra) { - var finalize = finalizer() - args = normalize(args, extra) + function interpolate(url, data, assemble) { + if (data == null) return url + url = url.replace(/:([^\/]+)/gi, function (m, key) { + return data[key] != null ? data[key] : m + }) + if (assemble && data != null) { + var querystring = buildQueryString(data) + if (querystring) url += (url.indexOf("?") < 0 ? "?" : "&") + querystring + } + return url + } - var promise = new Promise(function(resolve, reject) { - if (args.method == null) args.method = "GET" - args.method = args.method.toUpperCase() + return { + request: makeRequest(function(url, args, resolve, reject) { + var method = args.method != null ? args.method.toUpperCase() : "GET" + var useBody = method !== "GET" && method !== "TRACE" && + (typeof args.useBody !== "boolean" || args.useBody) - var useBody = (args.method === "GET" || args.method === "TRACE") ? false : (typeof args.useBody === "boolean" ? args.useBody : true) - - if (typeof args.serialize !== "function") args.serialize = typeof FormData !== "undefined" && args.data instanceof FormData ? function(value) {return value} : JSON.stringify - if (typeof args.deserialize !== "function") args.deserialize = deserialize - if (typeof args.extract !== "function") args.extract = extract - - args.url = interpolate(args.url, args.data) - if (useBody) args.data = args.serialize(args.data) - else args.url = assemble(args.url, args.data) + var data = args.data + var assumeJSON = (args.serialize == null || args.serialize === JSON.serialize) && !(data instanceof $window.FormData) + if (useBody) { + if (typeof args.serialize === "function") data = args.serialize(data) + else if (!(data instanceof $window.FormData)) data = JSON.stringify(data) + } var xhr = new $window.XMLHttpRequest(), aborted = false, _abort = xhr.abort - xhr.abort = function abort() { aborted = true _abort.call(xhr) } - xhr.open(args.method, args.url, typeof args.async === "boolean" ? args.async : true, typeof args.user === "string" ? args.user : undefined, typeof args.password === "string" ? args.password : undefined) + xhr.open(method, interpolate(url, args.data, !useBody), typeof args.async !== "boolean" || args.async, typeof args.user === "string" ? args.user : undefined, typeof args.password === "string" ? args.password : undefined) - if (args.serialize === JSON.stringify && useBody && !(args.headers && args.headers.hasOwnProperty("Content-Type"))) { + if (assumeJSON && useBody && !hasHeader(args, /^content-type$/i)) { xhr.setRequestHeader("Content-Type", "application/json; charset=utf-8") } - if (args.deserialize === deserialize && !(args.headers && args.headers.hasOwnProperty("Accept"))) { + if (typeof args.deserialize !== "function" && !hasHeader(args, /^accept$/i)) { xhr.setRequestHeader("Accept", "application/json, text/*") } if (args.withCredentials) xhr.withCredentials = args.withCredentials - if (args.timeout) xhr.timeout = args.timeout - if (args.responseType) xhr.responseType = args.responseType - for (var key in args.headers) if ({}.hasOwnProperty.call(args.headers, key)) { - xhr.setRequestHeader(key, args.headers[key]) + for (var key in args.headers) { + if ({}.hasOwnProperty.call(args.headers, key)) { + xhr.setRequestHeader(key, args.headers[key]) + } } if (typeof args.config === "function") xhr = args.config(xhr, args) || xhr @@ -91,10 +114,18 @@ module.exports = function($window, Promise) { if (xhr.readyState === 4) { try { - var response = (args.extract !== extract) ? args.extract(xhr, args) : args.deserialize(args.extract(xhr, args)) - if (args.extract !== extract || (xhr.status >= 200 && xhr.status < 300) || xhr.status === 304 || FILE_PROTOCOL_REGEX.test(args.url)) { - resolve(cast(args.type, response)) + var success = (xhr.status >= 200 && xhr.status < 300) || xhr.status === 304 || (/^file:\/\//i).test(url) + var response = xhr.responseText + if (typeof args.extract === "function") { + response = args.extract(xhr, args) + success = true + } else if (typeof args.deserialize === "function") { + response = args.deserialize(response) + } else { + try {response = response ? JSON.parse(response) : null} + catch (e) {throw new Error("Invalid JSON: " + response)} } + if (success) resolve(response) else { var error = new Error(xhr.responseText) error.code = xhr.status @@ -108,22 +139,15 @@ module.exports = function($window, Promise) { } } - if (useBody && (args.data != null)) xhr.send(args.data) + if (useBody && data != null) xhr.send(data) else xhr.send() - }) - return args.background === true ? promise : finalize(promise) - } - - function jsonp(args, extra) { - var finalize = finalizer() - args = normalize(args, extra) - - var promise = new Promise(function(resolve, reject) { + }), + jsonp: makeRequest(function(url, args, resolve, reject) { var callbackName = args.callbackName || "_mithril_" + Math.round(Math.random() * 1e16) + "_" + callbackCount++ var script = $window.document.createElement("script") $window[callbackName] = function(data) { script.parentNode.removeChild(script) - resolve(cast(args.type, data)) + resolve(data) delete $window[callbackName] } script.onerror = function() { @@ -131,55 +155,14 @@ module.exports = function($window, Promise) { reject(new Error("JSONP request failed")) delete $window[callbackName] } - if (args.data == null) args.data = {} - args.url = interpolate(args.url, args.data) - args.data[args.callbackKey || "callback"] = callbackName - script.src = assemble(args.url, args.data) + url = interpolate(url, args.data, true) + script.src = url + (url.indexOf("?") < 0 ? "?" : "&") + + encodeURIComponent(args.callbackKey || "callback") + "=" + + encodeURIComponent(callbackName) $window.document.documentElement.appendChild(script) - }) - return args.background === true? promise : finalize(promise) + }), + setCompletionCallback: function(callback) { + oncompletion = callback + }, } - - function interpolate(url, data) { - if (data == null) return url - - var tokens = url.match(/:[^\/]+/gi) || [] - for (var i = 0; i < tokens.length; i++) { - var key = tokens[i].slice(1) - if (data[key] != null) { - url = url.replace(tokens[i], data[key]) - } - } - return url - } - - function assemble(url, data) { - var querystring = buildQueryString(data) - if (querystring !== "") { - var prefix = url.indexOf("?") < 0 ? "?" : "&" - url += prefix + querystring - } - return url - } - - function deserialize(data) { - try {return data !== "" ? JSON.parse(data) : null} - catch (e) {throw new Error("Invalid JSON: " + data)} - } - - function extract(xhr) {return xhr.responseText} - - function cast(type, data) { - if (typeof type === "function") { - if (Array.isArray(data)) { - for (var i = 0; i < data.length; i++) { - data[i] = new type(data[i]) - } - } - else return new type(data) - } - return data - } - - return {request: request, jsonp: jsonp, setCompletionCallback: setCompletionCallback} } diff --git a/test-utils/xhrMock.js b/test-utils/xhrMock.js index 379bce28..71c2f5b7 100644 --- a/test-utils/xhrMock.js +++ b/test-utils/xhrMock.js @@ -11,7 +11,9 @@ module.exports = function() { return {status: 500, responseText: "server error, most likely the URL was not defined " + url} } + function FormData() {} var $window = { + FormData: FormData, XMLHttpRequest: function XMLHttpRequest() { var args = {} var headers = {}