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

Integrated typescript typings #44

Merged
merged 5 commits into from
Oct 20, 2016
Merged

Integrated typescript typings #44

merged 5 commits into from
Oct 20, 2016

Conversation

skovmand
Copy link
Contributor

@skovmand skovmand commented Oct 20, 2016

Hi José.

Here's a pull request that integrates Typescript typings for both the spotify-web-api-js library and the entire spotify web api into the library.

This is a big improvement over the Typescript typings I contributed in january. The typings are updated to include all new endpoints in the Spotify Web API and adds typings for the new methods in the library. Also, by including typings in the library, the user always has the correct typings and doesn't need to browse the typings repositories (DefinitelyTyped, et.al.)

Also, the README.md is updated with an animated GIF as example as well as instructions on how to import the library in Typescript globally or as a module.

I run a project for sports music in Denmark, https://idraetsmusik.dk and use the Spotify Web Api a lot. I find the typings help me a lot during development and I would like to continue keeping them updated in this library.

I hope you can use this contribution and that you have time to review the pull request. Please send me any suggestions for modifications.

Regards,
Niels K.H. Skovmand.

@coveralls
Copy link

coveralls commented Oct 20, 2016

Coverage Status

Coverage remained the same at 97.973% when pulling 9c5423d on skovmand:integrated-typescript-typings into c1e17c6 on JMPerez:master.

@coveralls
Copy link

coveralls commented Oct 20, 2016

Coverage Status

Coverage remained the same at 97.973% when pulling 9ff6bc3 on skovmand:integrated-typescript-typings into c1e17c6 on JMPerez:master.

@coveralls
Copy link

coveralls commented Oct 20, 2016

Coverage Status

Coverage remained the same at 97.973% when pulling 3955e3b on skovmand:integrated-typescript-typings into c1e17c6 on JMPerez:master.

@JMPerez
Copy link
Owner

JMPerez commented Oct 20, 2016

This is the most awesome PR I have seen in a long time. Thank you so much @skovmand!!

giphy

@JMPerez JMPerez merged commit 335140d into JMPerez:master Oct 20, 2016
@skovmand
Copy link
Contributor Author

Thanks! And thanks for the Chuck Norris approval :-)

@lnunno
Copy link

lnunno commented Sep 9, 2017

@skovmand Maybe I'm being dense, but is there anyway to access these types external to the SpotifyWebApiJs namespace?

For example I'd like to store certain things in React state e.g. something like:

import * as SpotifyWebApi from 'spotify-web-api-js';

interface AppState {
  spotify: SpotifyWebApi.SpotifyApi;
  playlist: SpotifyWebApi.PlaylistBaseObject;
}

However these interfaces are not exported from this namespace, was this intentional?

@lnunno
Copy link

lnunno commented Sep 9, 2017

D'oh totally figured this out. It's just

import * as SpotifyWebApi from 'spotify-web-api-js';

interface AppState {
  spotify: SpotifyApi.ListOfUsersPlaylistsResponse;
  playlist: SpotifyApi.PlaylistObjectSimplified;
}

Still getting used to how Typescript does namespaces.

@skovmand
Copy link
Contributor Author

Great to hear that. I was just about to investigate your problem.

@rshkv
Copy link

rshkv commented Oct 13, 2017

The SpotifyApi.* types work for me. But I can't use the type of the SpotifyWebApi object itself.
I'm trying something like:

function authorize(api: SpotifyWebApiJs.SpotifyWebApiJs): void {
    const token = "...";
    api.setAccessToken(token);
}

@skovmand, is there anything I need to import first?

@lnunno
Copy link

lnunno commented Oct 17, 2017

@rshkv I am unable to use that type as well.

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 this pull request may close these issues.

5 participants