From 6df755febc94c9b0849b146083ffae9dbd382c9c Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 5 Jan 2016 17:51:07 -0800 Subject: [PATCH] feat(ajax): add resultSelector and improve perf Restructures ajax to use an AjaxSubscriber. removes unnecessary onload error handler adds additional tests around errored resultSelector and createXHR calls adds tests around resultSelector adds AjaxRequest type improves typing throughout ajax code a bit uses named function trick to avoid closures --- spec/observables/dom/ajax-spec.js | 151 +++++++++++--- src/Rx.DOM.ts | 32 +-- src/add/observable/dom/ajax.ts | 2 +- src/observable/dom/ajax.ts | 331 +++++++++++++++++------------- 4 files changed, 340 insertions(+), 176 deletions(-) diff --git a/spec/observables/dom/ajax-spec.js b/spec/observables/dom/ajax-spec.js index c9d5989291..682605fa5c 100644 --- a/spec/observables/dom/ajax-spec.js +++ b/spec/observables/dom/ajax-spec.js @@ -2,7 +2,9 @@ var Rx = require('../../../dist/cjs/Rx.DOM'); var Observable = Rx.Observable; -function noop() { } +function noop() { + // nope. +} describe('Observable.ajax', function () { beforeEach(function () { @@ -13,34 +15,125 @@ describe('Observable.ajax', function () { jasmine.Ajax.uninstall(); }); + it('should have an optional resultSelector', function () { + var expected = 'avast ye swabs!'; + var result; + var complete = false; + + Rx.Observable + .ajax({ + url: '/flibbertyJibbet', + responseType: 'text', + resultSelector: function (res) { + return res.responseText; + } + }) + .subscribe(function(x) { + result = x; + }, function () { + throw 'should not have been called'; + }, function () { + complete = true; + }); + + expect(jasmine.Ajax.requests.mostRecent().url).toBe('/flibbertyJibbet'); + + jasmine.Ajax.requests.mostRecent().respondWith({ + 'status': 200, + 'contentType': 'application/json', + 'responseText': expected + }); + + expect(result).toBe(expected); + expect(complete).toBe(true); + }); + + it('should have error when resultSelector errors', function () { + var expected = 'avast ye swabs!'; + var error; + + Rx.Observable + .ajax({ + url: '/flibbertyJibbet', + responseType: 'text', + resultSelector: function (res) { + throw new Error('ha! ha! fooled you!'); + } + }) + .subscribe(function(x) { + throw 'should not next'; + }, function (err) { + error = err; + }, function () { + throw 'should not complete'; + }); + + expect(jasmine.Ajax.requests.mostRecent().url).toBe('/flibbertyJibbet'); + + jasmine.Ajax.requests.mostRecent().respondWith({ + 'status': 200, + 'contentType': 'application/json', + 'responseText': expected + }); + + expect(error).toEqual(new Error('ha! ha! fooled you!')); + }); + + it('should error if createXHR throws', function () { + var error; + + Rx.Observable + .ajax({ + url: '/flibbertyJibbet', + responseType: 'text', + createXHR: function () { + throw new Error('wokka wokka'); + } + }) + .subscribe(function(x) { + throw 'should not next'; + }, function (err) { + error = err; + }, function () { + throw 'should not complete'; + }); + + expect(error).toEqual(new Error('wokka wokka')); + }); + it('should succeed on 200', function () { var expected = { foo: 'bar' }; - var doneFn = jasmine.createSpy("success"); + var result; + var complete = false; Rx.Observable .ajax({ - url: '/flibbertyJibbet', upload: true, - emitType: 'json', responseType: 'json' + url: '/flibbertyJibbet', + responseType: 'text', }) - .subscribe(doneFn, function () { + .subscribe(function(x) { + result = x; + }, function () { throw 'should not have been called'; + }, function () { + complete = true; }); expect(jasmine.Ajax.requests.mostRecent().url).toBe('/flibbertyJibbet'); - expect(doneFn).not.toHaveBeenCalled(); jasmine.Ajax.requests.mostRecent().respondWith({ 'status': 200, - 'contentType': 'text/plain', + 'contentType': 'application/json', 'responseText': JSON.stringify(expected) }); - expect(doneFn).toHaveBeenCalledWith(expected); + expect(result.xhr).toBeDefined(); + expect(result.response).toBe(JSON.stringify({ foo: 'bar' })); + expect(complete).toBe(true); }); it('should fail on 404', function () { - var expected = JSON.stringify({ foo: 'bar' }); - var errorFn = jasmine.createSpy("success"); + var error; Rx.Observable .ajax({ @@ -49,25 +142,31 @@ describe('Observable.ajax', function () { return xhr.response || xhr.responseText; } }) - .subscribe(function () {}, errorFn, function () { - throw 'should not have been called'; + .subscribe(function (x) { + console.log(x); + throw 'should not next'; + }, function (x) { + error = x; + }, function () { + throw 'should not complete'; }); expect(jasmine.Ajax.requests.mostRecent().url).toBe('/flibbertyJibbet'); - expect(errorFn).not.toHaveBeenCalled(); jasmine.Ajax.requests.mostRecent().respondWith({ 'status': 404, 'contentType': 'text/plain', - 'responseText': expected + 'responseText': 'Wee! I am text!' }); - expect(errorFn).toHaveBeenCalledWith(expected); + expect(error instanceof Rx.AjaxError).toBe(true); + expect(error.message).toBe('ajax error 404'); + expect(error.status).toBe(404); }); - it('should fail on 300', function () { - var expected = JSON.stringify({ foo: 'bar' }); - var errorFn = jasmine.createSpy("success"); + + it('should fail on 404', function () { + var error; Rx.Observable .ajax({ @@ -76,20 +175,26 @@ describe('Observable.ajax', function () { return xhr.response || xhr.responseText; } }) - .subscribe(function () {}, errorFn, function () { - throw 'should not have been called'; + .subscribe(function (x) { + console.log(x); + throw 'should not next'; + }, function (x) { + error = x; + }, function () { + throw 'should not complete'; }); expect(jasmine.Ajax.requests.mostRecent().url).toBe('/flibbertyJibbet'); - expect(errorFn).not.toHaveBeenCalled(); jasmine.Ajax.requests.mostRecent().respondWith({ 'status': 300, 'contentType': 'text/plain', - 'responseText': expected + 'responseText': 'Wee! I am text!' }); - expect(errorFn).toHaveBeenCalledWith(expected); + expect(error instanceof Rx.AjaxError).toBe(true); + expect(error.message).toBe('ajax error 300'); + expect(error.status).toBe(300); }); it('should succeed no settings', function () { diff --git a/src/Rx.DOM.ts b/src/Rx.DOM.ts index 23bb8e2a43..15fd64953a 100644 --- a/src/Rx.DOM.ts +++ b/src/Rx.DOM.ts @@ -128,6 +128,7 @@ import {AsapScheduler} from './scheduler/AsapScheduler'; import {QueueScheduler} from './scheduler/QueueScheduler'; import {AnimationFrameScheduler} from './scheduler/AnimationFrameScheduler'; import {rxSubscriber} from './symbol/rxSubscriber'; +import {AjaxRequest, AjaxResponse, AjaxError, AjaxTimeoutError} from './observable/dom/ajax'; /* tslint:enable:no-unused-variable */ /* tslint:disable:no-var-keyword */ @@ -143,18 +144,21 @@ var Symbol = { /* tslint:enable:no-var-keyword */ export { - Subject, - Scheduler, - Observable, - Subscriber, - Subscription, - Symbol, - AsyncSubject, - ReplaySubject, - BehaviorSubject, - ConnectableObservable, - Notification, - EmptyError, - ArgumentOutOfRangeError, - ObjectUnsubscribedError + AjaxResponse, + AjaxError, + AjaxTimeoutError, + Subject, + Scheduler, + Observable, + Subscriber, + Subscription, + Symbol, + AsyncSubject, + ReplaySubject, + BehaviorSubject, + ConnectableObservable, + Notification, + EmptyError, + ArgumentOutOfRangeError, + ObjectUnsubscribedError }; diff --git a/src/add/observable/dom/ajax.ts b/src/add/observable/dom/ajax.ts index 0cabfdde3b..ba6bc0d6de 100644 --- a/src/add/observable/dom/ajax.ts +++ b/src/add/observable/dom/ajax.ts @@ -1,5 +1,5 @@ import {Observable} from '../../../Observable'; -import {AjaxObservable} from '../../../observable/dom/ajax'; +import { AjaxObservable } from '../../../observable/dom/ajax'; Observable.ajax = AjaxObservable.create; export var _void: void; diff --git a/src/observable/dom/ajax.ts b/src/observable/dom/ajax.ts index 462fa10fe3..7a604ae32a 100644 --- a/src/observable/dom/ajax.ts +++ b/src/observable/dom/ajax.ts @@ -5,7 +5,7 @@ import {Observable} from '../../Observable'; import {Subscriber} from '../../Subscriber'; import {Subscription} from '../../Subscription'; -interface AjaxSettings { +export interface AjaxRequest { url?: string; body?: any; user?: string; @@ -14,24 +14,38 @@ interface AjaxSettings { headers?: Object; timeout?: number; password?: string; - emitType?: string; hasContent?: boolean; - responseType?: string; crossDomain?: boolean; createXHR?: () => XMLHttpRequest; - normalizeError?: (e: any, xhr: any, type: any) => any; - normalizeSuccess?: (e: any, xhr: any, settings: any) => any; progressSubscriber?: Subscriber; + resultSelector?: (response: AjaxResponse) => T; + responseType?: string; } +const createXHRDefault = (): XMLHttpRequest => { + let xhr = new root.XMLHttpRequest(); + if (this.crossDomain) { + if ('withCredentials' in xhr) { + xhr.withCredentials = true; + return xhr; + } else if (!!root.XDomainRequest) { + return new root.XDomainRequest(); + } else { + throw new Error('CORS is not supported by your browser'); + } + } else { + return xhr; + } +}; + /** - * Creates an observable for an Ajax request with either a settings object with url, headers, etc or a string for a URL. + * Creates an observable for an Ajax request with either a request object with url, headers, etc or a string for a URL. * * @example * source = Rx.Observable.ajax('/products'); * source = Rx.Observable.ajax( url: 'products', method: 'GET' }); * - * @param {Object} settings Can be one of the following: + * @param {Object} request Can be one of the following: * * A string of the URL to make the Ajax call. * An object with the following properties @@ -41,72 +55,92 @@ interface AjaxSettings { * - async: Whether the request is async * - headers: Optional headers * - crossDomain: true if a cross domain request, else false - * + * - createXHR: a function to override if you need to use an alternate XMLHttpRequest implementation. + * - resultSelector: a function to use to alter the output value type of the Observable. Gets {AjaxResponse} as an argument * @returns {Observable} An observable sequence containing the XMLHttpRequest. */ export class AjaxObservable extends Observable { - static create(options: string | any): Observable { return new AjaxObservable(options); } - private settings: AjaxSettings; + private request: AjaxRequest; constructor(options: string | any) { super(); - const settings: AjaxSettings = { - method: 'GET', - crossDomain: false, + const request: AjaxRequest = { async: true, + createXHR: createXHRDefault, + crossDomain: false, headers: {}, - emitType: 'text', - responseType: 'text', - timeout: 0, - createXHR: function() { - return this.crossDomain ? getCORSRequest() : new root.XMLHttpRequest(); - }, - normalizeError: normalizeAjaxErrorEvent, - normalizeSuccess: normalizeAjaxSuccessEvent + method: 'GET', + responseType: 'json', + timeout: 0 }; if (typeof options === 'string') { - settings.url = options; + request.url = options; } else { for (const prop in options) { if (options.hasOwnProperty(prop)) { - settings[prop] = options[prop]; + request[prop] = options[prop]; } } } - if (!settings.crossDomain && !settings.headers['X-Requested-With']) { - settings.headers['X-Requested-With'] = 'XMLHttpRequest'; + if (!request.crossDomain && !request.headers['X-Requested-With']) { + request.headers['X-Requested-With'] = 'XMLHttpRequest'; } - settings.hasContent = settings.body !== undefined; + request.hasContent = request.body !== undefined; - this.settings = settings; + this.request = request; } _subscribe(subscriber: Subscriber): Subscription | Function | void { + return new AjaxSubscriber(subscriber, this.request); + } +} - let done = false; - const {settings} = this; - const { - createXHR, user, password, timeout, method, - url, async, headers, hasContent, body, emitType, - normalizeError, normalizeSuccess, progressSubscriber - } = settings; +export class AjaxSubscriber extends Subscriber { + xhr: XMLHttpRequest; + resultSelector: (response: AjaxResponse) => T; + done: boolean = false; + + constructor(destination: Subscriber, public request: AjaxRequest) { + super(destination); + this.resultSelector = request.resultSelector; + this.xhr = this.createXHR(); + if (this.xhr) { + this.send(); + } + } - let result: any = tryCatch(createXHR).call(settings); + next(e: Event): void { + this.done = true; + const { resultSelector, xhr, request, destination } = this; + const response = new AjaxResponse(e, xhr, request); - if (result === errorObject) { - return subscriber.error(errorObject.e); + if (resultSelector) { + const result = tryCatch(resultSelector)(response); + if (result === errorObject) { + this.error(errorObject.e); + } else { + destination.next(result); + } + } else { + destination.next(response); } + } - const xhr: XMLHttpRequest = ( result); + private send() { + const { + request: { user, method, url, async, password }, + xhr + } = this; + let result; if (user) { result = tryCatch(xhr.open).call(xhr, method, url, async, user, password); } else { @@ -114,133 +148,154 @@ export class AjaxObservable extends Observable { } if (result === errorObject) { - return subscriber.error(errorObject.e); - } - - for (const header in headers) { - if (headers.hasOwnProperty(header)) { - xhr.setRequestHeader(header, headers[header]); - } + return this.error(errorObject.e); } + } - xhr.timeout = timeout; - xhr.ontimeout = onTimeout; + private createXHR(): XMLHttpRequest { + const request = this.request; + const createXHR = request.createXHR; + const xhr = tryCatch(createXHR).call(request); - if (!xhr.upload || ('withCredentials' in xhr) || !root.XDomainRequest) { - xhr.onreadystatechange = onReadyStateChange; + if (xhr === errorObject) { + this.error(errorObject.e); } else { - xhr.onload = onLoad; - if (progressSubscriber) { - xhr.onprogress = onProgress; - } - xhr.onerror = onError; - } - - const contentType = headers['Content-Type'] || - headers['Content-type'] || - headers['content-type']; - - if (hasContent && - (contentType === 'application/x-www-form-urlencoded') && - (typeof body !== 'string')) { - const newBody = []; - for (const prop in body) { - if (body.hasOwnProperty(prop)) { - newBody.push(`${prop}=${body[prop]}`); - } - } - settings.body = newBody.join('&'); - } - - result = tryCatch(xhr.send).call(xhr, hasContent && settings.body || null); - - if (result === errorObject) { - return subscriber.error(errorObject.e); + xhr.timeout = request.timeout; + xhr.responseType = request.responseType; + this.setupEvents(xhr, request); + return xhr; } + } - return new Subscription(() => { - if (!done && xhr.readyState !== 4) { - xhr.abort(); - } - }); + private setupEvents(xhr: XMLHttpRequest, request: AjaxRequest) { + const progressSubscriber = request.progressSubscriber; - function onTimeout(e) { + xhr.ontimeout = function xhrTimeout(e) { + const {subscriber, progressSubscriber, request } = (xhrTimeout); if (progressSubscriber) { progressSubscriber.error(e); } - subscriber.error(normalizeError(e, xhr, 'timeout')); - } - - function onLoad(e) { - if (progressSubscriber) { - progressSubscriber.next(e); - progressSubscriber.complete(); - } - processResponse(xhr, e); - } - - function onProgress(e) { - progressSubscriber.next(e); - } + subscriber.error(new AjaxTimeoutError(this, request)); //TODO: Make betterer. + }; + (xhr.ontimeout).request = request; + (xhr.ontimeout).subscriber = this; + (xhr.ontimeout).progressSubscriber = progressSubscriber; - function onError(e) { - done = true; + if (xhr.upload && 'withCredentials' in xhr && root.XDomainRequest) { if (progressSubscriber) { - progressSubscriber.error(e); + xhr.onprogress = function xhrProgress(e) { + const { progressSubscriber } = (xhrProgress); + progressSubscriber.next(e); + }; + (xhr.onprogress).progressSubscriber = progressSubscriber; } - subscriber.error(normalizeError(e, xhr, 'error')); - } - function onReadyStateChange(e) { - if (xhr.readyState === 4) { - processResponse(xhr, e); - } + xhr.onerror = function xhrError(e) { + const { progressSubscriber, subscriber, request } = (xhrError); + if (progressSubscriber) { + progressSubscriber.error(e); + } + subscriber.error(new AjaxError('ajax error', this, request)); + }; + (xhr.onerror).request = request; + (xhr.onerror).subscriber = this; + (xhr.onerror).progressSubscriber = progressSubscriber; } - function processResponse(xhr, e) { - done = true; - const status: any = xhr.status === 1223 ? 204 : xhr.status; - if ((status >= 200 && status < 300) || (status === 0) || (status === '')) { - if (emitType === 'json') { - subscriber.next(normalizeSuccess(e, xhr, settings).response); - } else { - subscriber.next(normalizeSuccess(e, xhr, settings)); + xhr.onreadystatechange = function xhrReadyStateChange(e) { + const { subscriber, progressSubscriber, request } = (xhrReadyStateChange); + if (this.readyState === 4) { + // normalize IE9 bug (http://bugs.jquery.com/ticket/1450) + let status: number = this.status === 1223 ? 204 : this.status; + + // fix status code when it is 0 (0 status is undocumented). + // Occurs when accessing file resources or on Android 4.1 stock browser + // while retrieving files from application cache. + if (status === 0) { + status = (this.response || this.responseText) ? 200 : 0; } - if (!subscriber.isUnsubscribed) { + + if (200 <= status && status < 300) { + if (progressSubscriber) { + progressSubscriber.complete(); + } + subscriber.next(e); subscriber.complete(); + } else { + if (progressSubscriber) { + progressSubscriber.error(e); + } + subscriber.error(new AjaxError('ajax error ' + status, this, request)); } - } else { - subscriber.error(normalizeError(e, xhr, 'error')); } + }; + (xhr.onreadystatechange).subscriber = this; + (xhr.onreadystatechange).progressSubscriber = progressSubscriber; + (xhr.onreadystatechange).request = request; + } + + unsubscribe() { + const { done, xhr } = this; + if (!done && xhr && xhr.readyState !== 4) { + xhr.abort(); } + super.unsubscribe(); } } -// Get CORS support even for older IE -function getCORSRequest() { - let xhr = new root.XMLHttpRequest(); - if ('withCredentials' in xhr) { - xhr.withCredentials = true; - return xhr; - } else if (!!root.XDomainRequest) { - return new root.XDomainRequest(); - } else { - throw new Error('CORS is not supported by your browser'); +/** A normalized AJAX response */ +export class AjaxResponse { + /** {number} the HTTP status code */ + status: number; + + /** {string|ArrayBuffer|object|any} the response data */ + response: any; + + /** {string} the raw responseText */ + responseText: string; + + /** {string} the responsType (e.g. 'json' or 'array-buffer') */ + responseType: string; + + /** {Document} an XML Document from the response */ + responseXML: Document; + + constructor(public originalEvent: Event, public xhr: XMLHttpRequest, public request: AjaxRequest) { + this.status = xhr.status; + const responseType = xhr.responseType; + let response = ('response' in xhr) ? xhr.response : xhr.responseText; + if (responseType === 'json') { + response = JSON.parse(response || ''); + } + this.responseText = xhr.responseText; + this.responseType = responseType; + this.responseXML = xhr.responseXML; + this.response = response; } } -function normalizeAjaxSuccessEvent(originalEvent, xhr, settings) { - let { status, response } = xhr; - const { responseType } = settings; - if (!('response' in xhr)) { - response = xhr.responseText; - } - if (responseType === 'json') { - response = JSON.parse(response || ''); +/** A normalized AJAX error */ +export class AjaxError extends Error { + /** {XMLHttpRequest} the XHR instance associated with the error */ + xhr: XMLHttpRequest; + + /** {AjaxRequest} the AjaxRequest associated with the error */ + request: AjaxRequest; + + /** {number} the HTTP status code */ + status: number; + + constructor(message: string, xhr: XMLHttpRequest, request: AjaxRequest) { + super(message); + this.message = message; + this.xhr = xhr; + this.request = request; + this.status = xhr.status; } - return { xhr, status, response, responseType, originalEvent }; } -function normalizeAjaxErrorEvent(originalEvent, xhr, type) { - return { xhr, type, status: xhr.status, originalEvent }; -} +export class AjaxTimeoutError extends AjaxError { + constructor(xhr: XMLHttpRequest, request: AjaxRequest) { + super('ajax timeout', xhr, request); + } +} \ No newline at end of file