From 19ece66b2924c0c4501307c1a3efb716b41fca5e Mon Sep 17 00:00:00 2001 From: Justin Date: Fri, 13 Nov 2020 13:45:23 -0800 Subject: [PATCH 1/2] feat: add retries --- src/index.test.ts | 44 ++++++++++++++++++++++++++++++++++++---- src/index.ts | 51 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index bb9c7b98..7df5a254 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -14,7 +14,9 @@ * limitations under the License. */ -import { Loader, LoaderOptions } from "."; +import { DEFAULT_ID, Loader, LoaderOptions } from "."; + +jest.useFakeTimers(); afterEach(() => { document.getElementsByTagName("html")[0].innerHTML = ""; @@ -54,6 +56,10 @@ test.each([ expect(loader.createUrl()).toEqual(expected); }); +test("uses default id if empty string", () => { + expect(new Loader({ apiKey: "foo", id: "" }).id).toBe(DEFAULT_ID); +}); + test("setScript adds a script to head with correct attributes", () => { const loader = new Loader({ apiKey: "foo" }); @@ -110,7 +116,7 @@ test("loadCallback callback should fire", () => { }); test("script onerror should reject promise", async () => { - const loader = new Loader({ apiKey: "foo" }); + const loader = new Loader({ apiKey: "foo", retries: 0 }); const rejection = expect(loader.load()).rejects.toBeInstanceOf(ErrorEvent); @@ -119,11 +125,12 @@ test("script onerror should reject promise", async () => { await rejection; expect(loader["done"]).toBeTruthy(); expect(loader["loading"]).toBeFalsy(); + expect(loader["errors"].length).toBe(1); }); test("script onerror should reject promise with multiple loaders", async () => { - const loader = new Loader({ apiKey: "foo" }); - const extraLoader = new Loader({ apiKey: "foo" }); + const loader = new Loader({ apiKey: "foo", retries: 0 }); + const extraLoader = new Loader({ apiKey: "foo", retries: 0 }); let rejection = expect(loader.load()).rejects.toBeInstanceOf(ErrorEvent); loader["loadErrorCallback"](document.createEvent("ErrorEvent")); @@ -139,6 +146,24 @@ test("script onerror should reject promise with multiple loaders", async () => { expect(extraLoader["loading"]).toBeFalsy(); }); +test("script onerror should retry", async () => { + const loader = new Loader({ apiKey: "foo", retries: 1 }); + const deleteScript = jest.spyOn(loader, "deleteScript"); + const rejection = expect(loader.load()).rejects.toBeInstanceOf(ErrorEvent); + const log = jest.spyOn(console, "log").mockImplementation(()=>{}) + + loader["loadErrorCallback"](document.createEvent("ErrorEvent")); + loader["loadErrorCallback"](document.createEvent("ErrorEvent")); + jest.runAllTimers(); + + await rejection; + expect(loader["done"]).toBeTruthy(); + expect(loader["loading"]).toBeFalsy(); + expect(loader["errors"].length).toBe(2); + expect(deleteScript).toHaveBeenCalledTimes(1); + expect(log).toHaveBeenCalledTimes(loader.retries); +}); + test("singleton should be used", () => { const loader = new Loader({ apiKey: "foo" }); const extraLoader = new Loader({ apiKey: "foo" }); @@ -195,3 +220,14 @@ test("loader should resolve immediately when google.maps defined", async () => { delete window.google; expect(console.warn).toHaveBeenCalledTimes(1); }); + +test("deleteScript removes script tag from head", () => { + const loader = new Loader({ apiKey: "foo" }); + loader["setScript"](); + expect(document.head.childNodes.length).toBe(1); + loader.deleteScript(); + expect(document.head.childNodes.length).toBe(0); + // should work without script existing + loader.deleteScript(); + expect(document.head.childNodes.length).toBe(0); +}); diff --git a/src/index.ts b/src/index.ts index 234e17f1..4dd06c8f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -25,6 +25,8 @@ declare global { } } +export const DEFAULT_ID = "__googleMapsScriptId"; + type Libraries = ( | "drawing" | "geometry" @@ -37,6 +39,8 @@ type Libraries = ( * The Google Maps JavaScript API * [documentation](https://developers.google.com/maps/documentation/javascript/tutorial) * is the authoritative source for [[LoaderOptions]]. +/** + * Loader options */ export interface LoaderOptions { /** @@ -157,6 +161,10 @@ export interface LoaderOptions { * Use a cryptographic nonce attribute. */ nonce?: string; + /** + * The number of script load retries. + */ + retries?: number; } /** @@ -223,6 +231,11 @@ export class Loader { */ nonce: string | null; + /** + * See [[LoaderOptions.retries]] + */ + retries: number; + /** * See [[LoaderOptions.url]] */ @@ -234,6 +247,7 @@ export class Loader { private loading = false; private onerrorEvent: Event; private static instance: Loader; + private errors: ErrorEvent[] = []; /** * Creates an instance of Loader using [[LoaderOptions]]. No defaults are set @@ -248,25 +262,27 @@ export class Loader { apiKey, channel, client, - id = "__googleMapsScriptId", + id = DEFAULT_ID, libraries = [], language, region, version, mapIds, nonce, + retries = 3, url = "https://maps.googleapis.com/maps/api/js", }: LoaderOptions) { this.version = version; this.apiKey = apiKey; this.channel = channel; this.client = client; - this.id = id; + this.id = id || DEFAULT_ID; // Do not allow empty string this.libraries = libraries; this.language = language; this.region = region; this.mapIds = mapIds; this.nonce = nonce; + this.retries = retries; this.url = url; if (Loader.instance) { @@ -299,6 +315,7 @@ export class Loader { url: this.url, }; } + /** * CreateUrl returns the Google Maps JavaScript API script url given the [[LoaderOptions]]. * @@ -380,7 +397,7 @@ export class Loader { * Set the script on document. */ private setScript(): void { - if (this.id && document.getElementById(this.id)) { + if (document.getElementById(this.id)) { // TODO wrap onerror callback for cases where the script was loaded elsewhere this.callback(); return; @@ -402,9 +419,31 @@ export class Loader { document.head.appendChild(script); } - private loadErrorCallback(e: Event): void { - this.onerrorEvent = e; - this.callback(); + deleteScript(): void { + const script = document.getElementById(this.id); + if (script) { + script.remove(); + } + } + + private loadErrorCallback(e: ErrorEvent): void { + this.errors.push(e); + + if (this.errors.length <= this.retries) { + const delay = this.errors.length * 2 ** this.errors.length; + + console.log( + `Failed to load Google Maps script, retrying in ${delay} ms.` + ); + + setTimeout(() => { + this.deleteScript(); + this.setScript(); + }, delay); + } else { + this.onerrorEvent = e; + this.callback(); + } } private setCallback(): void { From 25b4ef13a2a4ee35234118cc80e2483a800a3667 Mon Sep 17 00:00:00 2001 From: Justin Date: Fri, 13 Nov 2020 13:50:23 -0800 Subject: [PATCH 2/2] style: fix lint issues --- src/index.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index 7df5a254..77746077 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -150,8 +150,9 @@ test("script onerror should retry", async () => { const loader = new Loader({ apiKey: "foo", retries: 1 }); const deleteScript = jest.spyOn(loader, "deleteScript"); const rejection = expect(loader.load()).rejects.toBeInstanceOf(ErrorEvent); - const log = jest.spyOn(console, "log").mockImplementation(()=>{}) - + // eslint-disable-next-line @typescript-eslint/no-empty-function + console.log = jest.fn(); + loader["loadErrorCallback"](document.createEvent("ErrorEvent")); loader["loadErrorCallback"](document.createEvent("ErrorEvent")); jest.runAllTimers(); @@ -161,7 +162,7 @@ test("script onerror should retry", async () => { expect(loader["loading"]).toBeFalsy(); expect(loader["errors"].length).toBe(2); expect(deleteScript).toHaveBeenCalledTimes(1); - expect(log).toHaveBeenCalledTimes(loader.retries); + expect(console.log).toHaveBeenCalledTimes(loader.retries); }); test("singleton should be used", () => {