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
ReactiveX · Jan 13, 2016 · 6df755f · 6df755f
1 parent ed8240e
commit 6df755f
Show file tree

Hide file tree

Showing 4 changed files with 340 additions and 176 deletions.
diff --git 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
@@ -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
@@ -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;