Skip to content
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.

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

Implement proper type mapping for importLibrary() #95

Open
usefulthink opened this issue Mar 6, 2024 · 1 comment
Open

Implement proper type mapping for importLibrary() #95

usefulthink opened this issue Mar 6, 2024 · 1 comment
Labels
triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@usefulthink
Copy link

Problem

The definition of the importLibrary function returns a union-type of all the possible libraries, which means that users will always need to explicitly type-cast the results to get a proper type for the library. In JS these type-casts aren't really an option, so JS users don't get proper IDE support at all. Also, with the parameter being typed as "string", there's no validation for correct library names.

js-types/index.d.ts

Lines 7358 to 7373 in 32725e7

export function importLibrary(
libraryName: string,
): Promise<
| google.maps.CoreLibrary
| google.maps.MapsLibrary
| google.maps.PlacesLibrary
| google.maps.GeocodingLibrary
| google.maps.RoutesLibrary
| google.maps.MarkerLibrary
| google.maps.GeometryLibrary
| google.maps.ElevationLibrary
| google.maps.StreetViewLibrary
| google.maps.JourneySharingLibrary
| google.maps.DrawingLibrary
| google.maps.VisualizationLibrary
>;

Possible solution

Add explicit typing for the importLibrary function, such that the returned library type is inferred from the parameter.
This will also automatically add validation for the library name parameter.

A good way to do this would be (typescript playground link):

interface ApiLibraryMap {
  core: google.maps.CoreLibrary;
  maps: google.maps.MapsLibrary;
  // ...
}

type ApiLibraryName = keyof ApiLibraryMap;
export async function importLibrary<K extends ApiLibraryName>(name: K): Promise<ApiLibraryMap[K]>;

This is the pattern that is used in the typescript DOM library for e.g. element types (for example to determine the correct return type of document.createElement).

By using an Interface to define the mapping from library name to implementation, the library declarations can be done throughout the .d.ts file and will use declaration-merging to create the final ApiLibraryMap.
This might also help with your generated structure here, since the individual libraries can be specified where they are defined. For example:

  // https://github.com/googlemaps/js-types/blob/32725e71a1cfc3fc8ea4e72d08368285ccb3261f/index.d.ts#L450C1-L451C1
  export interface CoreLibrary {
    // ...
  }
  // just add the core-library to the 'global' ApiLibraryMap
  interface ApiLibraryMap {
    core: CoreLibrary;
  }

  // https://github.com/googlemaps/js-types/blob/32725e71a1cfc3fc8ea4e72d08368285ccb3261f/index.d.ts#L4282
  export interface MapsLibrary {
    // ...
  }
  interface ApiLibraryMap {
    maps: MapsLibrary;
  }
@usefulthink usefulthink added triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Mar 6, 2024
@usefulthink
Copy link
Author

I have updated my typescript playground to address the concern that my previous suggestion wasn't backwards compatible.

To achieve full BC, multiple signatures for the importLibrary function can be declared like this:

type ApiLibraryName = keyof ApiLibraryMap;

// this is the existing return-type of `importLibrary`
type AnyLibrary = 
  | google.maps.CoreLibrary
  | google.maps.MapsLibrary
  | google.maps.PlacesLibrary
  | google.maps.MarkersLibrary
  | /* ... */;

// narrow signature returns a single library type based on the value of the name parameter:
export async function importLibrary<K extends ApiLibraryName>(name: K): Promise<ApiLibraryMap[K]>;

// backwards compatible signature; this is identical to the one available today:
export async function importLibrary(name:string): Promise<AnyLibrary>;

(the playground also contains some example code to demonstrate the backward-compatibility)

This would allow you to add the narrow definition for the importLibrary function and an empty ApiLibraryMap interface at first, and then incrementally add libraries to the ApiLibraryMap, without any problems breaking BC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

1 participant