diff --git a/modules/angular2/src/http/backends/jsonp_backend.ts b/modules/angular2/src/http/backends/jsonp_backend.ts index 57cfe58297..6b7a4a71a6 100644 --- a/modules/angular2/src/http/backends/jsonp_backend.ts +++ b/modules/angular2/src/http/backends/jsonp_backend.ts @@ -8,11 +8,12 @@ import {BrowserJsonp} from './browser_jsonp'; import {EventEmitter, ObservableWrapper} from 'angular2/src/core/facade/async'; import {makeTypeError} from 'angular2/src/core/facade/exceptions'; import {StringWrapper, isPresent} from 'angular2/src/core/facade/lang'; +var Observable = require('@reactivex/rxjs/dist/cjs/Observable'); export class JSONPConnection implements Connection { readyState: ReadyStates; request: Request; - response: EventEmitter; + response: any; private _id: string; private _script: Element; private _responseData: any; @@ -27,50 +28,66 @@ export class JSONPConnection implements Connection { throw makeTypeError("JSONP requests must use GET request method."); } this.request = req; - this.response = new EventEmitter(); - this.readyState = ReadyStates.Loading; - this._id = _dom.nextRequestID(); + this.response = new Observable(responseObserver => { - _dom.exposeConnection(this._id, this); + this.readyState = ReadyStates.Loading; + let id = this._id = _dom.nextRequestID(); - // Workaround Dart - // url = url.replace(/=JSONP_CALLBACK(&|$)/, `generated method`); - let callback = _dom.requestCallback(this._id); - let url: string = req.url; - if (url.indexOf('=JSONP_CALLBACK&') > -1) { - url = StringWrapper.replace(url, '=JSONP_CALLBACK&', `=${callback}&`); - } else if (url.lastIndexOf('=JSONP_CALLBACK') === url.length - '=JSONP_CALLBACK'.length) { - url = StringWrapper.substring(url, 0, url.length - '=JSONP_CALLBACK'.length) + `=${callback}`; - } + _dom.exposeConnection(id, this); - let script = this._script = _dom.build(url); - - script.addEventListener('load', (event) => { - if (this.readyState === ReadyStates.Cancelled) return; - this.readyState = ReadyStates.Done; - _dom.cleanup(script); - if (!this._finished) { - ObservableWrapper.callThrow( - this.response, makeTypeError('JSONP injected script did not invoke callback.')); - return; + // Workaround Dart + // url = url.replace(/=JSONP_CALLBACK(&|$)/, `generated method`); + let callback = _dom.requestCallback(this._id); + let url: string = req.url; + if (url.indexOf('=JSONP_CALLBACK&') > -1) { + url = StringWrapper.replace(url, '=JSONP_CALLBACK&', `=${callback}&`); + } else if (url.lastIndexOf('=JSONP_CALLBACK') === url.length - '=JSONP_CALLBACK'.length) { + url = + StringWrapper.substring(url, 0, url.length - '=JSONP_CALLBACK'.length) + `=${callback}`; } - let responseOptions = new ResponseOptions({body: this._responseData}); - if (isPresent(this.baseResponseOptions)) { - responseOptions = this.baseResponseOptions.merge(responseOptions); + let script = this._script = _dom.build(url); + + let onLoad = event => { + if (this.readyState === ReadyStates.Cancelled) return; + this.readyState = ReadyStates.Done; + _dom.cleanup(script); + if (!this._finished) { + responseObserver.error(makeTypeError('JSONP injected script did not invoke callback.')); + return; + } + + let responseOptions = new ResponseOptions({body: this._responseData}); + if (isPresent(this.baseResponseOptions)) { + responseOptions = this.baseResponseOptions.merge(responseOptions); + } + + responseObserver.next(new Response(responseOptions)); + responseObserver.complete(); + }; + + let onError = error => { + if (this.readyState === ReadyStates.Cancelled) return; + this.readyState = ReadyStates.Done; + _dom.cleanup(script); + responseObserver.error(error); + }; + + script.addEventListener('load', onLoad); + script.addEventListener('error', onError); + + _dom.send(script); + + return () => { + this.readyState = ReadyStates.Cancelled; + script.removeEventListener('load', onLoad); + script.removeEventListener('error', onError); + if (isPresent(script)) { + this._dom.cleanup(script); + } + } - - ObservableWrapper.callNext(this.response, new Response(responseOptions)); }); - - script.addEventListener('error', (error) => { - if (this.readyState === ReadyStates.Cancelled) return; - this.readyState = ReadyStates.Done; - _dom.cleanup(script); - ObservableWrapper.callThrow(this.response, error); - }); - - _dom.send(script); } finished(data?: any) { @@ -80,16 +97,6 @@ export class JSONPConnection implements Connection { if (this.readyState === ReadyStates.Cancelled) return; this._responseData = data; } - - dispose(): void { - this.readyState = ReadyStates.Cancelled; - let script = this._script; - this._script = null; - if (isPresent(script)) { - this._dom.cleanup(script); - } - ObservableWrapper.callReturn(this.response); - } } @Injectable() diff --git a/modules/angular2/src/http/backends/xhr_backend.ts b/modules/angular2/src/http/backends/xhr_backend.ts index 569ec8091e..c9903188e4 100644 --- a/modules/angular2/src/http/backends/xhr_backend.ts +++ b/modules/angular2/src/http/backends/xhr_backend.ts @@ -5,78 +5,78 @@ import {Response} from '../static_response'; import {ResponseOptions, BaseResponseOptions} from '../base_response_options'; import {Injectable} from 'angular2/src/core/di'; import {BrowserXhr} from './browser_xhr'; -import {EventEmitter, ObservableWrapper} from 'angular2/src/core/facade/async'; import {isPresent} from 'angular2/src/core/facade/lang'; - +var Observable = require('@reactivex/rxjs/dist/cjs/Observable'); /** - * Creates connections using `XMLHttpRequest`. Given a fully-qualified - * request, an `XHRConnection` will immediately create an `XMLHttpRequest` object and send the - * request. - * - * This class would typically not be created or interacted with directly inside applications, though - * the {@link MockConnection} may be interacted with in tests. - */ +* Creates connections using `XMLHttpRequest`. Given a fully-qualified +* request, an `XHRConnection` will immediately create an `XMLHttpRequest` object and send the +* request. +* +* This class would typically not be created or interacted with directly inside applications, though +* the {@link MockConnection} may be interacted with in tests. +*/ export class XHRConnection implements Connection { request: Request; /** * Response {@link EventEmitter} which emits a single {@link Response} value on load event of * `XMLHttpRequest`. */ - response: EventEmitter; // TODO: Make generic of ; + response: any; // TODO: Make generic of ; readyState: ReadyStates; - private _xhr; // TODO: make type XMLHttpRequest, pending resolution of - // https://github.com/angular/ts2dart/issues/230 constructor(req: Request, browserXHR: BrowserXhr, baseResponseOptions?: ResponseOptions) { this.request = req; - this.response = new EventEmitter(); - this._xhr = browserXHR.build(); - // TODO(jeffbcross): implement error listening/propagation - this._xhr.open(RequestMethods[req.method].toUpperCase(), req.url); - this._xhr.addEventListener('load', (_) => { - // responseText is the old-school way of retrieving response (supported by IE8 & 9) - // response/responseType properties were introduced in XHR Level2 spec (supported by IE10) - let response = isPresent(this._xhr.response) ? this._xhr.response : this._xhr.responseText; + this.response = new Observable(responseObserver => { + let _xhr: XMLHttpRequest = browserXHR.build(); + _xhr.open(RequestMethods[req.method].toUpperCase(), req.url); + // load event handler + let onLoad = () => { + // responseText is the old-school way of retrieving response (supported by IE8 & 9) + // response/responseType properties were introduced in XHR Level2 spec (supported by + // IE10) + let response = isPresent(_xhr.response) ? _xhr.response : _xhr.responseText; - // normalize IE9 bug (http://bugs.jquery.com/ticket/1450) - let status = this._xhr.status === 1223 ? 204 : this._xhr.status; + // normalize IE9 bug (http://bugs.jquery.com/ticket/1450) + let status = _xhr.status === 1223 ? 204 : _xhr.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 = response ? 200 : 0; + // 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 = response ? 200 : 0; + } + var responseOptions = new ResponseOptions({body: response, status: status}); + if (isPresent(baseResponseOptions)) { + responseOptions = baseResponseOptions.merge(responseOptions); + } + responseObserver.next(new Response(responseOptions)); + // TODO(gdi2290): defer complete if array buffer until done + responseObserver.complete(); + }; + // error event handler + let onError = (err) => { + var responseOptions = new ResponseOptions({body: err, type: ResponseTypes.Error}); + if (isPresent(baseResponseOptions)) { + responseOptions = baseResponseOptions.merge(responseOptions); + } + responseObserver.error(new Response(responseOptions)); + }; + + if (isPresent(req.headers)) { + req.headers.forEach((value, name) => { _xhr.setRequestHeader(name, value); }); } - var responseOptions = new ResponseOptions({body: response, status: status}); - if (isPresent(baseResponseOptions)) { - responseOptions = baseResponseOptions.merge(responseOptions); - } + _xhr.addEventListener('load', onLoad); + _xhr.addEventListener('error', onError); - ObservableWrapper.callNext(this.response, new Response(responseOptions)); - // TODO(gdi2290): defer complete if array buffer until done - ObservableWrapper.callReturn(this.response); + _xhr.send(this.request.text()); + + return () => { + _xhr.removeEventListener('load', onLoad); + _xhr.removeEventListener('error', onError); + _xhr.abort(); + }; }); - - this._xhr.addEventListener('error', (err) => { - var responseOptions = new ResponseOptions({body: err, type: ResponseTypes.Error}); - if (isPresent(baseResponseOptions)) { - responseOptions = baseResponseOptions.merge(responseOptions); - } - ObservableWrapper.callThrow(this.response, new Response(responseOptions)); - }); - // TODO(jeffbcross): make this more dynamic based on body type - - if (isPresent(req.headers)) { - req.headers.forEach((value, name) => { this._xhr.setRequestHeader(name, value); }); - } - - this._xhr.send(this.request.text()); } - - /** - * Calls abort on the underlying XMLHttpRequest. - */ - dispose(): void { this._xhr.abort(); } } /** diff --git a/modules/angular2/src/http/http.ts b/modules/angular2/src/http/http.ts index 159191f6e8..00f4b3101b 100644 --- a/modules/angular2/src/http/http.ts +++ b/modules/angular2/src/http/http.ts @@ -34,19 +34,9 @@ function mergeOptions(defaultOpts, providedOpts, method, url): RequestOptions { * Performs http requests using `XMLHttpRequest` as the default backend. * * `Http` is available as an injectable class, with methods to perform http requests. Calling - * `request` returns an {@link EventEmitter} which will emit a single {@link Response} when a + * `request` returns an {@link Observable} which will emit a single {@link Response} when a * response is received. * - * - * ## Breaking Change - * - * Previously, methods of `Http` would return an RxJS Observable directly. For now, - * the `toRx()` method of {@link EventEmitter} needs to be called in order to get the RxJS - * Subject. `EventEmitter` does not provide combinators like `map`, and has different semantics for - * subscribing/observing. This is temporary; the result of all `Http` method calls will be either an - * Observable - * or Dart Stream when [issue #2794](https://github.com/angular/angular/issues/2794) is resolved. - * * #Example * * ``` @@ -56,8 +46,6 @@ function mergeOptions(defaultOpts, providedOpts, method, url): RequestOptions { * class PeopleComponent { * constructor(http: Http) { * http.get('people.json') - * //Get the RxJS Subject - * .toRx() * // Call map on the response observable to get the parsed people object * .map(res => res.json()) * // Subscribe to the observable to get the parsed people object and attach it to the @@ -67,9 +55,6 @@ function mergeOptions(defaultOpts, providedOpts, method, url): RequestOptions { * } * ``` * - * To use the {@link EventEmitter} returned by `Http`, simply pass a generator (See "interface - *Generator" in the Async Generator spec: https://github.com/jhusain/asyncgenerator) to the - *`observer` method of the returned emitter, with optional methods of `next`, `throw`, and `return`. * * #Example * @@ -95,7 +80,7 @@ function mergeOptions(defaultOpts, providedOpts, method, url): RequestOptions { * [MockBackend, BaseRequestOptions]) * ]); * var http = injector.get(Http); - * http.get('request-from-mock-backend.json').toRx().subscribe((res:Response) => doSomething(res)); + * http.get('request-from-mock-backend.json').subscribe((res:Response) => doSomething(res)); * ``` * **/ diff --git a/modules/angular2/src/http/interfaces.ts b/modules/angular2/src/http/interfaces.ts index 99e40cf8e9..1e3f849602 100644 --- a/modules/angular2/src/http/interfaces.ts +++ b/modules/angular2/src/http/interfaces.ts @@ -26,7 +26,6 @@ export class Connection { readyState: ReadyStates; request: Request; response: EventEmitter; // TODO: generic of ; - dispose(): void { throw new BaseException('Abstract!'); } } /** diff --git a/modules/angular2/test/http/backends/jsonp_backend_spec.ts b/modules/angular2/test/http/backends/jsonp_backend_spec.ts index 193e48bb45..d9d578f2ed 100644 --- a/modules/angular2/test/http/backends/jsonp_backend_spec.ts +++ b/modules/angular2/test/http/backends/jsonp_backend_spec.ts @@ -38,11 +38,16 @@ class MockBrowserJsonp extends BrowserJsonp { addEventListener(type: string, cb: (data: any) => any) { this.callbacks.set(type, cb); } + removeEventListener(type: string, cb: Function) { this.callbacks.delete(type); } + dispatchEvent(type: string, argument?: any) { if (!isPresent(argument)) { argument = {}; } - this.callbacks.get(type)(argument); + let cb = this.callbacks.get(type); + if (isPresent(cb)) { + cb(argument); + } } build(url: string) { @@ -89,7 +94,7 @@ export function main() { inject([AsyncTestCompleter], async => { let connection = new JSONPConnection(sampleRequest, new MockBrowserJsonp(), new ResponseOptions({type: ResponseTypes.Error})); - ObservableWrapper.subscribe(connection.response, res => { + connection.response.subscribe(res => { expect(res.type).toBe(ResponseTypes.Error); async.done(); }); @@ -104,17 +109,17 @@ export function main() { let errorSpy = spy.spy('error'); let returnSpy = spy.spy('cancelled'); - ObservableWrapper.subscribe(connection.response, loadSpy, errorSpy, returnSpy); - connection.dispose(); - expect(connection.readyState).toBe(ReadyStates.Cancelled); + let request = connection.response.subscribe(loadSpy, errorSpy, returnSpy); + request.unsubscribe(); connection.finished('Fake data'); existingScripts[0].dispatchEvent('load'); TimerWrapper.setTimeout(() => { + expect(connection.readyState).toBe(ReadyStates.Cancelled); expect(loadSpy).not.toHaveBeenCalled(); expect(errorSpy).not.toHaveBeenCalled(); - expect(returnSpy).toHaveBeenCalled(); + expect(returnSpy).not.toHaveBeenCalled(); async.done(); }, 10); })); @@ -122,8 +127,7 @@ export function main() { it('should report error if loaded without invoking callback', inject([AsyncTestCompleter], async => { let connection = new JSONPConnection(sampleRequest, new MockBrowserJsonp()); - ObservableWrapper.subscribe( - connection.response, + connection.response.subscribe( res => { expect("response listener called").toBe(false); async.done(); @@ -139,15 +143,15 @@ export function main() { it('should report error if script contains error', inject([AsyncTestCompleter], async => { let connection = new JSONPConnection(sampleRequest, new MockBrowserJsonp()); - ObservableWrapper.subscribe(connection.response, - res => { - expect("response listener called").toBe(false); - async.done(); - }, - err => { - expect(err['message']).toBe('Oops!'); - async.done(); - }); + connection.response.subscribe( + res => { + expect("response listener called").toBe(false); + async.done(); + }, + err => { + expect(err['message']).toBe('Oops!'); + async.done(); + }); existingScripts[0].dispatchEvent('error', ({message: "Oops!"})); })); @@ -159,14 +163,15 @@ export function main() { let base = new BaseRequestOptions(); let req = new Request( base.merge(new RequestOptions({url: 'https://google.com', method: method}))); - expect(() => new JSONPConnection(req, new MockBrowserJsonp())).toThrowError(); + expect(() => new JSONPConnection(req, new MockBrowserJsonp()).response.subscribe()) + .toThrowError(); }); }); it('should respond with data passed to callback', inject([AsyncTestCompleter], async => { let connection = new JSONPConnection(sampleRequest, new MockBrowserJsonp()); - ObservableWrapper.subscribe(connection.response, res => { + connection.response.subscribe(res => { expect(res.json()).toEqual(({fake_payload: true, blob_id: 12345})); async.done(); }); diff --git a/modules/angular2/test/http/backends/xhr_backend_spec.ts b/modules/angular2/test/http/backends/xhr_backend_spec.ts index 4527565bd1..99f9578e60 100644 --- a/modules/angular2/test/http/backends/xhr_backend_spec.ts +++ b/modules/angular2/test/http/backends/xhr_backend_spec.ts @@ -58,6 +58,8 @@ class MockBrowserXHR extends BrowserXhr { addEventListener(type: string, cb: Function) { this.callbacks.set(type, cb); } + removeEventListener(type: string, cb: Function) { this.callbacks.delete(type); } + dispatchEvent(type: string) { this.callbacks.get(type)({}); } build() { @@ -95,7 +97,7 @@ export function main() { inject([AsyncTestCompleter], async => { var connection = new XHRConnection(sampleRequest, new MockBrowserXHR(), new ResponseOptions({type: ResponseTypes.Error})); - ObservableWrapper.subscribe(connection.response, res => { + connection.response.subscribe(res => { expect(res.type).toBe(ResponseTypes.Error); async.done(); }); @@ -105,40 +107,44 @@ export function main() { it('should complete a request', inject([AsyncTestCompleter], async => { var connection = new XHRConnection(sampleRequest, new MockBrowserXHR(), new ResponseOptions({type: ResponseTypes.Error})); - ObservableWrapper.subscribe(connection.response, res => { - expect(res.type).toBe(ResponseTypes.Error); - }, null, () => { async.done(); }); + connection.response.subscribe(res => { expect(res.type).toBe(ResponseTypes.Error); }, + null, () => { async.done(); }); existingXHRs[0].dispatchEvent('load'); })); it('should call abort when disposed', () => { var connection = new XHRConnection(sampleRequest, new MockBrowserXHR()); - connection.dispose(); + var request = connection.response.subscribe(); + request.unsubscribe(); expect(abortSpy).toHaveBeenCalled(); }); it('should create an error Response on error', inject([AsyncTestCompleter], async => { var connection = new XHRConnection(sampleRequest, new MockBrowserXHR(), new ResponseOptions({type: ResponseTypes.Error})); - ObservableWrapper.subscribe(connection.response, null, res => { + connection.response.subscribe(null, res => { expect(res.type).toBe(ResponseTypes.Error); async.done(); }); existingXHRs[0].dispatchEvent('error'); })); - it('should automatically call open with method and url', () => { - new XHRConnection(sampleRequest, new MockBrowserXHR()); + it('should call open with method and url when subscribed to', () => { + var connection = new XHRConnection(sampleRequest, new MockBrowserXHR()); + expect(openSpy).not.toHaveBeenCalled(); + connection.response.subscribe(); expect(openSpy).toHaveBeenCalledWith('GET', sampleRequest.url); }); - it('should automatically call send on the backend with request body', () => { + it('should call send on the backend with request body when subscribed to', () => { var body = 'Some body to love'; var base = new BaseRequestOptions(); - new XHRConnection(new Request(base.merge(new RequestOptions({body: body}))), - new MockBrowserXHR()); + var connection = new XHRConnection( + new Request(base.merge(new RequestOptions({body: body}))), new MockBrowserXHR()); + expect(sendSpy).not.toHaveBeenCalled(); + connection.response.subscribe(); expect(sendSpy).toHaveBeenCalledWith(body); }); @@ -146,8 +152,9 @@ export function main() { var headers = new Headers({'Content-Type': 'text/xml', 'Breaking-Bad': '<3'}); var base = new BaseRequestOptions(); - new XHRConnection(new Request(base.merge(new RequestOptions({headers: headers}))), - new MockBrowserXHR()); + var connection = new XHRConnection( + new Request(base.merge(new RequestOptions({headers: headers}))), new MockBrowserXHR()); + connection.response.subscribe(); expect(setRequestHeaderSpy).toHaveBeenCalledWith('Content-Type', ['text/xml']); expect(setRequestHeaderSpy).toHaveBeenCalledWith('Breaking-Bad', ['<3']); }); @@ -157,7 +164,7 @@ export function main() { var connection = new XHRConnection(sampleRequest, new MockBrowserXHR(), new ResponseOptions({status: statusCode})); - ObservableWrapper.subscribe(connection.response, res => { + connection.response.subscribe(res => { expect(res.status).toBe(statusCode); async.done(); }); @@ -172,7 +179,7 @@ export function main() { var connection = new XHRConnection(sampleRequest, new MockBrowserXHR(), new ResponseOptions({status: statusCode})); - ObservableWrapper.subscribe(connection.response, res => { + connection.response.subscribe(res => { expect(res.status).toBe(normalizedCode); async.done(); }); @@ -190,19 +197,18 @@ export function main() { var connection2 = new XHRConnection(sampleRequest, new MockBrowserXHR(), new ResponseOptions()); - ObservableWrapper.subscribe(connection1.response, res => { + connection1.response.subscribe(res => { expect(res.text()).toBe(responseBody); - ObservableWrapper.subscribe(connection2.response, ress => { + connection2.response.subscribe(ress => { expect(ress.text()).toBe(responseBody); async.done(); }); + existingXHRs[1].setResponse(responseBody); existingXHRs[1].dispatchEvent('load'); }); existingXHRs[0].setResponseText(responseBody); - existingXHRs[1].setResponse(responseBody); - existingXHRs[0].dispatchEvent('load'); })); diff --git a/modules/examples/src/http/http_comp.ts b/modules/examples/src/http/http_comp.ts index fac8537b47..5322311c79 100644 --- a/modules/examples/src/http/http_comp.ts +++ b/modules/examples/src/http/http_comp.ts @@ -1,6 +1,5 @@ import {Component, View, NgFor} from 'angular2/angular2'; -import {Http, Response} from 'angular2/http'; -import {ObservableWrapper} from 'angular2/src/core/facade/async'; +import {Http} from 'angular2/http'; @Component({selector: 'http-app'}) @View({ @@ -16,8 +15,5 @@ import {ObservableWrapper} from 'angular2/src/core/facade/async'; }) export class HttpCmp { people: Object; - constructor(http: Http) { - ObservableWrapper.subscribe(http.get('./people.json'), - res => this.people = res.json()); - } + constructor(http: Http) { http.get('./people.json').subscribe(res => this.people = res.json()); } } diff --git a/modules/examples/src/jsonp/jsonp_comp.ts b/modules/examples/src/jsonp/jsonp_comp.ts index 9292ff28e8..9bc3e45f50 100644 --- a/modules/examples/src/jsonp/jsonp_comp.ts +++ b/modules/examples/src/jsonp/jsonp_comp.ts @@ -17,7 +17,6 @@ import {ObservableWrapper} from 'angular2/src/core/facade/async'; export class JsonpCmp { people: Object; constructor(jsonp: Jsonp) { - ObservableWrapper.subscribe(jsonp.get('./people.json?callback=JSONP_CALLBACK'), - res => this.people = res.json()); + jsonp.get('./people.json?callback=JSONP_CALLBACK').subscribe(res => this.people = res.json()); } }