From 146dbf1270b7f7d082e43aec11c76f848e51e6e3 Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Tue, 23 Jun 2015 20:46:03 -0700 Subject: [PATCH] refactor(Http): remove HttpFactory BREAKING CHANGE: HttpFactory is no longer available. This factory provided a function alternative to the `request` method of the Http class, but added no real value. The additional factory required an additional IHttp interface, an odd way to inject while preserving type information (`@Inject(HttpFactory) http:IHttp`), and required additional documentation in the http module. Closes #2564 --- modules/angular2/http.ts | 16 +++------ modules/angular2/src/http/http.ts | 33 ----------------- modules/angular2/src/http/interfaces.ts | 27 +------------- modules/angular2/test/http/http_spec.ts | 48 +------------------------ 4 files changed, 6 insertions(+), 118 deletions(-) diff --git a/modules/angular2/http.ts b/modules/angular2/http.ts index 19bc06de40..66e312a319 100644 --- a/modules/angular2/http.ts +++ b/modules/angular2/http.ts @@ -6,7 +6,7 @@ * class. */ import {bind, Binding} from 'angular2/di'; -import {Http, HttpFactory} from './src/http/http'; +import {Http} from './src/http/http'; import {XHRBackend, XHRConnection} from 'angular2/src/http/backends/xhr_backend'; import {BrowserXHR} from 'angular2/src/http/backends/browser_xhr'; import {BaseRequestOptions, RequestOptions} from 'angular2/src/http/base_request_options'; @@ -17,7 +17,6 @@ export {Request} from 'angular2/src/http/static_request'; export {Response} from 'angular2/src/http/static_response'; export { - IHttp, IRequestOptions, IResponse, Connection, @@ -26,7 +25,7 @@ export { export {BaseRequestOptions, RequestOptions} from 'angular2/src/http/base_request_options'; export {XHRBackend, XHRConnection} from 'angular2/src/http/backends/xhr_backend'; -export {Http, HttpFactory} from './src/http/http'; +export {Http} from './src/http/http'; export {Headers} from 'angular2/src/http/headers'; @@ -50,12 +49,5 @@ export {URLSearchParams} from 'angular2/src/http/url_search_params'; * ``` * */ -export var httpInjectables: List = [ - bind(ConnectionBackend) - .toClass(XHRBackend), - BrowserXHR, - XHRBackend, - BaseRequestOptions, - bind(HttpFactory).toFactory(HttpFactory, [XHRBackend, BaseRequestOptions]), - Http -]; +export var httpInjectables: List = + [bind(ConnectionBackend).toClass(XHRBackend), BrowserXHR, XHRBackend, BaseRequestOptions, Http]; diff --git a/modules/angular2/src/http/http.ts b/modules/angular2/src/http/http.ts index 2670517a13..e25ea763db 100644 --- a/modules/angular2/src/http/http.ts +++ b/modules/angular2/src/http/http.ts @@ -144,36 +144,3 @@ export class Http { RequestMethods.HEAD, url))); } } - -/** - * - * Alias to the `request` method of {@link Http}, for those who'd prefer a simple function instead - * of an object. In order to get TypeScript type information about the `HttpFactory`, the {@link - * IHttp} interface can be used as shown in the following example. - * - * #Example - * - * ``` - * import {httpInjectables, HttpFactory, IHttp} from 'angular2/http'; - * @Component({ - * appInjector: [httpInjectables] - * }) - * @View({ - * templateUrl: 'people.html' - * }) - * class MyComponent { - * constructor(@Inject(HttpFactory) http:IHttp) { - * http('people.json').subscribe(res => this.people = res.json()); - * } - * } - * ``` - **/ -export function HttpFactory(backend: ConnectionBackend, defaultOptions: BaseRequestOptions) { - return function(url: string | Request, options?: IRequestOptions) { - if (isString(url)) { - return httpRequest(backend, new Request(mergeOptions(defaultOptions, options, null, url))); - } else if (url instanceof Request) { - return httpRequest(backend, url); - } - }; -} diff --git a/modules/angular2/src/http/interfaces.ts b/modules/angular2/src/http/interfaces.ts index 8aff05c215..0b832dbdd9 100644 --- a/modules/angular2/src/http/interfaces.ts +++ b/modules/angular2/src/http/interfaces.ts @@ -55,33 +55,8 @@ export interface IResponse { url: string; totalBytes: number; bytesLoaded: number; - blob(): any; // TODO: Blob + blob(): any; // TODO: Blob arrayBuffer(): any; // TODO: ArrayBuffer text(): string; json(): Object; } - -/** - * Provides an interface to provide type information for {@link HttpFactory} when injecting. - * - * #Example - * - * ``` - * * import {httpInjectables, HttpFactory, IHttp} from 'angular2/http'; - * @Component({ - * appInjector: [httpInjectables] - * }) - * @View({ - * templateUrl: 'people.html' - * }) - * class MyComponent { - * constructor(@Inject(HttpFactory) http:IHttp) { - * http('people.json').subscribe(res => this.people = res.json()); - * } - * } - * ``` - * - */ -// Prefixed as IHttp because used in conjunction with Http class, but interface is callable -// constructor(@Inject(Http) http:IHttp) -export interface IHttp { (url: string, options?: IRequestOptions): EventEmitter } diff --git a/modules/angular2/test/http/http_spec.ts b/modules/angular2/test/http/http_spec.ts index 6553e73e80..fb97d7aabf 100644 --- a/modules/angular2/test/http/http_spec.ts +++ b/modules/angular2/test/http/http_spec.ts @@ -11,7 +11,7 @@ import { xit, SpyObject } from 'angular2/test_lib'; -import {Http, HttpFactory} from 'angular2/src/http/http'; +import {Http} from 'angular2/src/http/http'; import {Injector, bind} from 'angular2/di'; import {MockBackend} from 'angular2/src/http/backends/mock_backend'; import {Response} from 'angular2/src/http/static_response'; @@ -40,13 +40,10 @@ export function main() { var injector: Injector; var backend: MockBackend; var baseResponse; - var httpFactory; beforeEach(() => { injector = Injector.resolveAndCreate([ BaseRequestOptions, MockBackend, - bind(ConnectionBackend).toClass(MockBackend), - bind(HttpFactory).toFactory(HttpFactory, [MockBackend, BaseRequestOptions]), bind(Http).toFactory( function(backend: ConnectionBackend, defaultOptions: BaseRequestOptions) { return new Http(backend, defaultOptions); @@ -54,55 +51,12 @@ export function main() { [MockBackend, BaseRequestOptions]) ]); http = injector.get(Http); - httpFactory = injector.get(HttpFactory); backend = injector.get(MockBackend); baseResponse = new Response({body: 'base response'}); }); afterEach(() => backend.verifyNoPendingRequests()); - describe('HttpFactory', () => { - it('should return an Observable', () => { - expect(ObservableWrapper.isObservable(httpFactory(url))).toBe(true); - backend.resolveAllConnections(); - }); - - - it('should perform a get request for given url if only passed a string', - inject([AsyncTestCompleter], (async) => { - ObservableWrapper.subscribe(backend.connections, c => c.mockRespond(baseResponse)); - ObservableWrapper.subscribe(httpFactory('http://basic.connection'), res => { - expect(res.text()).toBe('base response'); - async.done(); - }); - - })); - - - it('should accept a fully-qualified request as its only parameter', - inject([AsyncTestCompleter], (async) => { - var req = new Request(new RequestOptions({url: 'https://google.com'})); - ObservableWrapper.subscribe(backend.connections, c => { - expect(c.request.url).toBe('https://google.com'); - c.mockRespond(new Response({body: 'Thank you'})); - async.done(); - }); - ObservableWrapper.subscribe(httpFactory(req), (res) => {}); - })); - - // TODO: make dart not complain about "argument type 'Map' cannot be assigned to the parameter - // type 'IRequestOptions'" - // xit('should perform a get request for given url if passed a dictionary', - // inject([AsyncTestCompleter], async => { - // ObservableWrapper.subscribe(backend.connections, c => c.mockRespond(baseResponse)); - // ObservableWrapper.subscribe(httpFactory(url, {method: RequestMethods.GET}), res => { - // expect(res.text()).toBe('base response'); - // async.done(); - // }); - // })); - }); - - describe('Http', () => { describe('.request()', () => { it('should return an Observable',