Make Headers more idiomatic

denoland · Oct 22, 2018 · ec60a59 · ec60a59
1 parent c4bddc4
commit ec60a59
Show file tree

Hide file tree

Showing 9 changed files with 196 additions and 151 deletions.
diff --git a/js/dom_types.ts b/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"

diff --git a/js/fetch.ts b/js/fetch.ts
@@ -5,126 +5,106 @@ import {
   createResolvable,
   Resolvable,
   typedArrayToArrayBuffer,
-  notImplemented,
-  CreateIterableIterator
+  notImplemented
 } from "./util";
 import * as flatbuffers from "./flatbuffers";
 import { sendAsync } from "./dispatch";
 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";
 
-// ref: https://fetch.spec.whatwg.org/#dom-headers
-export class DenoHeaders implements domTypes.Headers {
-  private headerMap: Map<string, string> = new Map();
+// tslint:disable-next-line:no-any
+function isHeaders(value: any): value is domTypes.Headers {
+  return value instanceof Headers;
+}
 
-  constructor(init?: domTypes.HeadersInit) {
-    if (arguments.length === 0 || init === undefined) {
-      return;
-    }
+const headerMap = Symbol("header map");
 
-    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");
-    }
-  }
+// ref: https://fetch.spec.whatwg.org/#dom-headers
+class HeadersBase {
+  private [headerMap]: Map<string, string>;
 
-  private normalizeParams(name: string, value?: string): string[] {
+  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 v = this.headerMap.get(newname);
+    const [newname, newvalue] = this._normalizeParams(name, value);
+    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);
-    this.headerMap.delete(newname);
-  }
-
-  entries(): IterableIterator<[string, string]> {
-    const iterators = this.headerMap.entries();
-    return new CreateIterableIterator(iterators);
+    const [newname] = this._normalizeParams(name);
+    this[headerMap].delete(newname);
   }
 
   get(name: string): string | null {
-    const [newname] = this.normalizeParams(name);
-    const value = this.headerMap.get(newname);
+    const [newname] = this._normalizeParams(name);
+    const value = this[headerMap].get(newname);
     return value || null;
   }
 
   has(name: string): boolean {
-    const [newname] = this.normalizeParams(name);
-    return this.headerMap.has(newname);
-  }
-
-  keys(): IterableIterator<string> {
-    const iterators = this.headerMap.keys();
-    return new CreateIterableIterator(iterators);
+    const [newname] = this._normalizeParams(name);
+    return this[headerMap].has(newname);
   }
 
   set(name: string, value: string): void {
-    const [newname, newvalue] = this.normalizeParams(name, value);
-    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();
+    const [newname, newvalue] = this._normalizeParams(name, value);
+    this[headerMap].set(newname, newvalue);
   }
 }
 
+// @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;
   bodyUsed = false; // TODO
   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_);

diff --git a/js/fetch_test.ts b/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");
-  }
-
-  try {
-    const init = [["a", "b", "c"]];
-    new Headers(init);
-  } catch (e) {
-    assertEqual(e.message, "Failed to construct 'Headers': Invalid value");
+    assertEqual(
+      e.message,
+      "Failed to construct 'Headers'; The provided value was not valid"
+    );
   }
 });
 
@@ -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));
   }

diff --git a/js/globals.ts b/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;
diff --git a/js/mixins/dom_iterable.ts b/js/mixins/dom_iterable.ts
@@ -0,0 +1,55 @@
+import { DomIterable } from "../dom_types";
+
+// 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.
+
+  return class DomIterable 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 {
+      for (const [key, value] of (this as any)[dataSymbol].entries()) {
+        callbackfn.call(thisArg || this, value, key, this);
+      }
+    }
+
+    *[Symbol.iterator](): IterableIterator<[K, V]> {
+      for (const entry of (this as any)[dataSymbol]) {
+        yield entry;
+      }
+    }
+  };
+}
+// tslint:enable:no-any