Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(http): remove all facade methods from http module #12870

Merged
merged 1 commit into from Nov 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/@angular/http/src/backends/browser_jsonp.ts
Expand Up @@ -7,15 +7,15 @@
*/

import {Injectable} from '@angular/core';
import {global} from '../facade/lang';

let _nextRequestId = 0;
export const JSONP_HOME = '__ng_jsonp__';
let _jsonpConnections: {[key: string]: any} = null;

function _getJsonpConnections(): {[key: string]: any} {
const w: {[key: string]: any} = typeof window == 'object' ? window : {};
if (_jsonpConnections === null) {
_jsonpConnections = (<{[key: string]: any}>global)[JSONP_HOME] = {};
_jsonpConnections = w[JSONP_HOME] = {};
}
return _jsonpConnections;
}
Expand Down
12 changes: 4 additions & 8 deletions modules/@angular/http/src/backends/jsonp_backend.ts
Expand Up @@ -12,7 +12,6 @@ import {Observer} from 'rxjs/Observer';

import {ResponseOptions} from '../base_response_options';
import {ReadyState, RequestMethod, ResponseType} from '../enums';
import {isPresent} from '../facade/lang';
import {Connection, ConnectionBackend} from '../interfaces';
import {Request} from '../static_request';
import {Response} from '../static_response';
Expand Down Expand Up @@ -89,15 +88,15 @@ export class JSONPConnection_ extends JSONPConnection {
if (!this._finished) {
let responseOptions =
new ResponseOptions({body: JSONP_ERR_NO_CALLBACK, type: ResponseType.Error, url});
if (isPresent(baseResponseOptions)) {
if (baseResponseOptions) {
responseOptions = baseResponseOptions.merge(responseOptions);
}
responseObserver.error(new Response(responseOptions));
return;
}

let responseOptions = new ResponseOptions({body: this._responseData, url});
if (isPresent(this.baseResponseOptions)) {
if (this.baseResponseOptions) {
responseOptions = this.baseResponseOptions.merge(responseOptions);
}

Expand All @@ -110,7 +109,7 @@ export class JSONPConnection_ extends JSONPConnection {
this.readyState = ReadyState.Done;
_dom.cleanup(script);
let responseOptions = new ResponseOptions({body: error.message, type: ResponseType.Error});
if (isPresent(baseResponseOptions)) {
if (baseResponseOptions) {
responseOptions = baseResponseOptions.merge(responseOptions);
}
responseObserver.error(new Response(responseOptions));
Expand All @@ -125,10 +124,7 @@ export class JSONPConnection_ extends JSONPConnection {
this.readyState = ReadyState.Cancelled;
script.removeEventListener('load', onLoad);
script.removeEventListener('error', onError);
if (isPresent(script)) {
this._dom.cleanup(script);
}

this._dom.cleanup(script);
};
});
}
Expand Down
40 changes: 18 additions & 22 deletions modules/@angular/http/src/base_request_options.ts
Expand Up @@ -8,8 +8,6 @@

import {Injectable} from '@angular/core';

import {isPresent} from '../src/facade/lang';

import {RequestMethod, ResponseContentType} from './enums';
import {Headers} from './headers';
import {normalizeMethodName} from './http_utils';
Expand Down Expand Up @@ -77,16 +75,14 @@ export class RequestOptions {
constructor(
{method, headers, body, url, search, withCredentials,
responseType}: RequestOptionsArgs = {}) {
this.method = isPresent(method) ? normalizeMethodName(method) : null;
this.headers = isPresent(headers) ? headers : null;
this.body = isPresent(body) ? body : null;
this.url = isPresent(url) ? url : null;
this.search = isPresent(search) ?
(typeof search === 'string' ? new URLSearchParams(<string>(search)) :
<URLSearchParams>(search)) :
null;
this.withCredentials = isPresent(withCredentials) ? withCredentials : null;
this.responseType = isPresent(responseType) ? responseType : null;
this.method = method != null ? normalizeMethodName(method) : null;
this.headers = headers != null ? headers : null;
this.body = body != null ? body : null;
this.url = url != null ? url : null;
this.search =
search != null ? (typeof search === 'string' ? new URLSearchParams(search) : search) : null;
this.withCredentials = withCredentials != null ? withCredentials : null;
this.responseType = responseType != null ? responseType : null;
}

/**
Expand Down Expand Up @@ -116,18 +112,18 @@ export class RequestOptions {
*/
merge(options?: RequestOptionsArgs): RequestOptions {
return new RequestOptions({
method: options && isPresent(options.method) ? options.method : this.method,
headers: options && isPresent(options.headers) ? options.headers : this.headers,
body: options && isPresent(options.body) ? options.body : this.body,
url: options && isPresent(options.url) ? options.url : this.url,
search: options && isPresent(options.search) ?
method: options && options.method != null ? options.method : this.method,
headers: options && options.headers != null ? options.headers : this.headers,
body: options && options.body != null ? options.body : this.body,
url: options && options.url != null ? options.url : this.url,
search: options && options.search != null ?
(typeof options.search === 'string' ? new URLSearchParams(options.search) :
(<URLSearchParams>(options.search)).clone()) :
options.search.clone()) :
this.search,
withCredentials: options && isPresent(options.withCredentials) ? options.withCredentials :
this.withCredentials,
responseType: options && isPresent(options.responseType) ? options.responseType :
this.responseType
withCredentials: options && options.withCredentials != null ? options.withCredentials :
this.withCredentials,
responseType: options && options.responseType != null ? options.responseType :
this.responseType
});
}
}
Expand Down
27 changes: 12 additions & 15 deletions modules/@angular/http/src/base_response_options.ts
Expand Up @@ -8,8 +8,6 @@

import {Injectable} from '@angular/core';

import {isPresent} from '../src/facade/lang';

import {ResponseType} from './enums';
import {Headers} from './headers';
import {ResponseOptionsArgs} from './interfaces';
Expand Down Expand Up @@ -68,12 +66,12 @@ export class ResponseOptions {
type: ResponseType;
url: string;
constructor({body, status, headers, statusText, type, url}: ResponseOptionsArgs = {}) {
this.body = isPresent(body) ? body : null;
this.status = isPresent(status) ? status : null;
this.headers = isPresent(headers) ? headers : null;
this.statusText = isPresent(statusText) ? statusText : null;
this.type = isPresent(type) ? type : null;
this.url = isPresent(url) ? url : null;
this.body = body != null ? body : null;
this.status = status != null ? status : null;
this.headers = headers != null ? headers : null;
this.statusText = statusText != null ? statusText : null;
this.type = type != null ? type : null;
this.url = url != null ? url : null;
}

/**
Expand Down Expand Up @@ -103,13 +101,12 @@ export class ResponseOptions {
*/
merge(options?: ResponseOptionsArgs): ResponseOptions {
return new ResponseOptions({
body: isPresent(options) && isPresent(options.body) ? options.body : this.body,
status: isPresent(options) && isPresent(options.status) ? options.status : this.status,
headers: isPresent(options) && isPresent(options.headers) ? options.headers : this.headers,
statusText: isPresent(options) && isPresent(options.statusText) ? options.statusText :
this.statusText,
type: isPresent(options) && isPresent(options.type) ? options.type : this.type,
url: isPresent(options) && isPresent(options.url) ? options.url : this.url,
body: options && options.body != null ? options.body : this.body,
status: options && options.status != null ? options.status : this.status,
headers: options && options.headers != null ? options.headers : this.headers,
statusText: options && options.statusText != null ? options.statusText : this.statusText,
type: options && options.type != null ? options.type : this.type,
url: options && options.url != null ? options.url : this.url,
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/@angular/http/src/body.ts
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {isJsObject, stringToArrayBuffer} from './http_utils';
import {stringToArrayBuffer} from './http_utils';
import {URLSearchParams} from './url_search_params';


Expand Down Expand Up @@ -51,7 +51,7 @@ export abstract class Body {
return '';
}

if (isJsObject(this._body)) {
if (typeof this._body === 'object') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (typeof this._body === 'object' && this._body !== null ) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null is already being checked a few lines above. (;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right :)

return JSON.stringify(this._body, null, 2);
}

Expand Down
1 change: 0 additions & 1 deletion modules/@angular/http/src/facade

This file was deleted.

10 changes: 3 additions & 7 deletions modules/@angular/http/src/http.ts
Expand Up @@ -8,7 +8,6 @@

import {Injectable} from '@angular/core';
import {Observable} from 'rxjs/Observable';
import {isPresent} from '../src/facade/lang';
import {BaseRequestOptions, RequestOptions} from './base_request_options';
import {RequestMethod} from './enums';
import {ConnectionBackend, RequestOptionsArgs} from './interfaces';
Expand All @@ -23,7 +22,7 @@ function mergeOptions(
defaultOpts: BaseRequestOptions, providedOpts: RequestOptionsArgs, method: RequestMethod,
url: string): RequestOptions {
const newOptions = defaultOpts;
if (isPresent(providedOpts)) {
if (providedOpts) {
// Hack so Dart can used named parameters
return newOptions.merge(new RequestOptions({
method: providedOpts.method || method,
Expand All @@ -35,11 +34,8 @@ function mergeOptions(
responseType: providedOpts.responseType
}));
}
if (isPresent(method)) {
return newOptions.merge(new RequestOptions({method: method, url: url}));
} else {
return newOptions.merge(new RequestOptions({url: url}));
}

return newOptions.merge(new RequestOptions({method, url}));
}

/**
Expand Down
2 changes: 0 additions & 2 deletions modules/@angular/http/src/http_utils.ts
Expand Up @@ -49,5 +49,3 @@ export function stringToArrayBuffer(input: String): ArrayBuffer {
}
return view.buffer;
}

export {isJsObject} from '../src/facade/lang';
5 changes: 1 addition & 4 deletions modules/@angular/http/src/static_request.ts
Expand Up @@ -6,8 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

import {isPresent} from '../src/facade/lang';

import {Body} from './body';
import {ContentType, RequestMethod, ResponseContentType} from './enums';
import {Headers} from './headers';
Expand Down Expand Up @@ -78,7 +76,7 @@ export class Request extends Body {
// TODO: assert that url is present
const url = requestOptions.url;
this.url = requestOptions.url;
if (isPresent(requestOptions.search)) {
if (requestOptions.search) {
const search = requestOptions.search.toString();
if (search.length > 0) {
let prefix = '?';
Expand All @@ -93,7 +91,6 @@ export class Request extends Body {
this.method = normalizeMethodName(requestOptions.method);
// TODO(jeffbcross): implement behavior
// Defaults to 'omit', consistent with browser
// TODO(jeffbcross): implement behavior
this.headers = new Headers(requestOptions.headers);
this.contentType = this.detectContentType();
this.withCredentials = requestOptions.withCredentials;
Expand Down
9 changes: 2 additions & 7 deletions modules/@angular/http/test/backends/jsonp_backend_spec.ts
Expand Up @@ -14,26 +14,21 @@ import {JSONPBackend, JSONPBackend_, JSONPConnection, JSONPConnection_} from '..
import {BaseRequestOptions, RequestOptions} from '../../src/base_request_options';
import {BaseResponseOptions, ResponseOptions} from '../../src/base_response_options';
import {ReadyState, RequestMethod, ResponseType} from '../../src/enums';
import {isPresent} from '../../src/facade/lang';
import {Request} from '../../src/static_request';

let existingScripts: MockBrowserJsonp[] = [];

class MockBrowserJsonp extends BrowserJsonp {
src: string;
callbacks = new Map<string, (data: any) => any>();
constructor() { super(); }

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 = {};
}
dispatchEvent(type: string, argument: any = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't this change the public API ?

cb(argument || {});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a spec, so should be good

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, nv

const cb = this.callbacks.get(type);
if (isPresent(cb)) {
if (cb) {
cb(argument);
}
}
Expand Down