Repository: googlemaps/js-api-loader
Pull request #797 (2023-05-31 07:00:37)

feat: Adopt the inline bootstrap loader

https://developers.google.com/maps/documentation/javascript/load-maps-js-api#dynamic-library-import

chrisjshull: My goal was to make this a non-breaking change. Can you elaborate on the breakage you are seeing?
amuramoto: Ah I see, I had thought the libraries specified in `LoaderOptions` wouldn't be available until after `importLibrary` was called, because I wasn't aware of the `libraries` options prop of the inline loader that you are able to pass those through to.

One question since I haven't been able to test this manually yes - if a user is still using `Loader.load()`, is `google.maps.Map` available when the callback in invoked? I ask because when using the inline loader script outside this library, you need to specify `libraries: ['maps']` in the script to have `google.maps.Map` available on load.
chrisjshull: > One question since I haven't been able to test this manually yes - if a user is still using `Loader.load()`, is `google.maps.Map` available when the callback in invoked? 

It should be...

> I ask because when using the inline loader script outside this library, you need to specify `libraries: ['maps']` in the script to have `google.maps.Map` available on load.

That's not how it's intended to function. Gave it a quick test: https://jsfiddle.net/q1xe8fs0/ - google.maps.Map is available as soon as any part of maps JS is loaded. Happy to investigate a sample that demonstrates otherwise.

