From c0492ef061afd5af2044d5952432d223615841a7 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Tue, 23 Oct 2018 22:43:43 +1100 Subject: [PATCH] Make Headers more idiomatic (#1062) --- js/dom_types.ts | 26 +++++- js/fetch.ts | 142 ++++++++++++++------------------- js/fetch_test.ts | 25 +----- js/globals.ts | 5 +- js/mixins/dom_iterable.ts | 70 ++++++++++++++++ js/mixins/dom_iterable_test.ts | 78 ++++++++++++++++++ js/unit_tests.ts | 2 +- js/util.ts | 19 ----- js/util_test.ts | 26 ------ 9 files changed, 241 insertions(+), 152 deletions(-) create mode 100644 js/mixins/dom_iterable.ts create mode 100644 js/mixins/dom_iterable_test.ts delete mode 100644 js/util_test.ts diff --git a/js/dom_types.ts b/js/dom_types.ts index 75f683be5b..3435825b13 100644 --- 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; +export type HeadersInit = + | Headers + | Array<[string, string]> + | Record; export type URLSearchParamsInit = string | string[][] | Record; type BodyInit = | Blob @@ -36,6 +39,18 @@ export type EventListenerOrEventListenerObject = | EventListener | EventListenerObject; +export interface DomIterable { + keys(): IterableIterator; + values(): IterableIterator; + 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; } -export interface Headers { +export interface Headers extends DomIterable { /** 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; 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 index 25db32d1d2..c0d1dc3e7d 100644 --- a/js/fetch.ts +++ b/js/fetch.ts @@ -5,8 +5,7 @@ import { createResolvable, Resolvable, typedArrayToArrayBuffer, - notImplemented, - CreateIterableIterator + notImplemented } 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 { - private headerMap: Map = new Map(); +class HeadersBase { + private [headerMap]: Map; - constructor(init?: domTypes.HeadersInit) { - 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)[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[] { + 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 { - 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 { - 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( + 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; //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 index 2f73544aa9..f92968d78c 100644 --- 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 index 401819ac61..d29d3b3da0 100644 --- 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 new file mode 100644 index 0000000000..0152265893 --- /dev/null +++ b/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 = 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( + // tslint:disable-next-line:variable-name + Base: TBase, + dataSymbol: symbol +): TBase & Constructor> { + // 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 { + for (const key of (this as any)[dataSymbol].keys()) { + yield key; + } + } + + *values(): IterableIterator { + 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 diff --git a/js/mixins/dom_iterable_test.ts b/js/mixins/dom_iterable_test.ts new file mode 100644 index 0000000000..a4791c1b8a --- /dev/null +++ b/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(); + + constructor( + data: Array<[string, number]> | IterableIterator<[string, number]> + ) { + for (const [key, value] of data) { + this[dataSymbol].set(key, value); + } + } + } + + return { + Base, + DomIterable: DomIterableMixin(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); +}); diff --git a/js/unit_tests.ts b/js/unit_tests.ts index 35cffc7673..f4b1e81392 100644 --- a/js/unit_tests.ts +++ b/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"; diff --git a/js/util.ts b/js/util.ts index fbd461bbe8..0c50acebb7 100644 --- a/js/util.ts +++ b/js/util.ts @@ -125,22 +125,3 @@ export function deferred(): Deferred { reject: reject! }; } - -/** Create a IterableIterator. */ -// @internal -export class CreateIterableIterator implements IterableIterator { - private readonly _iterators: IterableIterator; - readonly [Symbol.toStringTag] = "Iterator"; - - constructor(iterators: IterableIterator) { - this._iterators = iterators; - } - - [Symbol.iterator](): IterableIterator { - return this; - } - - next(): IteratorResult { - return this._iterators.next(); - } -} diff --git a/js/util_test.ts b/js/util_test.ts deleted file mode 100644 index 0ca2f4433d..0000000000 --- a/js/util_test.ts +++ /dev/null @@ -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]); - } -});