JMPerez / spotify-web-api-js

A client-side JS wrapper for the Spotify Web API
https://jmperezperez.com/spotify-web-api-js/
MIT License
1.86k stars 260 forks source link

TypeScript: `getUserPlaylists()` ignores `options` when `userId` is `undefined` #181

Open laurence-myers opened 4 years ago

laurence-myers commented 4 years ago

In TypeScript, when calling getUserPlaylists(), the options object is ignored when userId is passed as undefined.

const result = await spotify.getUserPlaylists(undefined, { limit: 50 });
console.log(result.length); // <-- 20, the default limit

This is because the method getUserPlaylists() has dynamic behaviour, re-assigning its arguments if the first argument is not a string. The current types for getUserPlaylists() don't reflect this beahviour.

Code in question:


  Constr.prototype.getUserPlaylists = function (userId, options, callback) {
    var requestData;
    if (typeof userId === 'string') {
      requestData = {
        url: _baseUri + '/users/' + encodeURIComponent(userId) + '/playlists'
      };
    } else {
      requestData = {
        url: _baseUri + '/me/playlists'
      };
      callback = options;
      options = userId;
    }
    return _checkParamsAndPerformRequest(requestData, options, callback);
  };

Types:

getUserPlaylists(
      userId?: string,
      options?: Object,
      callback?: ResultsCallback<SpotifyApi.ListOfUsersPlaylistsResponse>
    ): Promise<SpotifyApi.ListOfUsersPlaylistsResponse>;

Possible fix

You could fix the types by overloading the function signature.

getUserPlaylists(
      options?: Object,
      callback?: ResultsCallback<SpotifyApi.ListOfUsersPlaylistsResponse>
    ): Promise<SpotifyApi.ListOfUsersPlaylistsResponse>;
getUserPlaylists(
      userId: string,
      options?: Object,
      callback?: ResultsCallback<SpotifyApi.ListOfUsersPlaylistsResponse>
    ): Promise<SpotifyApi.ListOfUsersPlaylistsResponse>;

You could also make the types more accurate by using an interface instead of the catch-all Object type for the options, and distinguishing between the Promise-returning and callback versions of the method.

interface PageOptions {
  limit?: number;
  offset?: number;
}

getUserPlaylists(
      options?: PageOptions,
    ): Promise<SpotifyApi.ListOfUsersPlaylistsResponse>;
getUserPlaylists(
      options: PageOptions | undefined,
      callback: ResultsCallback<SpotifyApi.ListOfUsersPlaylistsResponse>
    ): void;
getUserPlaylists(
      userId: string,
      options?: PageOptions,
    ): Promise<SpotifyApi.ListOfUsersPlaylistsResponse>;
getUserPlaylists(
      userId: string,
      options: PageOptions | undefined,
      callback: ResultsCallback<SpotifyApi.ListOfUsersPlaylistsResponse>
    ): void;
osnodegeoffrey commented 2 years ago

+1 to get this fixed