From e55f9f00422288e414f88414ce2650a93c28e0d2 Mon Sep 17 00:00:00 2001 From: Anders Riutta Date: Mon, 25 Jul 2016 15:10:52 -0700 Subject: [PATCH 1/7] Added ReadableByteStream/fetch for supporting browsers (Chrome). Related issue: #7 --- browser.js | 51 ++++++++++++++++++++++++++++++++++++++++------- fetch-wrapper.js | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ package.json | 4 ++-- requested.js | 4 ++-- test/index.js | 4 ++++ 5 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 fetch-wrapper.js diff --git a/browser.js b/browser.js index 0d84cc7..07f6d63 100644 --- a/browser.js +++ b/browser.js @@ -1,11 +1,27 @@ 'use strict'; -var Requested = require('./requested') +var FetchWrapper = require('./fetch-wrapper.js') + , Requested = require('./requested') , listeners = require('loads') , send = require('xhr-send') , hang = require('hang') , AXO = require('axo'); +/** + * Root reference for iframes. + * Taken from + * https://github.com/visionmedia/superagent/blob/83892f35fe15676a4567a0eb51eecd096939ad36/lib/client.js#L1 + */ +var root; +if (typeof window !== 'undefined') { // Browser window + root = window; +} else if (typeof self !== 'undefined') { // Web Worker + root = self; +} else { // Other environments + console.warn('Using browser-only version of requests in non-browser environment'); + root = this; +} + /** * RequestS(tream). * @@ -67,12 +83,13 @@ var Requests = module.exports = Requested.extend({ * @param {Object} options Passed in defaults. * @api private */ - open: function open() { + open: function open(options) { var what - , slice = true , requests = this , socket = requests.socket; + var slice = (requests.hasOwnProperty('slice')) ? requests.slice : true; + requests.on('stream', function stream(data) { if (!slice) { return requests.emit('data', data); @@ -179,6 +196,21 @@ var Requests = module.exports = Requested.extend({ } }); +/** + * Create a new FetchWrapper. + * + * @returns {FetchWrapper} + * @type {Object} requests + * @api private + */ +Requests.FETCH = function create(requests) { + // TODO we need to pass the requests object to FetchWrapper, + // and we need to set requests.slice to false. + // This seems kludgy because it's not parallel with Requests.XHR and Requests.AXO. + requests.slice = false; + return new FetchWrapper(requests); +}; + /** * Create a new XMLHttpRequest. * @@ -229,7 +261,11 @@ Requests.active = {}; * @type {String} * @public */ -Requests.method = !!Requests.XHR() ? 'XHR' : (!!Requests.AXO() ? 'AXO' : ''); +if (typeof root.ReadableByteStream === 'function') { + Requests.method = 'FETCH'; +} else { + Requests.method = !!Requests.XHR() ? 'XHR' : (!!Requests.AXO() ? 'AXO' : ''); +} /** * Boolean indicating @@ -240,13 +276,13 @@ Requests.method = !!Requests.XHR() ? 'XHR' : (!!Requests.AXO() ? 'AXO' : ''); Requests.supported = !!Requests.method; /** - * The different type of `responseType` parsers that are supported in this XHR + * The different types of `responseType` parsers that are supported in this XHR * implementation. * * @type {Object} * @public */ -Requests.type = 'XHR' === Requests.method ? (function detect() { +Requests.type = ('XHR' === Requests.method) ? (function detect() { var types = 'arraybuffer,blob,document,json,text,moz-blob,moz-chunked-text,moz-chunked-arraybuffer,ms-stream'.split(',') , supported = {} , type, xhr, prop; @@ -282,7 +318,7 @@ Requests.type = 'XHR' === Requests.method ? (function detect() { * @type {Boolean} * @private */ -Requests.streaming = 'XHR' === Requests.method && ( +Requests.streaming = ('XHR' === Requests.method) && ( 'multipart' in XMLHttpRequest.prototype || Requests.type.mozchunkedarraybuffer || Requests.type.mozchunkedtext @@ -296,6 +332,7 @@ Requests.streaming = 'XHR' === Requests.method && ( // // The solution is to completely clean up all active running requests. // +// TODO global vs. root? do we need both? if (global.attachEvent) global.attachEvent('onunload', function reap() { for (var id in Requests.active) { Requests.active[id].destroy(); diff --git a/fetch-wrapper.js b/fetch-wrapper.js new file mode 100644 index 0000000..cbac8c7 --- /dev/null +++ b/fetch-wrapper.js @@ -0,0 +1,52 @@ +// From https://github.com/jonnyreeves/chunked-request/blob/4dd6b7568e79a920f6cab3cd4d91d1e8d30b0798/src/impl/fetch.js + +var EventEmitter = require('eventemitter3'); + +class FetchWrapper extends EventEmitter { + constructor(requests) { + super(); + var fetchWrapper = this; + fetchWrapper.requests = requests; + + // TODO why doesn't this work? + //const {headers, mode, body, credentials} = requests; + var headers = requests.headers; + var mode = requests.mode; + var body = requests.body; + var credentials = requests.credentials; + + const decoder = new TextDecoder(); + fetchWrapper.decoder = decoder; + + fetchWrapper.fetchOptions = {headers, mode, body, credentials}; + } + + onError(err) { + var fetchWrapper = this; + fetchWrapper.requests.emit('error', err); + } + + pump(reader, res) { + var fetchWrapper = this; + return reader.read() + .then(result => { + if (result.done) { + fetchWrapper.requests.emit('end'); + // NOTE: when result.done = true, result.value will always be null + return; + } + fetchWrapper.requests.emit('stream', fetchWrapper.decoder.decode(result.value)); + return fetchWrapper.pump(reader, res); + }); + } + + // TODO is streaming the third arg? + open(method, url, streaming) { + var fetchWrapper = this; + fetch(url, fetchWrapper.fetchOptions) + .then(res => fetchWrapper.pump(res.body.getReader(), res)) + .catch(fetchWrapper.onError); + } +} + +module.exports = FetchWrapper; diff --git a/package.json b/package.json index 9435a20..269e85c 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "requests", "version": "0.1.7", - "description": "An streaming XHR abstraction that works in browsers and node.js", + "description": "A streaming XHR abstraction that works in browsers and node.js", "main": "index.js", "browser": "browser.js", "scripts": { @@ -47,7 +47,7 @@ }, "devDependencies": { "argh": "0.1.x", - "assume": "1.3.x", + "assume": "bigpipe/assume#67b166ff09d1f4199458f56bfbecbfc66e6f700a", "async-each": "0.1.x", "browserify": "11.x.x", "http-proxy": "1.11.x", diff --git a/requested.js b/requested.js index 7cf1b60..d626305 100644 --- a/requested.js +++ b/requested.js @@ -21,7 +21,7 @@ function Requested(url, options) { this.writable = false; if (this.initialize) this.initialize(url); - if (!this.manual && this.open) this.open(url); + if (!this.manual && this.open) this.open(options); } Requested.extend = require('extendible'); @@ -70,7 +70,7 @@ Requested.prototype.merge = function merge(target) { /** * The defaults for the Requests. These values will be used if no options object - * or matching key is provided. It can be override globally if needed but this + * or matching key is provided. It can be overriden globally if needed, but this * is not advised as it can have some potential side affects for other libraries * that use this module. * diff --git a/test/index.js b/test/index.js index fbe6854..4dfe840 100644 --- a/test/index.js +++ b/test/index.js @@ -102,6 +102,10 @@ kill.hooks = []; wd: argv.wd, ui: argv.ui }) + .on('error', function(err) { + console.error('saw an error'); + console.error(err); + }) .bundle(next); } ]); From b2ea41fe02cfb91fb69455387f613cf95adbae58 Mon Sep 17 00:00:00 2001 From: Anders Riutta Date: Mon, 25 Jul 2016 15:51:14 -0700 Subject: [PATCH 2/7] Use ES5. --- fetch-wrapper.js | 94 ++++++++++++++++++++++++------------------------ test/index.js | 5 +-- 2 files changed, 48 insertions(+), 51 deletions(-) diff --git a/fetch-wrapper.js b/fetch-wrapper.js index cbac8c7..5e1a926 100644 --- a/fetch-wrapper.js +++ b/fetch-wrapper.js @@ -1,52 +1,52 @@ // From https://github.com/jonnyreeves/chunked-request/blob/4dd6b7568e79a920f6cab3cd4d91d1e8d30b0798/src/impl/fetch.js -var EventEmitter = require('eventemitter3'); - -class FetchWrapper extends EventEmitter { - constructor(requests) { - super(); - var fetchWrapper = this; - fetchWrapper.requests = requests; - - // TODO why doesn't this work? - //const {headers, mode, body, credentials} = requests; - var headers = requests.headers; - var mode = requests.mode; - var body = requests.body; - var credentials = requests.credentials; - - const decoder = new TextDecoder(); - fetchWrapper.decoder = decoder; - - fetchWrapper.fetchOptions = {headers, mode, body, credentials}; - } - - onError(err) { - var fetchWrapper = this; - fetchWrapper.requests.emit('error', err); - } - - pump(reader, res) { - var fetchWrapper = this; - return reader.read() - .then(result => { - if (result.done) { - fetchWrapper.requests.emit('end'); - // NOTE: when result.done = true, result.value will always be null - return; - } - fetchWrapper.requests.emit('stream', fetchWrapper.decoder.decode(result.value)); - return fetchWrapper.pump(reader, res); - }); - } - - // TODO is streaming the third arg? - open(method, url, streaming) { - var fetchWrapper = this; - fetch(url, fetchWrapper.fetchOptions) - .then(res => fetchWrapper.pump(res.body.getReader(), res)) - .catch(fetchWrapper.onError); - } +function FetchWrapper(requests) { + var fetchWrapper = this; + fetchWrapper.requests = requests; + + var headers = requests.headers; + var mode = requests.mode; + var body = requests.body; + var credentials = requests.credentials; + + var decoder = new TextDecoder(); + fetchWrapper.decoder = decoder; + + fetchWrapper.fetchOptions = { + headers: headers, + mode: mode, + body: body, + credentials: credentials + }; +} + +FetchWrapper.prototype.onError = function(err) { + var fetchWrapper = this; + fetchWrapper.requests.emit('error', err); +} + +FetchWrapper.prototype.pump = function(reader, res) { + var fetchWrapper = this; + return reader.read() + .then(function(result) { + if (result.done) { + fetchWrapper.requests.emit('end'); + // NOTE: when result.done = true, result.value will always be null + return; + } + fetchWrapper.requests.emit('stream', fetchWrapper.decoder.decode(result.value)); + return fetchWrapper.pump(reader, res); + }); +} + +// TODO is the third arg supposed to be "streaming"? +FetchWrapper.prototype.open = function(method, url, streaming) { + var fetchWrapper = this; + fetch(url, fetchWrapper.fetchOptions) + .then(function(res) { + return fetchWrapper.pump(res.body.getReader(), res) + }) + .catch(fetchWrapper.onError); } module.exports = FetchWrapper; diff --git a/test/index.js b/test/index.js index 4dfe840..994a5a3 100644 --- a/test/index.js +++ b/test/index.js @@ -102,10 +102,7 @@ kill.hooks = []; wd: argv.wd, ui: argv.ui }) - .on('error', function(err) { - console.error('saw an error'); - console.error(err); - }) + .on('error', console.error) .bundle(next); } ]); From 32c8a3fe1ddf1e9bddef02f65502d09aa4066bd0 Mon Sep 17 00:00:00 2001 From: Anders Riutta Date: Mon, 25 Jul 2016 18:30:24 -0700 Subject: [PATCH 3/7] Update version of assume dep. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 269e85c..ed7e379 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ }, "devDependencies": { "argh": "0.1.x", - "assume": "bigpipe/assume#67b166ff09d1f4199458f56bfbecbfc66e6f700a", + "assume": "1.4.x", "async-each": "0.1.x", "browserify": "11.x.x", "http-proxy": "1.11.x", From 54f7538b0030e59324b51c6273d415e02fbd5fcd Mon Sep 17 00:00:00 2001 From: Anders Riutta Date: Mon, 25 Jul 2016 18:36:30 -0700 Subject: [PATCH 4/7] Describe Chrome as using ReadableByteStream --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 924a454..ede67a3 100644 --- a/README.md +++ b/README.md @@ -9,12 +9,13 @@ [cover]: https://img.shields.io/coveralls/unshiftio/requests/master.svg?style=flat-square [irc]: https://img.shields.io/badge/IRC-irc.freenode.net%23unshift-00a8ff.svg?style=flat-square -Requests is a small library that implements fully and true streaming XHR for +Requests is a small library that fully implements true streaming XHR for browsers that support these methods. It uses a variety of proprietary `responseType` properties to force a streaming connection, even for binary data. For browsers that don't support this we will simply fallback to a regular but **async** XHR 1/2 request or ActiveXObject in even older deprecated browsers. +- Chrome: `ReadableByteStream` - Internet Explorer >= 10: `ms-stream` - FireFox >= 9: `moz-chunked` - FireFox < 20: `multipart` From 49d2a241e316ac28f30dc57c73d1f96b3a7cfcef Mon Sep 17 00:00:00 2001 From: Anders Riutta Date: Mon, 25 Jul 2016 18:37:51 -0700 Subject: [PATCH 5/7] Make sure it's clear that Node is supported. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ede67a3..5da3b14 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ [irc]: https://img.shields.io/badge/IRC-irc.freenode.net%23unshift-00a8ff.svg?style=flat-square Requests is a small library that fully implements true streaming XHR for -browsers that support these methods. It uses a variety of proprietary +Node.js and browsers that support these methods. It uses a variety of proprietary `responseType` properties to force a streaming connection, even for binary data. For browsers that don't support this we will simply fallback to a regular but **async** XHR 1/2 request or ActiveXObject in even older deprecated browsers. From 07872596627da53e0675555f78168c35f4da3e8d Mon Sep 17 00:00:00 2001 From: Anders Riutta Date: Wed, 27 Jul 2016 19:16:09 -0700 Subject: [PATCH 6/7] Further improvements --- browser.js | 96 ++++++++++++++++++++++++++++++++++++------------ fetch-wrapper.js | 9 ++++- package.json | 1 + requested.js | 8 +++- 4 files changed, 87 insertions(+), 27 deletions(-) diff --git a/browser.js b/browser.js index 07f6d63..a028cb7 100644 --- a/browser.js +++ b/browser.js @@ -4,6 +4,7 @@ var FetchWrapper = require('./fetch-wrapper.js') , Requested = require('./requested') , listeners = require('loads') , send = require('xhr-send') + , sameOrigin = require('same-origin') , hang = require('hang') , AXO = require('axo'); @@ -35,7 +36,7 @@ if (typeof window !== 'undefined') { // Browser window * * @constructor * @param {String} url The URL we want to request. - * @param {Object} options Various of request options. + * @param {Object} options Various request options. * @api public */ var Requests = module.exports = Requested.extend({ @@ -80,10 +81,11 @@ var Requests = module.exports = Requested.extend({ /** * Initialize and start requesting the supplied resource. * - * @param {Object} options Passed in defaults. + * @param {String} url The URL we want to request. + * @param {Object} options Various request options. * @api private */ - open: function open(options) { + open: function open(url, options) { var what , requests = this , socket = requests.socket; @@ -111,15 +113,46 @@ var Requests = module.exports = Requested.extend({ }); if (this.timeout) { + // NOTE the "+" before this.timeout just ensures + // socket.timeout is a number. socket.timeout = +this.timeout; } - if ('cors' === this.mode.toLowerCase() && 'withCredentials' in socket) { - socket.withCredentials = true; + // Polyfilling XMLHttpRequest to accept fetch options + // see https://fetch.spec.whatwg.org/#cors-protocol-and-credentials and + // https://fetch.spec.whatwg.org/#concept-request-credentials-mode + // + // ...credentials mode, which is "omit", "same-origin", or "include". + // Unless stated otherwise, it is "omit". + // + // When request's mode is "navigate", its credentials mode is assumed to + // be "include" and fetch does not currently account for other values. + // If HTML changes here, this standard will need corresponding changes. + if ('withCredentials' in socket) { + var credentials = this.credentials; + if (credentials) { + credentials = credentials.toLowerCase(); + } else if (this.mode) { + var mode = this.mode.toLowerCase(); + if (mode === 'navigate') { + credentials = 'include'; + } + } + + if (credentials) { + if (credentials === 'include') { + socket.withCredentials = true; + } else { + var origin = root.location.origin || (root.location.protocol + '//' + root.location.host); + if (credentials === 'same-origin' && sameOrigin(origin, url)) { + socket.withCredentials = true; + } + } + } } // - // ActiveXObject will throw an `Type Mismatch` exception when setting the to + // ActiveXObject will throw a `Type Mismatch` exception when setting the to // an null-value and to be consistent with all XHR implementations we're going // to cast the value to a string. // @@ -133,15 +166,16 @@ var Requests = module.exports = Requested.extend({ // already eliminates duplicate headers. // for (what in this.headers) { - if (this.headers[what] !== undefined && this.socket.setRequestHeader) { - this.socket.setRequestHeader(what, this.headers[what] +''); + if (this.headers[what] !== undefined && socket.setRequestHeader) { + socket.setRequestHeader(what, this.headers[what] + ''); } } // // Set the correct responseType method. // - if (requests.streaming) { + // TODO how should fetch/ReadableByteStream be handled here? + if (requests.streaming && (requests.method !== 'FETCH')) { if (!this.body || 'string' === typeof this.body) { if ('multipart' in socket) { socket.multipart = true; @@ -159,17 +193,29 @@ var Requests = module.exports = Requested.extend({ } } + // Polyfill XMLHttpRequest to use the fetch headers API + if (!socket.response || !socket.response.headers) { + socket.response = socket.response || {}; + var responseHeaders = {}; + responseHeaders.get = socket.getResponseHeader; + socket.response.headers = responseHeaders; + } + listeners(socket, requests, requests.streaming); + requests.emit('before', socket); - send(socket, this.body, hang(function send(err) { - if (err) { - requests.emit('error', err); - requests.emit('end', err); - } + if (requests.method !== 'FETCH') { + send(socket, this.body, hang(function send(err) { + if (err) { + requests.emit('error', err); + requests.emit('end', err); + } - requests.emit('send'); - })); + // NOTE the send event for fetch is in fetch-wrapper.js + requests.emit('send'); + })); + } }, /** @@ -205,7 +251,6 @@ var Requests = module.exports = Requested.extend({ */ Requests.FETCH = function create(requests) { // TODO we need to pass the requests object to FetchWrapper, - // and we need to set requests.slice to false. // This seems kludgy because it's not parallel with Requests.XHR and Requests.AXO. requests.slice = false; return new FetchWrapper(requests); @@ -318,13 +363,16 @@ Requests.type = ('XHR' === Requests.method) ? (function detect() { * @type {Boolean} * @private */ -Requests.streaming = ('XHR' === Requests.method) && ( - 'multipart' in XMLHttpRequest.prototype - || Requests.type.mozchunkedarraybuffer - || Requests.type.mozchunkedtext - || Requests.type.msstream - || Requests.type.mozblob -); +Requests.streaming = (Requests.method === 'FETCH') || + ( + (Requests.method === 'XHR') && ( + 'multipart' in XMLHttpRequest.prototype || + Requests.type.mozchunkedarraybuffer || + Requests.type.mozchunkedtext || + Requests.type.msstream || + Requests.type.mozblob + ) + ); // // IE has a bug which causes IE10 to freeze when close WebPage during an XHR diff --git a/fetch-wrapper.js b/fetch-wrapper.js index 5e1a926..8071b1e 100644 --- a/fetch-wrapper.js +++ b/fetch-wrapper.js @@ -1,5 +1,6 @@ // From https://github.com/jonnyreeves/chunked-request/blob/4dd6b7568e79a920f6cab3cd4d91d1e8d30b0798/src/impl/fetch.js +//var Requested = require('./requested'); function FetchWrapper(requests) { var fetchWrapper = this; fetchWrapper.requests = requests; @@ -22,7 +23,9 @@ function FetchWrapper(requests) { FetchWrapper.prototype.onError = function(err) { var fetchWrapper = this; - fetchWrapper.requests.emit('error', err); + console.error(err.message); + console.error(err.stack); + //fetchWrapper.requests.emit('error', err); } FetchWrapper.prototype.pump = function(reader, res) { @@ -36,6 +39,8 @@ FetchWrapper.prototype.pump = function(reader, res) { } fetchWrapper.requests.emit('stream', fetchWrapper.decoder.decode(result.value)); return fetchWrapper.pump(reader, res); + }, function(err) { + fetchWrapper.requests.emit('error', err); }); } @@ -44,6 +49,8 @@ FetchWrapper.prototype.open = function(method, url, streaming) { var fetchWrapper = this; fetch(url, fetchWrapper.fetchOptions) .then(function(res) { + fetchWrapper.response = res; + fetchWrapper.requests.emit('send', fetchWrapper); return fetchWrapper.pump(res.body.getReader(), res) }) .catch(fetchWrapper.onError); diff --git a/package.json b/package.json index ed7e379..2f0d671 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "extendible": "0.1.x", "hang": "1.0.x", "loads": "0.0.x", + "same-origin": "^0.1.1", "xhr-send": "1.0.x" }, "devDependencies": { diff --git a/requested.js b/requested.js index d626305..b4ea081 100644 --- a/requested.js +++ b/requested.js @@ -21,7 +21,10 @@ function Requested(url, options) { this.writable = false; if (this.initialize) this.initialize(url); - if (!this.manual && this.open) this.open(options); + // TODO AR changed what is passed in to this.open(). + // But what was happening did not match the + // documentation for this.open anyway. + if (!this.manual && this.open) this.open(url, options); } Requested.extend = require('extendible'); @@ -81,7 +84,8 @@ Requested.defaults = { streaming: false, manual: false, method: 'GET', - mode: 'cors', + // A request has an associated mode, which is "same-origin", "cors", "no-cors", "navigate", + // or "websocket". Unless stated otherwise, it is "no-cors". headers: { // // We're forcing text/plain mode by default to ensure that regular From f82feae0b6f68aa8501546d047959d4062e4e624 Mon Sep 17 00:00:00 2001 From: Anders Riutta Date: Wed, 27 Jul 2016 19:59:26 -0700 Subject: [PATCH 7/7] Better integrate fetch into lib. --- browser.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/browser.js b/browser.js index a028cb7..54b1a8a 100644 --- a/browser.js +++ b/browser.js @@ -194,16 +194,24 @@ var Requests = module.exports = Requested.extend({ } // Polyfill XMLHttpRequest to use the fetch headers API - if (!socket.response || !socket.response.headers) { - socket.response = socket.response || {}; - var responseHeaders = {}; - responseHeaders.get = socket.getResponseHeader; - socket.response.headers = responseHeaders; + var fetchOrFake = {} + if (requests.method !== 'FETCH') { + // fetchOrFake is fake + fetchOrFake.response = { + headers: { + get: socket.getResponseHeader + } + }; + fetchOrFake.request = socket.request || {}; + // TODO this is just a start + } else { + // fetchOrFake is real fetch + fetchOrFake = socket; } listeners(socket, requests, requests.streaming); - requests.emit('before', socket); + requests.emit('before', fetchOrFake); if (requests.method !== 'FETCH') { send(socket, this.body, hang(function send(err) {