Aside: are there plans to fix the e2e tests (#795) soon? Happy to wait for that if we want to increase confidence. So far my testing has just been the unit tests.
amuramoto: > That's not how it's intended to function. Gave it a quick test: https://jsfiddle.net/q1xe8fs0/ - google.maps.Map is available as soon as any part of maps JS is loaded. Happy to investigate a sample that demonstrates otherwise.

If I remove `await google.maps.importLibrary("core");` from that jsfiddle, then `google.maps.Map` is not available. AFAIK, this is the expected behavior from the inline API loader, but doesn't that then introduce a breaking change into this lib? 

If I upgrade after this PR, I would have to explicitly add `CoreLibrary` or `MapLibrary` to the `libraries` prop in the bootstrap, then load one of those with `importLibrary`. This would be so the `google.maps.Map` is available by default at load time, which is the current behavior. (or it could be that `CoreLibrary` would be the right choice here? Maybe `MapLibrary` doesn't provide the same APIs on load as the old loader  - I haven't done a comprehensive comparison between them.)

So existing code that looks like this:

```javascript
const loader = new Loader({
  apiKey: "myKey",
  libraries: ["places"]
});

loader
  .load()
  .then((google) => {
    new google.maps.Map(document.getElementById("map"), mapOptions);
  });
``` 

would instead have to look like this:

```javascript
const loader = new Loader({
  apiKey: "myKey",
  libraries: ["places, maps"] // or "core"
});

loader
  .load()
  .then(async (google) => {
    await google.maps.importLibrary("maps"); // or core
    new google.maps.Map(document.getElementById("map"), mapOptions);
  });
``` 

> Aside: are there plans to fix the e2e tests (#795) soon? Happy to wait for that if we want to increase confidence. So far my testing has just been the unit tests.

I'll try to take a look soon, since I'd like to get this PR merged. Since this is going to change the lib to only use the inline loader script under the hood, can you also take a look at `/examples` and update whatever is needed there?
chrisjshull: 
> If I remove `await google.maps.importLibrary("core");` from that jsfiddle, then `google.maps.Map` is not available. AFAIK, this is the expected behavior from the inline API loader, but doesn't that then introduce a breaking change into this lib?

That's because the inline bootstrap loader doesn't load anything until the first importLibrary call. That's why this PR includes a call to `loader.importLibrary("core");` (really it could have been any library) inside `setScript()`, in order to maintain backwards compatibility. 



> If I upgrade after this PR, I would have to explicitly add `CoreLibrary` or `MapLibrary` to the `libraries` prop in the bootstrap, then load one of those with `importLibrary`. This would be so the `google.maps.Map` is available by default at load time, which is the current behavior. (or it could be that `CoreLibrary` would be the right choice here? Maybe `MapLibrary` doesn't provide the same APIs on load as the old loader - I haven't done a comprehensive comparison between them.)
> 
> So existing code that looks like this:
> 
> ```js
> const loader = new Loader({
>   apiKey: "myKey",
>   libraries: ["places"]
> });
> 
> loader
>   .load()
>   .then((google) => {
>     new google.maps.Map(document.getElementById("map"), mapOptions);
>   });
> ```

Tested manually, this still works.

> Since this is going to change the lib to only use the inline loader script under the hood, can you also take a look at `/examples` and update whatever is needed there?

done

Patch:
diff --git a/src/index.ts b/src/index.ts
index c7b2160..42fe909 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -16,25 +16,24 @@
 
 import isEqual from "fast-deep-equal";
 
-/**
- * @ignore
- */
-declare global {
-  interface Window {
-    __googleMapsCallback: (e: Event) => void;
-  }
-}
-
 export const DEFAULT_ID = "__googleMapsScriptId";
 
-export type Libraries = (
-  | "drawing"
-  | "geometry"
-  | "localContext"
-  | "marker"
+// https://developers.google.com/maps/documentation/javascript/libraries#libraries-for-dynamic-library-import
+export type Library =
+  | "core"
+  | "maps"
   | "places"
-  | "visualization"
-)[];
+  | "geocoding"
+  | "routes"
+  | "marker"
+  | "geometry"
+  | "elevation"
+  | "streetView"
+  | "journeySharing"
+  | "drawing"
+  | "visualization";
+
+export type Libraries = Library[];
 
 /**
  * The Google Maps JavaScript API
@@ -268,7 +267,6 @@ export class Loader {
    */
   public readonly authReferrerPolicy: "origin";
 
-  private CALLBACK = "__googleMapsCallback";
   private callbacks: ((e: ErrorEvent) => void)[] = [];
   private done = false;
   private loading = false;
@@ -366,11 +364,12 @@ export class Loader {
    * CreateUrl returns the Google Maps JavaScript API script url given the [[LoaderOptions]].
    *
    * @ignore
+   * @deprecated
    */
   public createUrl(): string {
     let url = this.url;
 
-    url += `?callback=${this.CALLBACK}`;
+    url += `?callback=__googleMapsCallback`;
 
     if (this.apiKey) {
       url += `&key=${this.apiKey}`;
@@ -420,6 +419,7 @@ export class Loader {
 
   /**
    * Load the Google Maps JavaScript API script and return a Promise.
+   * @deprecated, use importLibrary() instead.
    */
   public load(): Promise<typeof google> {
     return this.loadPromise();
@@ -429,6 +429,7 @@ export class Loader {
    * Load the Google Maps JavaScript API script and return a Promise.
    *
    * @ignore
+   * @deprecated, use importLibrary() instead.
    */
   public loadPromise(): Promise<typeof google> {
     return new Promise((resolve, reject) => {
@@ -442,8 +443,39 @@ export class Loader {
     });
   }
 
+  /**
+   * See https://developers.google.com/maps/documentation/javascript/reference/top-level#google.maps.importLibrary
+   */
+  public importLibrary(name: "core"): Promise<google.maps.CoreLibrary>;
+  public importLibrary(name: "maps"): Promise<google.maps.MapsLibrary>;
+  public importLibrary(name: "places"): Promise<google.maps.PlacesLibrary>;
+  public importLibrary(
+    name: "geocoding"
+  ): Promise<google.maps.GeocodingLibrary>;
+  public importLibrary(name: "routes"): Promise<google.maps.RoutesLibrary>;
+  public importLibrary(name: "marker"): Promise<google.maps.MarkerLibrary>;
+  public importLibrary(name: "geometry"): Promise<google.maps.GeometryLibrary>;
+  public importLibrary(
+    name: "elevation"
+  ): Promise<google.maps.ElevationLibrary>;
+  public importLibrary(
+    name: "streetView"
+  ): Promise<google.maps.StreetViewLibrary>;
+  public importLibrary(
+    name: "journeySharing"
+  ): Promise<google.maps.JourneySharingLibrary>;
+  public importLibrary(name: "drawing"): Promise<google.maps.DrawingLibrary>;
+  public importLibrary(
+    name: "visualization"
+  ): Promise<google.maps.VisualizationLibrary>;
+  public importLibrary(name: Library): Promise<unknown> {
+    this.execute();
+    return google.maps.importLibrary(name);
+  }
+
   /**
    * Load the Google Maps JavaScript API script with a callback.
+   * @deprecated, use importLibrary() instead.
    */
   public loadCallback(fn: (e: ErrorEvent) => void): void {
     this.callbacks.push(fn);
@@ -460,20 +492,66 @@ export class Loader {
       return;
     }
 
-    const url = this.createUrl();
-    const script = document.createElement("script");
-    script.id = this.id;
-    script.type = "text/javascript";
-    script.src = url;
-    script.onerror = this.loadErrorCallback.bind(this);
-    script.defer = true;
-    script.async = true;
-
-    if (this.nonce) {
-      script.nonce = this.nonce;
+    if (!window?.google?.maps?.importLibrary) {
+      // tweaked copy of https://developers.google.com/maps/documentation/javascript/load-maps-js-api#dynamic-library-import
+      // which also sets the url, the id, and the nonce
+      /* eslint-disable */
+      ((g) => {
+        // @ts-ignore
+        let h,
+          a,
+          k,
+          p = "The Google Maps JavaScript API",
+          c = "google",
+          l = "importLibrary",
+          q = "__ib__",
+          m = document,
+          b = window;
+        // @ts-ignore
+        b = b[c] || (b[c] = {});
+        // @ts-ignore
+        const d = b.maps || (b.maps = {}),
+          r = new Set(),
+          e = new URLSearchParams(),
+          u = () =>
+            // @ts-ignore
+            h || (h = new Promise(async (f, n) => {
+              await (a = m.createElement("script"));
+              a.id = this.id;
+              e.set("libraries", [...r] + "");
+              // @ts-ignore
+              for (k in g) e.set(k.replace(/[A-Z]/g, (t) => "_" + t[0].toLowerCase()), g[k]);
+              e.set("callback", c + ".maps." + q);
+              a.src = this.url + `?` + e;
+              d[q] = f;
+              a.onerror = () => (h = n(Error(p + " could not load.")));
+              // @ts-ignore
+              a.nonce = this.nonce || m.querySelector("script[nonce]")?.nonce || "";
+              m.head.append(a);
+            }));
+        // @ts-ignore
+        d[l] ? console.warn(p + " only loads once. Ignoring:", g) : (d[l] = (f, ...n) => r.add(f) && u().then(() => d[l](f, ...n)));
+      })({
+        key: this.apiKey,
+        channel: this.channel,
+        client: this.client,
+        libraries: this.libraries,
+        v: this.version,
+        mapIds: this.mapIds,
+        language: this.language,
+        region: this.region,
+        authReferrerPolicy: this.authReferrerPolicy,
+      });
+      /* eslint-enable */
     }
 
-    document.head.appendChild(script);
+    this.importLibrary("core").then(
+      () => this.callback(),
+      (error) => {
+        const event = new ErrorEvent("error", { error }); // for backwards compat
+        this.loadErrorCallback(event);
+      }
+    );
   }
 
   /**
@@ -499,7 +577,7 @@ export class Loader {
     if (this.errors.length <= this.retries) {
       const delay = this.errors.length * 2 ** this.errors.length;
 
-      console.log(
+      console.error(
         `Failed to load Google Maps script, retrying in ${delay} ms.`
       );
 
@@ -513,10 +591,6 @@ export class Loader {
     }
   }
 
-  private setCallback(): void {
-    window.__googleMapsCallback = this.callback.bind(this);
-  }
-
   private callback(): void {
     this.done = true;
     this.loading = false;
@@ -548,7 +622,7 @@ export class Loader {
         // do nothing but wait
       } else {
         this.loading = true;
-        this.setCallback();
+
         this.setScript();
       }
     }


Test patch:
diff --git a/src/index.test.ts b/src/index.test.ts
index e69e716..75f03a4 100644
--- a/src/index.test.ts
+++ b/src/index.test.ts
@@ -21,6 +21,7 @@ jest.useFakeTimers();
 afterEach(() => {
   document.getElementsByTagName("html")[0].innerHTML = "";
   delete Loader["instance"];
+  if (window.google) delete window.google;
 });
 
 test.each([
@@ -65,59 +66,59 @@ 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", () => {
+test("setScript adds a script to head with correct attributes", async () => {
   const loader = new Loader({ apiKey: "foo" });
 
   loader["setScript"]();
+  await 0;
 
   const script = document.head.childNodes[0] as HTMLScriptElement;
 
   expect(script.id).toEqual(loader.id);
-  expect(script.src).toEqual(loader.createUrl());
-  expect(script.defer).toBeTruthy();
-  expect(script.async).toBeTruthy();
-  expect(script.onerror).toBeTruthy();
-  expect(script.type).toEqual("text/javascript");
 });
 
-test("setScript does not add second script with same id", () => {
-  new Loader({ apiKey: "foo", id: "bar" })["setScript"]();
-  new Loader({ apiKey: "foo", id: "bar" })["setScript"]();
-
-  expect(document.head.childNodes.length).toBe(1);
-});
-
-test("setScript adds a script with id", () => {
+test("setScript adds a script with id", async () => {
   const loader = new Loader({ apiKey: "foo", id: "bar" });
   loader["setScript"]();
+  await 0;
 
   const script = document.head.childNodes[0] as HTMLScriptElement;
-  expect(script.id).toEqual(loader.id);
+  expect(script.localName).toEqual("script");
+  expect(loader.id).toEqual("bar");
+  expect(script.id).toEqual("bar");
+});
+
+test("setScript does not add second script with same id", async () => {
+  new Loader({ apiKey: "foo", id: "bar" })["setScript"]();
+  new Loader({ apiKey: "foo", id: "bar" })["setScript"]();
+  await 0;
+  new Loader({ apiKey: "foo", id: "bar" })["setScript"]();
+  await 0;
+
+  expect(document.head.childNodes.length).toBe(1);
 });
 
 test("load should return a promise that resolves even if called twice", () => {
   const loader = new Loader({ apiKey: "foo" });
+  loader.importLibrary = (() => Promise.resolve()) as any;
 
   expect.assertions(1);
   const promise = Promise.all([loader.load(), loader.load()]).then(() => {
     expect(loader["done"]).toBeTruthy();
   });
 
-  window.__googleMapsCallback(null);
-
   return promise;
 });
 
 test("loadCallback callback should fire", () => {
   const loader = new Loader({ apiKey: "foo" });
+  loader.importLibrary = (() => Promise.resolve()) as any;
 
   expect.assertions(2);
   loader.loadCallback((e: Event) => {
     expect(loader["done"]).toBeTruthy();
     expect(e).toBeUndefined();
   });
-
-  window.__googleMapsCallback(null);
 });
 
 test("script onerror should reject promise", async () => {
@@ -163,74 +164,70 @@ test("script onerror should reject promise with multiple loaders", async () => {
 test("script onerror should retry", async () => {
   const loader = new Loader({ apiKey: "foo", retries: 1 });
   const deleteScript = jest.spyOn(loader, "deleteScript");
+  loader.importLibrary = (() => Promise.reject(new Error("fake error"))) as any;
   const rejection = expect(loader.load()).rejects.toBeInstanceOf(Error);
   // eslint-disable-next-line @typescript-eslint/no-empty-function
-  console.log = jest.fn();
+  console.error = jest.fn();
 
-  loader["loadErrorCallback"](
-    new ErrorEvent("ErrorEvent(", { error: new Error("") })
-  );
-  loader["loadErrorCallback"](
-    new ErrorEvent("ErrorEvent(", { error: new Error("") })
-  );
+  // wait for the first failure
+  await 0;
+  expect(loader["errors"].length).toBe(1);
+  // trigger the retry delay:
   jest.runAllTimers();
 
   await rejection;
+  expect(loader["errors"].length).toBe(2);
   expect(loader["done"]).toBeTruthy();
+  expect(loader["failed"]).toBeTruthy();
   expect(loader["loading"]).toBeFalsy();
-  expect(loader["errors"].length).toBe(2);
   expect(deleteScript).toHaveBeenCalledTimes(1);
-  expect(console.log).toHaveBeenCalledTimes(loader.retries);
+  expect(console.error).toHaveBeenCalledTimes(loader.retries);
 });
 
 test("script onerror should reset retry mechanism with next loader", async () => {
   const loader = new Loader({ apiKey: "foo", retries: 1 });
   const deleteScript = jest.spyOn(loader, "deleteScript");
+  loader.importLibrary = (() => Promise.reject(new Error("fake error"))) as any;
   // eslint-disable-next-line @typescript-eslint/no-empty-function
-  console.log = jest.fn();
+  console.error = jest.fn();
 
   let rejection = expect(loader.load()).rejects.toBeInstanceOf(Error);
-  loader["loadErrorCallback"](
-    new ErrorEvent("ErrorEvent(", { error: new Error("") })
-  );
-  loader["loadErrorCallback"](
-    new ErrorEvent("ErrorEvent(", { error: new Error("") })
-  );
+  // wait for the first first failure
+  await 0;
+  expect(loader["errors"].length).toBe(1);
+  // trigger the retry delay:
   jest.runAllTimers();
   await rejection;
 
+  // try again...
   rejection = expect(loader.load()).rejects.toBeInstanceOf(Error);
   expect(loader["done"]).toBeFalsy();
+  expect(loader["failed"]).toBeFalsy();
   expect(loader["loading"]).toBeTruthy();
   expect(loader["errors"].length).toBe(0);
 
-  loader["loadErrorCallback"](
-    new ErrorEvent("ErrorEvent(", { error: new Error("") })
-  );
-  loader["loadErrorCallback"](
-    new ErrorEvent("ErrorEvent(", { error: new Error("") })
-  );
+  // wait for the second first failure
+  await 0;
+  expect(loader["errors"].length).toBe(1);
+  // trigger the retry delay:
   jest.runAllTimers();
 
   await rejection;
   expect(deleteScript).toHaveBeenCalledTimes(3);
-  expect(console.log).toHaveBeenCalledTimes(loader.retries * 2);
+  expect(console.error).toHaveBeenCalledTimes(loader.retries * 2);
 });
 
 test("script onerror should not reset retry mechanism with parallel loaders", async () => {
   const loader = new Loader({ apiKey: "foo", retries: 1 });
   const deleteScript = jest.spyOn(loader, "deleteScript");
+  loader.importLibrary = (() => Promise.reject(new Error("fake error"))) as any;
   // eslint-disable-next-line @typescript-eslint/no-empty-function
-  console.log = jest.fn();
+  console.error = jest.fn();
 
   const rejection1 = expect(loader.load()).rejects.toBeInstanceOf(Error);
   const rejection2 = expect(loader.load()).rejects.toBeInstanceOf(Error);
-  loader["loadErrorCallback"](
-    new ErrorEvent("ErrorEvent(", { error: new Error("") })
-  );
-  loader["loadErrorCallback"](
-    new ErrorEvent("ErrorEvent(", { error: new Error("") })
-  );
+  // wait for the first first failure
+  await 0;
   jest.runAllTimers();
 
   await Promise.all([rejection1, rejection2]);
@@ -238,7 +235,7 @@ test("script onerror should not reset retry mechanism with parallel loaders", as
   expect(loader["loading"]).toBeFalsy();
   expect(loader["errors"].length).toBe(2);
   expect(deleteScript).toHaveBeenCalledTimes(1);
-  expect(console.log).toHaveBeenCalledTimes(loader.retries);
+  expect(console.error).toHaveBeenCalledTimes(loader.retries);
 });
 
 test("reset should clear state", () => {
@@ -288,12 +285,12 @@ test("failed getter should be correct", () => {
   expect(loader["failed"]).toBeTruthy();
 });
 
-test("loader should not reset retry mechanism if successfully loaded", () => {
+test("loader should not reset retry mechanism if successfully loaded", async () => {
   const loader = new Loader({ apiKey: "foo", retries: 0 });
   const deleteScript = jest.spyOn(loader, "deleteScript");
+  loader.importLibrary = (() => Promise.resolve()) as any;
 
-  loader["done"] = true;
-  expect(loader.load()).resolves.toBeUndefined();
+  await expect(loader.load()).resolves.not.toBeUndefined();
 
   expect(loader["done"]).toBeTruthy();
   expect(loader["loading"]).toBeFalsy();
@@ -344,10 +341,11 @@ test("loader should wait if already loading", () => {
   loader.load();
 });
 
-test("setScript adds a nonce", () => {
+test("setScript adds a nonce", async () => {
   const nonce = "bar";
   const loader = new Loader({ apiKey: "foo", nonce });
   loader["setScript"]();
+  await 0;
   const script = document.head.childNodes[0] as HTMLScriptElement;
   expect(script.nonce).toBe(nonce);
 });
@@ -371,9 +369,10 @@ test("loader should not warn if done and google.maps is defined", async () => {
   expect(console.warn).toHaveBeenCalledTimes(0);
 });
 
-test("deleteScript removes script tag from head", () => {
+test("deleteScript removes script tag from head", async () => {
   const loader = new Loader({ apiKey: "foo" });
   loader["setScript"]();
+  await 0;
   expect(document.head.childNodes.length).toBe(1);
   loader.deleteScript();
   expect(document.head.childNodes.length).toBe(0);
@@ -381,3 +380,14 @@ test("deleteScript removes script tag from head", () => {
   loader.deleteScript();
   expect(document.head.childNodes.length).toBe(0);
 });
+
+test("importLibrary resolves correctly", async () => {
+  window.google = { maps: {} } as any;
+  google.maps.importLibrary = async (name) => ({ [name]: "fake" } as any);
+
+  const loader = new Loader({ apiKey: "foo" });
+  const corePromise = loader.importLibrary("core");
+
+  const core = await corePromise;
+  expect(core).toEqual({ core: "fake" });
+});