Make Headers more idiomatic (#1062)

js/dom_types.ts

@@ -13,7 +13,10 @@ See the Apache Version 2.0 License for specific language governing permissions
 and limitations under the License.
 *******************************************************************************/
 
-export type HeadersInit = Headers | string[][] | Record<string, string>;
+export type HeadersInit =
+  | Headers
+  | Array<[string, string]>
+  | Record<string, string>;
 export type URLSearchParamsInit = string | string[][] | Record<string, string>;
 type BodyInit =
   | Blob
@@ -36,6 +39,18 @@ export type EventListenerOrEventListenerObject =
   | EventListener
   | EventListenerObject;
 
+export interface DomIterable<K, V> {
+  keys(): IterableIterator<K>;
+  values(): IterableIterator<V>;
+  entries(): IterableIterator<[K, V]>;
+  [Symbol.iterator](): IterableIterator<[K, V]>;
+  forEach(
+    callback: (value: V, key: K, parent: this) => void,
+    // tslint:disable-next-line:no-any
+    thisArg?: any
+  ): void;
+}
+
 interface Element {
   // TODO
 }
@@ -289,7 +304,7 @@ interface Body {
   text(): Promise<string>;
 }
 
-export interface Headers {
+export interface Headers extends DomIterable<string, string> {
   /** Appends a new value onto an existing header inside a `Headers` object, or
    * adds the header if it does not already exist.
    */
@@ -322,7 +337,7 @@ export interface Headers {
    */
   values(): IterableIterator<string>;
   forEach(
-    callbackfn: (value: string, key: string, parent: Headers) => void,
+    callbackfn: (value: string, key: string, parent: this) => void,
     // tslint:disable-next-line:no-any
     thisArg?: any
   ): void;
@@ -332,6 +347,11 @@ export interface Headers {
   [Symbol.iterator](): IterableIterator<[string, string]>;
 }
 
+export interface HeadersConstructor {
+  new (init?: HeadersInit): Headers;
+  prototype: Headers;
+}
+
 type RequestCache =
   | "default"
   | "no-store"

js/fetch.ts

@@ -5,8 +5,7 @@ import {
   createResolvable,
   Resolvable,
   typedArrayToArrayBuffer,
-  notImplemented,
+  notImplemented
-  CreateIterableIterator
 } from "./util";
 import * as flatbuffers from "./flatbuffers";
 import { sendAsync } from "./dispatch";
@@ -14,109 +13,90 @@ import * as msg from "gen/msg_generated";
 import * as domTypes from "./dom_types";
 import { TextDecoder } from "./text_encoding";
 import { DenoBlob } from "./blob";
+import { DomIterableMixin } from "./mixins/dom_iterable";
+
+// tslint:disable-next-line:no-any
+function isHeaders(value: any): value is domTypes.Headers {
+  return value instanceof Headers;
+}
+
+const headerMap = Symbol("header map");
 
 // ref: https://fetch.spec.whatwg.org/#dom-headers
-export class DenoHeaders implements domTypes.Headers {
+class HeadersBase {
-  private headerMap: Map<string, string> = new Map();
+  private [headerMap]: Map<string, string>;
 
-  constructor(init?: domTypes.HeadersInit) {
+  private _normalizeParams(name: string, value?: string): string[] {
-    if (arguments.length === 0 || init === undefined) {
-      return;
-    }
-
-    if (init instanceof DenoHeaders) {
-      // init is the instance of Header
-      init.forEach((value: string, name: string) => {
-        this.headerMap.set(name, value);
-      });
-    } else if (Array.isArray(init)) {
-      // init is a sequence
-      init.forEach(item => {
-        if (item.length !== 2) {
-          throw new TypeError("Failed to construct 'Headers': Invalid value");
-        }
-        const [name, value] = this.normalizeParams(item[0], item[1]);
-        const v = this.headerMap.get(name);
-        const str = v ? `${v}, ${value}` : value;
-        this.headerMap.set(name, str);
-      });
-    } else if (Object.prototype.toString.call(init) === "[object Object]") {
-      // init is a object
-      const names = Object.keys(init);
-      names.forEach(name => {
-        const value = (init as Record<string, string>)[name];
-        const [newname, newvalue] = this.normalizeParams(name, value);
-        this.headerMap.set(newname, newvalue);
-      });
-    } else {
-      throw new TypeError("Failed to construct 'Headers': Invalid value");
-    }
-  }
-
-  private normalizeParams(name: string, value?: string): string[] {
     name = String(name).toLowerCase();
     value = String(value).trim();
     return [name, value];
   }
 
+  constructor(init?: domTypes.HeadersInit) {
+    if (init === null) {
+      throw new TypeError(
+        "Failed to construct 'Headers'; The provided value was not valid"
+      );
+    } else if (isHeaders(init)) {
+      this[headerMap] = new Map(init);
+    } else {
+      this[headerMap] = new Map();
+      if (Array.isArray(init)) {
+        for (const [rawName, rawValue] of init) {
+          const [name, value] = this._normalizeParams(rawName, rawValue);
+          const existingValue = this[headerMap].get(name);
+          this[headerMap].set(
+            name,
+            existingValue ? `${existingValue}, ${value}` : value
+          );
+        }
+      } else if (init) {
+        const names = Object.keys(init);
+        for (const rawName of names) {
+          const rawValue = init[rawName];
+          const [name, value] = this._normalizeParams(rawName, rawValue);
+          this[headerMap].set(name, value);
+        }
+      }
+    }
+  }
+
   append(name: string, value: string): void {
-    const [newname, newvalue] = this.normalizeParams(name, value);
+    const [newname, newvalue] = this._normalizeParams(name, value);
-    const v = this.headerMap.get(newname);
+    const v = this[headerMap].get(newname);
     const str = v ? `${v}, ${newvalue}` : newvalue;
-    this.headerMap.set(newname, str);
+    this[headerMap].set(newname, str);
   }
 
   delete(name: string): void {
-    const [newname] = this.normalizeParams(name);
+    const [newname] = this._normalizeParams(name);
-    this.headerMap.delete(newname);
+    this[headerMap].delete(newname);
-  }
-
-  entries(): IterableIterator<[string, string]> {
-    const iterators = this.headerMap.entries();
-    return new CreateIterableIterator(iterators);
   }
 
   get(name: string): string | null {
-    const [newname] = this.normalizeParams(name);
+    const [newname] = this._normalizeParams(name);
-    const value = this.headerMap.get(newname);
+    const value = this[headerMap].get(newname);
     return value || null;
   }
 
   has(name: string): boolean {
-    const [newname] = this.normalizeParams(name);
+    const [newname] = this._normalizeParams(name);
-    return this.headerMap.has(newname);
+    return this[headerMap].has(newname);
-  }
-
-  keys(): IterableIterator<string> {
-    const iterators = this.headerMap.keys();
-    return new CreateIterableIterator(iterators);
   }
 
   set(name: string, value: string): void {
-    const [newname, newvalue] = this.normalizeParams(name, value);
+    const [newname, newvalue] = this._normalizeParams(name, value);
-    this.headerMap.set(newname, newvalue);
+    this[headerMap].set(newname, newvalue);
-  }
-
-  values(): IterableIterator<string> {
-    const iterators = this.headerMap.values();
-    return new CreateIterableIterator(iterators);
-  }
-
-  forEach(
-    callbackfn: (value: string, key: string, parent: domTypes.Headers) => void,
-    // tslint:disable-next-line:no-any
-    thisArg?: any
-  ): void {
-    this.headerMap.forEach((value, name) => {
-      callbackfn(value, name, this);
-    });
-  }
-
-  [Symbol.iterator](): IterableIterator<[string, string]> {
-    return this.entries();
   }
 }
 
+// @internal
+// tslint:disable-next-line:variable-name
+export const Headers = DomIterableMixin<string, string, typeof HeadersBase>(
+  HeadersBase,
+  headerMap
+);
+
 class FetchResponse implements domTypes.Response {
   readonly url: string = "";
   body: null;
@@ -124,7 +104,7 @@ class FetchResponse implements domTypes.Response {
   statusText = "FIXME"; // TODO
   readonly type = "basic"; // TODO
   redirected = false; // TODO
-  headers: DenoHeaders;
+  headers: domTypes.Headers;
   readonly trailer: Promise<domTypes.Headers>;
   //private bodyChunks: Uint8Array[] = [];
   private first = true;
@@ -138,7 +118,7 @@ class FetchResponse implements domTypes.Response {
   ) {
     this.bodyWaiter = createResolvable();
     this.trailer = createResolvable();
-    this.headers = new DenoHeaders(headersList);
+    this.headers = new Headers(headersList);
     this.bodyData = body_;
     setTimeout(() => {
       this.bodyWaiter.resolve(body_);

js/fetch_test.ts

@@ -26,16 +26,6 @@ testPerm({ net: true }, async function fetchHeaders() {
   assert(headers.get("Server").startsWith("SimpleHTTP"));
 });
 
-test(async function headersAppend() {
-  let err;
-  try {
-    const headers = new Headers([["foo", "bar", "baz"]]);
-  } catch (e) {
-    err = e;
-  }
-  assert(err instanceof TypeError);
-});
-
 testPerm({ net: true }, async function fetchBlob() {
   const response = await fetch("http://localhost:4545/package.json");
   const headers = response.headers;
@@ -69,14 +59,10 @@ test(function newHeaderTest() {
   try {
     new Headers(null);
   } catch (e) {
-    assertEqual(e.message, "Failed to construct 'Headers': Invalid value");
+    assertEqual(
-  }
+      e.message,
-
+      "Failed to construct 'Headers'; The provided value was not valid"
-  try {
+    );
-    const init = [["a", "b", "c"]];
-    new Headers(init);
-  } catch (e) {
-    assertEqual(e.message, "Failed to construct 'Headers': Invalid value");
   }
 });
 
@@ -163,7 +149,6 @@ test(function headerGetSuccess() {
 test(function headerEntriesSuccess() {
   const headers = new Headers(headerDict);
   const iterators = headers.entries();
-  assertEqual(Object.prototype.toString.call(iterators), "[object Iterator]");
   for (const it of iterators) {
     const key = it[0];
     const value = it[1];
@@ -175,7 +160,6 @@ test(function headerEntriesSuccess() {
 test(function headerKeysSuccess() {
   const headers = new Headers(headerDict);
   const iterators = headers.keys();
-  assertEqual(Object.prototype.toString.call(iterators), "[object Iterator]");
   for (const it of iterators) {
     assert(headers.has(it));
   }
@@ -189,7 +173,6 @@ test(function headerValuesSuccess() {
   for (const pair of entries) {
     values.push(pair[1]);
   }
-  assertEqual(Object.prototype.toString.call(iterators), "[object Iterator]");
   for (const it of iterators) {
     assert(values.includes(it));
   }

js/globals.ts

@@ -7,6 +7,7 @@ import { libdeno } from "./libdeno";
 import * as textEncoding from "./text_encoding";
 import * as timers from "./timers";
 import * as urlSearchParams from "./url_search_params";
+import * as domTypes from "./dom_types";
 
 // During the build process, augmentations to the variable `window` in this
 // file are tracked and created as part of default library that is built into
@@ -38,5 +39,7 @@ window.URLSearchParams = urlSearchParams.URLSearchParams;
 
 window.fetch = fetch_.fetch;
 
-window.Headers = fetch_.DenoHeaders;
+// using the `as` keyword to mask the internal types when generating the
+// runtime library
+window.Headers = fetch_.Headers as domTypes.HeadersConstructor;
 window.Blob = blob.DenoBlob;

js/mixins/dom_iterable.ts

@@ -0,0 +1,70 @@
+import { DomIterable } from "../dom_types";
+import { globalEval } from "../global_eval";
+
+// if we import it directly from "globals" it will break the unit tests so we
+// have to grab a reference to the global scope a different way
+const window = globalEval("this");
+
+// tslint:disable:no-any
+type Constructor<T = {}> = new (...args: any[]) => T;
+
+/** Mixes in a DOM iterable methods into a base class, assumes that there is
+ * a private data iterable that is part of the base class, located at
+ * `[dataSymbol]`.
+ */
+export function DomIterableMixin<K, V, TBase extends Constructor>(
+  // tslint:disable-next-line:variable-name
+  Base: TBase,
+  dataSymbol: symbol
+): TBase & Constructor<DomIterable<K, V>> {
+  // we have to cast `this` as `any` because there is no way to describe the
+  // Base class in a way where the Symbol `dataSymbol` is defined.  So the
+  // runtime code works, but we do lose a little bit of type safety.
+
+  // tslint:disable-next-line:variable-name
+  const DomIterable = class extends Base {
+    *entries(): IterableIterator<[K, V]> {
+      for (const entry of (this as any)[dataSymbol].entries()) {
+        yield entry;
+      }
+    }
+
+    *keys(): IterableIterator<K> {
+      for (const key of (this as any)[dataSymbol].keys()) {
+        yield key;
+      }
+    }
+
+    *values(): IterableIterator<V> {
+      for (const value of (this as any)[dataSymbol].values()) {
+        yield value;
+      }
+    }
+
+    forEach(
+      callbackfn: (value: V, key: K, parent: this) => void,
+      // tslint:disable-next-line:no-any
+      thisArg?: any
+    ): void {
+      callbackfn = callbackfn.bind(thisArg == null ? window : Object(thisArg));
+      for (const [key, value] of (this as any)[dataSymbol].entries()) {
+        callbackfn(value, key, this);
+      }
+    }
+
+    *[Symbol.iterator](): IterableIterator<[K, V]> {
+      for (const entry of (this as any)[dataSymbol]) {
+        yield entry;
+      }
+    }
+  };
+
+  // we want the Base class name to be the name of the class.
+  Object.defineProperty(DomIterable, "name", {
+    value: Base.name,
+    configurable: true
+  });
+
+  return DomIterable;
+}
+// tslint:enable:no-any

js/mixins/dom_iterable_test.ts

@@ -0,0 +1,78 @@
+// Copyright 2018 the Deno authors. All rights reserved. MIT license.
+import { test, assert, assertEqual } from "../test_util.ts";
+import { DomIterableMixin } from "./dom_iterable.ts";
+
+function setup() {
+  const dataSymbol = Symbol("data symbol");
+  class Base {
+    private [dataSymbol] = new Map<string, number>();
+
+    constructor(
+      data: Array<[string, number]> | IterableIterator<[string, number]>
+    ) {
+      for (const [key, value] of data) {
+        this[dataSymbol].set(key, value);
+      }
+    }
+  }
+
+  return {
+    Base,
+    DomIterable: DomIterableMixin<string, number, typeof Base>(Base, dataSymbol)
+  };
+}
+
+test(function testDomIterable() {
+  // tslint:disable-next-line:variable-name
+  const { DomIterable, Base } = setup();
+
+  const fixture: Array<[string, number]> = [["foo", 1], ["bar", 2]];
+
+  const domIterable = new DomIterable(fixture);
+
+  assertEqual(Array.from(domIterable.entries()), fixture);
+  assertEqual(Array.from(domIterable.values()), [1, 2]);
+  assertEqual(Array.from(domIterable.keys()), ["foo", "bar"]);
+
+  let result: Array<[string, number]> = [];
+  for (const [key, value] of domIterable) {
+    assert(key != null);
+    assert(value != null);
+    result.push([key, value]);
+  }
+  assertEqual(fixture, result);
+
+  result = [];
+  const scope = {};
+  function callback(value, key, parent) {
+    assertEqual(parent, domIterable);
+    assert(key != null);
+    assert(value != null);
+    assert(this === scope);
+    result.push([key, value]);
+  }
+  domIterable.forEach(callback, scope);
+  assertEqual(fixture, result);
+
+  assertEqual(DomIterable.name, Base.name);
+});
+
+test(function testDomIterableScope() {
+  // tslint:disable-next-line:variable-name
+  const { DomIterable } = setup();
+
+  const domIterable = new DomIterable([["foo", 1]]);
+
+  // tslint:disable-next-line:no-any
+  function checkScope(thisArg: any, expected: any) {
+    function callback() {
+      assertEqual(this, expected);
+    }
+    domIterable.forEach(callback, thisArg);
+  }
+
+  checkScope(0, Object(0));
+  checkScope("", Object(""));
+  checkScope(null, window);
+  checkScope(undefined, window);
+});

js/unit_tests.ts

@@ -29,4 +29,4 @@ import "./v8_source_maps_test.ts";
 import "../website/app_test.js";
 import "./metrics_test.ts";
 import "./url_search_params_test.ts";
-import "./util_test.ts";
+import "./mixins/dom_iterable_test.ts";

js/util.ts

@@ -125,22 +125,3 @@ export function deferred(): Deferred {
     reject: reject!
   };
 }
-
-/** Create a IterableIterator. */
-// @internal
-export class CreateIterableIterator<T> implements IterableIterator<T> {
-  private readonly _iterators: IterableIterator<T>;
-  readonly [Symbol.toStringTag] = "Iterator";
-
-  constructor(iterators: IterableIterator<T>) {
-    this._iterators = iterators;
-  }
-
-  [Symbol.iterator](): IterableIterator<T> {
-    return this;
-  }
-
-  next(): IteratorResult<T> {
-    return this._iterators.next();
-  }
-}

js/util_test.ts

@@ -1,26 +0,0 @@
-// Copyright 2018 the Deno authors. All rights reserved. MIT license.
-import { test, assert, assertEqual } from "./test_util.ts";
-import { CreateIterableIterator } from "./util";
-
-test(function CreateIterableIteratorSuccess() {
-  const list = [1, 2, 3, 4, 5];
-  const listIterators = new CreateIterableIterator(list.values());
-  let idx = 0;
-  for (const it of listIterators) {
-    assertEqual(it, list[idx++]);
-  }
-  const obj = {
-    a: "foo",
-    b: "bar",
-    c: "baz"
-  };
-  const list1 = [];
-  const keys = Object.keys(obj);
-  keys.forEach(key => list1.push([key, obj[key]]));
-  const objectIterators = new CreateIterableIterator(list1.values());
-  for (const it of objectIterators) {
-    const [key, value] = it;
-    assert(key in obj);
-    assertEqual(value, obj[key]);
-  }
-});