fix(AjaxObservable): drop resultSelector support in ajax method

closes #1783

BREAKING CHANGE: ajax.*() method no longer support resultSelector, encourage to use `map` instead

Author: kwonoj

spec/observables/dom/ajax-spec.ts

@@ -141,68 +141,6 @@ describe('Observable.ajax', () => {
     });
   });
 
-  it('should have an optional resultSelector', () => {
-    const expected = 'avast ye swabs!';
-    let result;
-    let complete = false;
-
-    const obj = {
-      url: '/flibbertyJibbet',
-      responseType: 'text',
-      resultSelector: (res: any) => res.response
-    };
-
-    (<any>Rx.Observable.ajax)(obj)
-      .subscribe((x: any) => {
-        result = x;
-      }, null, () => {
-        complete = true;
-      });
-
-    expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet');
-
-    MockXMLHttpRequest.mostRecent.respondWith({
-      'status': 200,
-      'contentType': 'application/json',
-      'responseText': expected
-    });
-
-    expect(result).to.equal(expected);
-    expect(complete).to.be.true;
-  });
-
-  it('should have error when resultSelector errors', () => {
-    const expected = 'avast ye swabs!';
-    let error;
-    const obj = {
-      url: '/flibbertyJibbet',
-      responseType: 'text',
-      resultSelector: (res: any) => {
-        throw new Error('ha! ha! fooled you!');
-      },
-      method: ''
-    };
-
-    Rx.Observable.ajax(obj)
-      .subscribe((x: any) => {
-        throw 'should not next';
-      }, (err: any) => {
-        error = err;
-      }, () => {
-        throw 'should not complete';
-      });
-
-    expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet');
-
-    MockXMLHttpRequest.mostRecent.respondWith({
-      'status': 200,
-      'contentType': 'application/json',
-      'responseText': expected
-    });
-
-    expect(error).to.be.an('error', 'ha! ha! fooled you!');
-  });
-
   it('should error if createXHR throws', () => {
     let error;
     const obj = {
@@ -213,7 +151,7 @@ describe('Observable.ajax', () => {
       }
     };
 
-    (<any>Rx.Observable.ajax)(obj)
+    Rx.Observable.ajax(<any>obj)
       .subscribe((x: any) => {
         throw 'should not next';
       }, (err: any) => {
@@ -299,13 +237,13 @@ describe('Observable.ajax', () => {
       method: ''
     };
 
-    Rx.Observable.ajax(obj).subscribe((x: any) => {
-        throw 'should not next';
-      }, (err: any) => {
-        error = err;
-      }, () => {
-        throw 'should not complete';
-      });
+    Rx.Observable.ajax(obj).subscribe(x => {
+      throw 'should not next';
+    }, (err: any) => {
+      error = err;
+    }, () => {
+      throw 'should not complete';
+    });
 
     expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet');
 
@@ -324,13 +262,13 @@ describe('Observable.ajax', () => {
     const expected = JSON.stringify({ foo: 'bar' });
 
     Rx.Observable.ajax('/flibbertyJibbet')
-        .subscribe((x: any) => {
-          expect(x.status).to.equal(200);
-          expect(x.xhr.method).to.equal('GET');
-          expect(x.xhr.responseText).to.equal(expected);
-        }, () => {
-          throw 'should not have been called';
-        });
+      .subscribe((x: any) => {
+        expect(x.status).to.equal(200);
+        expect(x.xhr.method).to.equal('GET');
+        expect(x.xhr.responseText).to.equal(expected);
+      }, () => {
+        throw 'should not have been called';
+      });
 
     expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet');
     MockXMLHttpRequest.mostRecent.respondWith({
@@ -344,15 +282,15 @@ describe('Observable.ajax', () => {
     const expected = JSON.stringify({ foo: 'bar' });
 
     Rx.Observable.ajax('/flibbertyJibbet')
-        .subscribe(() => {
-          throw 'should not have been called';
-        }, (x: any) => {
-          expect(x.status).to.equal(500);
-          expect(x.xhr.method).to.equal('GET');
-          expect(x.xhr.responseText).to.equal(expected);
-        }, () => {
-          throw 'should not have been called';
-        });
+      .subscribe(() => {
+        throw 'should not have been called';
+      }, (x: any) => {
+        expect(x.status).to.equal(500);
+        expect(x.xhr.method).to.equal('GET');
+        expect(x.xhr.responseText).to.equal(expected);
+      }, () => {
+        throw 'should not have been called';
+      });
 
     expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet');
     MockXMLHttpRequest.mostRecent.respondWith({
@@ -367,7 +305,7 @@ describe('Observable.ajax', () => {
 
     beforeEach(() => {
       rFormData = root.FormData;
-      root.FormData = root.FormData || class {};
+      root.FormData = root.FormData || class { };
     });
 
     afterEach(() => {
@@ -468,8 +406,8 @@ describe('Observable.ajax', () => {
 
       Rx.Observable
         .ajax.get('/flibbertyJibbet')
-        .subscribe((x: any) => {
-          result = x;
+        .subscribe(x => {
+          result = x.response;
         }, null, () => {
           complete = true;
         });
@@ -488,70 +426,39 @@ describe('Observable.ajax', () => {
       expect(complete).to.be.true;
     });
 
-    it('should succeed on 200 with a resultSelector', () => {
-      const expected = { larf: 'hahahahaha' };
-      let result;
-      let innerResult;
-      let complete = false;
-
-      Rx.Observable
-        .ajax.get('/flibbertyJibbet', (x: any) => {
-          innerResult = x;
-          return x.response.larf.toUpperCase();
-        })
-        .subscribe((x: any) => {
-          result = x;
-        }, null , () => {
-          complete = true;
+    describe('ajax.post', () => {
+      it('should succeed on 200', () => {
+        const expected = { foo: 'bar', hi: 'there you' };
+        let result: Rx.AjaxResponse;
+        let complete = false;
+
+        Rx.Observable
+          .ajax.post('/flibbertyJibbet', expected)
+          .subscribe(x => {
+            result = x;
+          }, null, () => {
+            complete = true;
+          });
+
+        const request = MockXMLHttpRequest.mostRecent;
+
+        expect(request.method).to.equal('POST');
+        expect(request.url).to.equal('/flibbertyJibbet');
+        expect(request.requestHeaders).to.deep.equal({
+          'X-Requested-With': 'XMLHttpRequest',
+          'Content-Type': 'application/x-www-form-urlencoded; charset=UTF-8'
         });
 
-      expect(MockXMLHttpRequest.mostRecent.url).to.equal('/flibbertyJibbet');
-
-      MockXMLHttpRequest.mostRecent.respondWith({
-        'status': 200,
-        'contentType': 'application/json',
-        'responseText': JSON.stringify(expected)
-      });
-
-      expect(innerResult.xhr).exist;
-      expect(innerResult.response).to.deep.equal({ larf: 'hahahahaha' });
-      expect(result).to.equal('HAHAHAHAHA');
-      expect(complete).to.be.true;
-    });
-  });
-
-  describe('ajax.post', () => {
-    it('should succeed on 200', () => {
-      const expected = { foo: 'bar', hi: 'there you' };
-      let result;
-      let complete = false;
-
-      Rx.Observable
-        .ajax.post('/flibbertyJibbet', expected)
-        .subscribe((x: any) => {
-          result = x;
-        }, null , () => {
-          complete = true;
+        request.respondWith({
+          'status': 200,
+          'contentType': 'application/json',
+          'responseText': JSON.stringify(expected)
         });
 
-      const request = MockXMLHttpRequest.mostRecent;
-
-      expect(request.method).to.equal('POST');
-      expect(request.url).to.equal('/flibbertyJibbet');
-      expect(request.requestHeaders).to.deep.equal({
-        'X-Requested-With': 'XMLHttpRequest',
-        'Content-Type': 'application/x-www-form-urlencoded; charset=UTF-8'
-      });
-
-      request.respondWith({
-        'status': 200,
-        'contentType': 'application/json',
-        'responseText': JSON.stringify(expected)
+        expect(request.data).to.equal('foo=bar&hi=there%20you');
+        expect(result.response).to.deep.equal(expected);
+        expect(complete).to.be.true;
       });
-
-      expect(request.data).to.equal('foo=bar&hi=there%20you');
-      expect(result.response).to.deep.equal(expected);
-      expect(complete).to.be.true;
     });
   });
-});
+});

src/observable/dom/AjaxObservable.ts

@@ -19,7 +19,6 @@ export interface AjaxRequest {
   withCredentials?: boolean;
   createXHR?: () => XMLHttpRequest;
   progressSubscriber?: Subscriber<any>;
-  resultSelector?: <T>(response: AjaxResponse) => T;
   responseType?: string;
 }
 
@@ -62,37 +61,32 @@ function getXMLHttpRequest(): XMLHttpRequest {
 }
 
 export interface AjaxCreationMethod {
-  <T>(urlOrRequest: string | AjaxRequest): Observable<T>;
-  get<T>(url: string, resultSelector?: (response: AjaxResponse) => T, headers?: Object): Observable<T>;
-  post<T>(url: string, body?: any, headers?: Object): Observable<T>;
-  put<T>(url: string, body?: any, headers?: Object): Observable<T>;
-  delete<T>(url: string, headers?: Object): Observable<T>;
+  (urlOrRequest: string | AjaxRequest): Observable<AjaxResponse>;
+  get(url: string, headers?: Object): Observable<AjaxResponse>;
+  post(url: string, body?: any, headers?: Object): Observable<AjaxResponse>;
+  put(url: string, body?: any, headers?: Object): Observable<AjaxResponse>;
+  delete(url: string, headers?: Object): Observable<AjaxResponse>;
   getJSON<T, R>(url: string, resultSelector?: (data: T) => R, headers?: Object): Observable<R>;
 }
 
-function defaultGetResultSelector<T>(response: AjaxResponse): T {
-  return response.response;
-}
-
-export function ajaxGet<T>(url: string, resultSelector: (response: AjaxResponse) => T = defaultGetResultSelector, headers: Object = null) {
-  return new AjaxObservable<T>({ method: 'GET', url, resultSelector, headers });
+export function ajaxGet(url: string, headers: Object = null) {
+  return new AjaxObservable<AjaxResponse>({ method: 'GET', url, headers });
 };
 
-export function ajaxPost<T>(url: string, body?: any, headers?: Object): Observable<T> {
-  return new AjaxObservable<T>({ method: 'POST', url, body, headers });
+export function ajaxPost(url: string, body?: any, headers?: Object): Observable<AjaxResponse> {
+  return new AjaxObservable<AjaxResponse>({ method: 'POST', url, body, headers });
 };
 
-export function ajaxDelete<T>(url: string, headers?: Object): Observable<T> {
-  return new AjaxObservable<T>({ method: 'DELETE', url, headers });
+export function ajaxDelete(url: string, headers?: Object): Observable<AjaxResponse> {
+  return new AjaxObservable<AjaxResponse>({ method: 'DELETE', url, headers });
 };
 
-export function ajaxPut<T>(url: string, body?: any, headers?: Object): Observable<T> {
-  return new AjaxObservable<T>({ method: 'PUT', url, body, headers });
+export function ajaxPut(url: string, body?: any, headers?: Object): Observable<AjaxResponse> {
+  return new AjaxObservable<AjaxResponse>({ method: 'PUT', url, body, headers });
 };
 
-export function ajaxGetJSON<T, R>(url: string, resultSelector?: (data: T) => R, headers?: Object): Observable<R> {
-  const finalResultSelector = resultSelector ? (res: AjaxResponse) => resultSelector(res.response) : (res: AjaxResponse) => res.response;
-  return new AjaxObservable<R>({ method: 'GET', url, responseType: 'json', resultSelector: finalResultSelector, headers });
+export function ajaxGetJSON<T>(url: string, headers?: Object): Observable<T> {
+  return new AjaxObservable<AjaxResponse>({ method: 'GET', url, responseType: 'json', headers }).map(x => x.response);
 };
 
 /**
@@ -184,7 +178,6 @@ export class AjaxObservable<T> extends Observable<T> {
  */
 export class AjaxSubscriber<T> extends Subscriber<Event> {
   private xhr: XMLHttpRequest;
-  private resultSelector: (response: AjaxResponse) => T;
   private done: boolean = false;
 
   constructor(destination: Subscriber<T>, public request: AjaxRequest) {
@@ -205,25 +198,15 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
     // properly serialize body
     request.body = this.serializeBody(request.body, request.headers['Content-Type']);
 
-    this.resultSelector = request.resultSelector;
     this.send();
   }
 
   next(e: Event): void {
     this.done = true;
-    const { resultSelector, xhr, request, destination } = this;
+    const { xhr, request, destination } = this;
     const response = new AjaxResponse(e, xhr, request);
 
-    if (resultSelector) {
-      const result = tryCatch(resultSelector)(response);
-      if (result === errorObject) {
-        this.error(errorObject.e);
-      } else {
-        destination.next(result);
-      }
-    } else {
-      destination.next(response);
-    }
+    destination.next(response);
   }
 
   private send(): XMLHttpRequest {