From 7a77437205a18905b0c2974fedf0ee4e8c624bd6 Mon Sep 17 00:00:00 2001 From: OJ Kwon Date: Fri, 8 Jul 2016 10:55:06 -0700 Subject: [PATCH] fix(AjaxObservable): drop resultSelector support in ajax method closes #1783 BREAKING CHANGE: ajax.*() method no longer support resultSelector, encourage to use `map` instead --- spec/observables/dom/ajax-spec.ts | 205 ++++++++------------------- src/observable/dom/AjaxObservable.ts | 51 +++---- 2 files changed, 73 insertions(+), 183 deletions(-) diff --git a/spec/observables/dom/ajax-spec.ts b/spec/observables/dom/ajax-spec.ts index 9970376ca3..1c37aeb98d 100644 --- a/spec/observables/dom/ajax-spec.ts +++ b/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 - }; - - (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', () => { } }; - (Rx.Observable.ajax)(obj) + Rx.Observable.ajax(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; }); }); -}); +}); \ No newline at end of file diff --git a/src/observable/dom/AjaxObservable.ts b/src/observable/dom/AjaxObservable.ts index 0129437b3e..a3b5689590 100644 --- a/src/observable/dom/AjaxObservable.ts +++ b/src/observable/dom/AjaxObservable.ts @@ -19,7 +19,6 @@ export interface AjaxRequest { withCredentials?: boolean; createXHR?: () => XMLHttpRequest; progressSubscriber?: Subscriber; - resultSelector?: (response: AjaxResponse) => T; responseType?: string; } @@ -62,37 +61,32 @@ function getXMLHttpRequest(): XMLHttpRequest { } export interface AjaxCreationMethod { - (urlOrRequest: string | AjaxRequest): Observable; - get(url: string, resultSelector?: (response: AjaxResponse) => T, headers?: Object): Observable; - post(url: string, body?: any, headers?: Object): Observable; - put(url: string, body?: any, headers?: Object): Observable; - delete(url: string, headers?: Object): Observable; + (urlOrRequest: string | AjaxRequest): Observable; + get(url: string, headers?: Object): Observable; + post(url: string, body?: any, headers?: Object): Observable; + put(url: string, body?: any, headers?: Object): Observable; + delete(url: string, headers?: Object): Observable; getJSON(url: string, resultSelector?: (data: T) => R, headers?: Object): Observable; } -function defaultGetResultSelector(response: AjaxResponse): T { - return response.response; -} - -export function ajaxGet(url: string, resultSelector: (response: AjaxResponse) => T = defaultGetResultSelector, headers: Object = null) { - return new AjaxObservable({ method: 'GET', url, resultSelector, headers }); +export function ajaxGet(url: string, headers: Object = null) { + return new AjaxObservable({ method: 'GET', url, headers }); }; -export function ajaxPost(url: string, body?: any, headers?: Object): Observable { - return new AjaxObservable({ method: 'POST', url, body, headers }); +export function ajaxPost(url: string, body?: any, headers?: Object): Observable { + return new AjaxObservable({ method: 'POST', url, body, headers }); }; -export function ajaxDelete(url: string, headers?: Object): Observable { - return new AjaxObservable({ method: 'DELETE', url, headers }); +export function ajaxDelete(url: string, headers?: Object): Observable { + return new AjaxObservable({ method: 'DELETE', url, headers }); }; -export function ajaxPut(url: string, body?: any, headers?: Object): Observable { - return new AjaxObservable({ method: 'PUT', url, body, headers }); +export function ajaxPut(url: string, body?: any, headers?: Object): Observable { + return new AjaxObservable({ method: 'PUT', url, body, headers }); }; -export function ajaxGetJSON(url: string, resultSelector?: (data: T) => R, headers?: Object): Observable { - const finalResultSelector = resultSelector ? (res: AjaxResponse) => resultSelector(res.response) : (res: AjaxResponse) => res.response; - return new AjaxObservable({ method: 'GET', url, responseType: 'json', resultSelector: finalResultSelector, headers }); +export function ajaxGetJSON(url: string, headers?: Object): Observable { + return new AjaxObservable({ method: 'GET', url, responseType: 'json', headers }).map(x => x.response); }; /** @@ -184,7 +178,6 @@ export class AjaxObservable extends Observable { */ export class AjaxSubscriber extends Subscriber { private xhr: XMLHttpRequest; - private resultSelector: (response: AjaxResponse) => T; private done: boolean = false; constructor(destination: Subscriber, public request: AjaxRequest) { @@ -205,25 +198,15 @@ export class AjaxSubscriber extends Subscriber { // 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 {