Skip to content

Commit

Permalink
fix(AjaxObservable): drop resultSelector support in ajax method
Browse files Browse the repository at this point in the history 
closes ReactiveX#1783

BREAKING CHANGE: ajax.*() method no longer support resultSelector, encourage to use `map` instead
  • Loading branch information
@kwonoj
kwonoj committed Jul 11, 2016
1 parent ffcb9fc commit 6988a3d
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 183 deletions.
205 changes: 56 additions & 149 deletions spec/observables/dom/ajax-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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) => {
Expand Down Expand Up @@ -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');

Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -367,7 +305,7 @@ describe('Observable.ajax', () => {

beforeEach(() => {
rFormData = root.FormData;
root.FormData = root.FormData || class {};
root.FormData = root.FormData || class { };
});

afterEach(() => {
Expand Down Expand Up @@ -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;
});
Expand All @@ -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;
});
});
});
});
51 changes: 17 additions & 34 deletions src/observable/dom/AjaxObservable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export interface AjaxRequest {
withCredentials?: boolean;
createXHR?: () => XMLHttpRequest;
progressSubscriber?: Subscriber<any>;
resultSelector?: <T>(response: AjaxResponse) => T;
responseType?: string;
}

Expand Down Expand Up @@ -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);
};

/**
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down

0 comments on commit 6988a3d

Please sign in to comment.