fix the signatures of importLibrary

[vicb]

Thank you for opening a Pull Request!

The variadic signature is not implemented so it is not usable (the implementation would have to take a string[]) so I removed it.

With the 2 overrides, you can verify that:

const m = await importLibrary("maps");
const m2 = await importLibrary("not-maps");

m is a google.maps.MapsLibrary
m2 is unknown

---

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open a GitHub issue as a bug/feature request before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

[vicb]

Also I think the docs should be updated to remove the cast:

[usefulthink]

The variadic signature is not implemented so it is not usable (the implementation would have to take a string[]) so I removed it.

Maybe I misunderstand you, but what that signature actually does is define our importLibrary function as inheriting the exact type definition that exists for google.maps.importLibrary. Since that definition from @types/google.maps currently has some problems with unspecific return types, we added a more specific definition to make sure the return values are properly typed.

(note also that the ...parameters is there to match the type of Parameters<...> (which is [name: string] for the importLibrary function))

Our own implementation (APILibraryMap etc) will be removed once the @types/google.maps package has been updated with better typings.

[vicb]

@usefulthink

What I mean by "The variadic signature is not implemented" is that you can not importLibrary("a", "b", "c")
It should be easy enough to check?

[usefulthink]

Yes, but that's fully intentional. We wanted to stick to the rather minimalist implementation of dynamic library loading available here: https://developers.google.com/maps/documentation/javascript/load-maps-js-api, but have it in a way better suited for projects using bundlers etc.

Loading multiple libraries at once isn't trivial to solve due to the handling of the return values, and everything we looked at missed some important points or felt weird in some way.

If you need parts of different libraries, there's nothing wrong with loading them sequentially, or using Promise.all if you feel like it, but that should be up to the developer using this library.

One more thing: Thanks for the remark on the docs! I'll forward that to someone at google who can take care of that.

[vicb]

@usefulthink

We seem to agree that loading multiple libraries is not implemented, right?

The current code says it:

js-api-loader/src/index.ts

Lines 104 to 106 in 5e3bffb

	export async function importLibrary(
	...parameters: Parameters<typeof google.maps.importLibrary>
	): ReturnType<typeof google.maps.importLibrary>;

What this PR does is to remove that signature so that typescript correctly report that try to

importLibrary("a", "b");

becomes a type error.

Without this PR, TS is happy with importLibrary("a", "b"); but it obviously fails at runtime (at not implemented)
With this PR, TS reports that importLibrary("a", "b"); is invalid which I think is the right behavior.

As I added in the PR description, the following is now typed as expected:

With the 2 overrides, you can verify that:

const m = await importLibrary("maps");
const m2 = await importLibrary("not-maps");

m is a google.maps.MapsLibrary
m2 is unknown

So I believe the PR is right and should have been merged instead of being closed?

(say hello to Chris JS for me)

[usefulthink]

Here's a typescript playground isolating this part of our types.

What you can see there: as long as there's a working definition of google.maps.importLibrary, our importLibrary function will not accept multiple parameters. (Note that Parameters<...> will be an array with fixed length 1, and thus ...parameters will also have this fixed length and not allow a variable number of arguments.

Eventually, we want to solely rely on the types from the @types/google.maps package, which is why that declaration is there in the first place.

[vicb]

@usefulthink you are absolutely right? I just tested this again and it's WAI. I must have had another issue before (probably "as long as there's a working definition of google.maps.importLibrary").

Sorry for the noise!