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

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

Open
laurence-myers opened this issue Aug 16, 2020 · 1 comment · May be fixed by #212
Open

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

laurence-myers opened this issue Aug 16, 2020 · 1 comment · May be fixed by #212

Comments

@laurence-myers
Copy link

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;
baizel added a commit to baizel/spotify-web-api-js that referenced this issue Aug 11, 2021
This issue was raised in JMPerez#181 but not yet fixed. This fork fixes this issue
@osnodegeoffrey
Copy link

+1 to get this fixed

schl3ck added a commit to schl3ck/spotify-web-api-scriptable that referenced this issue Jan 3, 2022
tkhduracell added a commit to tkhduracell/spotify-web-api-js that referenced this issue Feb 15, 2022
@tkhduracell tkhduracell linked a pull request Feb 15, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants