Skip to content

feat: add retries #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 13, 2020
Merged

feat: add retries #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19ece66
feat: add retries
jpoehnelt Nov 13, 2020
25b4ef1
style: fix lint issues
jpoehnelt Nov 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 41 additions & 4 deletions src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "";
Expand Down Expand Up @@ -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" });

Expand Down Expand Up @@ -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);

Expand All @@ -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"));
Expand All @@ -139,6 +146,25 @@ 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);
// 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();

await rejection;
expect(loader["done"]).toBeTruthy();
expect(loader["loading"]).toBeFalsy();
expect(loader["errors"].length).toBe(2);
expect(deleteScript).toHaveBeenCalledTimes(1);
expect(console.log).toHaveBeenCalledTimes(loader.retries);
});

test("singleton should be used", () => {
const loader = new Loader({ apiKey: "foo" });
const extraLoader = new Loader({ apiKey: "foo" });
Expand Down Expand Up @@ -195,3 +221,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);
});
51 changes: 45 additions & 6 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ declare global {
}
}

export const DEFAULT_ID = "__googleMapsScriptId";

type Libraries = (
| "drawing"
| "geometry"
Expand All @@ -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 {
/**
Expand Down Expand Up @@ -157,6 +161,10 @@ export interface LoaderOptions {
* Use a cryptographic nonce attribute.
*/
nonce?: string;
/**
* The number of script load retries.
*/
retries?: number;
}

/**
Expand Down Expand Up @@ -223,6 +231,11 @@ export class Loader {
*/
nonce: string | null;

/**
* See [[LoaderOptions.retries]]
*/
retries: number;

/**
* See [[LoaderOptions.url]]
*/
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -299,6 +315,7 @@ export class Loader {
url: this.url,
};
}

/**
* CreateUrl returns the Google Maps JavaScript API script url given the [[LoaderOptions]].
*
Expand Down Expand Up @@ -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;
Expand All @@ -402,9 +419,31 @@ export class Loader {
document.head.appendChild(script);
}

private loadErrorCallback(e: Event): void {
this.onerrorEvent = e;
this.callback();
deleteScript(): void {
Copy link
Contributor

@louisnow louisnow Nov 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpoehnelt While working on #102 I noticed this deleteScript function wasn't marked private. Was this intentional, is it a public facing API? I can update the docs if so. :)

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 {
Expand Down