Merge pull request #1306 from jridgewell/xhr-cleanup

XHR cleanup

extensions/amp-analytics/0.1/amp-analytics.js

@@ -21,6 +21,7 @@ import {log} from '../../../src/log';
 import {urlReplacementsFor} from '../../../src/url-replacements';
 import {expandTemplate} from '../../../src/string';
 import {xhrFor} from '../../../src/xhr';
+import {isArray, isObject} from '../../../src/types';
 
 import {addListener} from './instrumentation';
 import {sendRequest} from './transport';
@@ -309,20 +310,15 @@ export class AmpAnalytics extends AMP.BaseElement {
    * @private
    */
   mergeObjects_(from, to) {
-    // Checks if the given object is a plain object.
-    const isObject = function(someObj) {
-      return Object.prototype.toString.call(someObj) === '[object Object]';
-    };
-
     if (to === null || to === undefined) {
       to = {};
     }
 
     for (const property in from) {
+      // Only deal with own properties.
       if (from.hasOwnProperty(property)) {
-        // Only deal with own properties.
-        if (Array.isArray(from[property])) {
-          if (!Array.isArray(to[property])) {
+        if (isArray(from[property])) {
+          if (!isArray(to[property])) {
             to[property] = [];
           }
           to[property] = this.mergeObjects_(from[property], to[property]);

src/service/action-impl.js

@@ -19,6 +19,7 @@ import {getService} from '../service';
 import {log} from '../log';
 import {timer} from '../timer';
 import {vsyncFor} from '../vsync';
+import {isArray} from '../types';
 
 /** @const {string} */
 const TAG_ = 'Action';
@@ -156,8 +157,11 @@ export class ActionService {
 
     const currentQueue = target[ACTION_QUEUE_];
     if (currentQueue) {
-      assert(Object.prototype.toString.call(currentQueue) == '[object Array]',
-          'Expected queue to be an array: %s', debugid);
+      assert(
+        isArray(currentQueue),
+        'Expected queue to be an array: %s',
+        debugid
+      );
     }
 
     // Override queue with the handler.

src/types.js

@@ -0,0 +1,45 @@
+/**
+ * Copyright 2015 The AMP HTML Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS-IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* @const */
+const toString_ = Object.prototype.toString;
+
+/**
+ * Returns the ECMA [[Class]] of a value
+ * @param {*} value
+ * @return {string}
+ */
+function toString(value) {
+  return toString_.call(value);
+}
+
+/**
+ * Determines if value is actually an Array.
+ * @param {*} value
+ * @return {boolean}
+ */
+export function isArray(value) {
+  return toString(value) === '[object Array]';
+}
+
+/**
+ * Determines if value is actually an Object.
+ * @param {*} value
+ * @return {boolean}
+ */
+export function isObject(value) {
+  return toString(value) === '[object Object]';
+}

src/xhr.js

@@ -16,6 +16,7 @@
 
 import {assert} from './asserts';
 import {getService} from './service';
+import {isArray, isObject} from './types';
 
 
 /**
@@ -36,8 +37,8 @@ let FetchInitDef;
 /** @private @const {!Array<string>} */
 const allowedMethods_ = ['GET', 'POST'];
 
-/** @private @const {!Array<string>} */
-const allowedBodyTypes_ = ['[object Object]', '[object Array]'];
+/** @private @const {!Array<function:boolean>} */
+const allowedBodyTypes_ = [isArray, isObject];
 
 
 /**
@@ -103,7 +104,16 @@ export function normalizeMethod_(method) {
   if (method === undefined) {
     return 'GET';
   }
-  return method.toUpperCase();
+  method = method.toUpperCase();
+
+  assert(
+    allowedMethods_.indexOf(method) > -1,
+    'Only one of %s is currently allowed. Got %s',
+    allowedMethods_.join(', '),
+    method
+  );
+
+  return method;
 }
 
 /**
@@ -112,22 +122,18 @@ export function normalizeMethod_(method) {
  * @private
  */
 function setupJson_(init) {
-  assert(allowedMethods_.indexOf(init.method) != -1, 'Only one of ' +
-      allowedMethods_.join(', ') + ' is currently allowed. Got %s',
-      init.method);
-
   init.headers = {
     'Accept': 'application/json'
   };
 
   if (init.method == 'POST') {
-    const bodyType = Object.prototype.toString.call(init.body);
-
     // Assume JSON strict mode where only objects or arrays are allowed
     // as body.
-    assert(allowedBodyTypes_.indexOf(bodyType) > -1,
-        'body must be of type object or array. %s',
-        init.body);
+    assert(
+      allowedBodyTypes_.some(test => test(init.body)),
+      'body must be of type object or array. %s',
+      init.body
+    );
 
     init.headers['Content-Type'] = 'application/json;charset=utf-8';
     init.body = JSON.stringify(init.body);