search

bencevans / node-sonos

🔈 Sonos Media Player Interface/Client
https://www.npmjs.com/package/sonos
MIT License
704 stars 147 forks source link

Add types to node-sonos #512

Open realwakils opened 3 years ago

realwakils commented 3 years ago

Hello, I'm adding this issue to raise awareness about the desire of not only me, but I'm sure for others, for node-sonos.

Tbh i'm not personally very familiar with adding types to packages, but I might work on it.

Anyways if anyone else wants to, you can refer to this issue

(NOTE: I can't label this as a 'enhancement' or 'feature request'?)

svrooij commented 3 years ago

Your question is understandable, but this library is build in JavaScript and not in typescript. Personally I have no experience on how to add types to an existing library. Is there a typescript command to do that?

apart from that, as a contributor to this library a few year ago I made a rewrite in Typescript. Maybe that solves your issue? https://github.com/svrooij/node-sonos-ts

I think @bencevans would probably accept a PR to add the typing file, if there is also a script to generate them from the current source.

realwakils commented 3 years ago

Your question is understandable, but this library is build in JavaScript and not in typescript. Personally I have no experience on how to add types to an existing library. Is there a typescript command to do that?

apart from that, as a contributor to this library a few year ago I made a rewrite in Typescript. Maybe that solves your issue? https://github.com/svrooij/node-sonos-ts

I think @bencevans would probably accept a PR to add the typing file, if there is also a script to generate them from the current source.

Hi and thank you for your comment. Interesting library but perhaps it isn't updated and has the same features as this one? Anyways I definetly think that there should be types for this specific library anyways.

I did some research and found out that people add these types via DefinetlyTyped.

I have added SOME of the types but because I'm not too familiar with this library, I don't think I can move on further and also I have no idea how to actually run the tests for the types and so the experience of adding types with DefinetlyTyped has left me quite confused.

I thought that my types could potentially still be used though, so I have provided them down here. Beaware that they are not finished and might not even be correct. I hope some more familiar, maybe @bencevans, would want to finish/add to the types.

Thank you, wakils

https://github.com/realwakils/DefinitelyTyped/commit/7db80cab2420b5321e893a83333301b074a10923

svrooij commented 3 years ago

@realwakils you should check out the other library. It has at least all the features this library has.

My opinion about the types, it's better to have no types (like at the moment) then have incomplete/incorrect types, like you're proposing.

It should be possible to generate those, but creating them manually and not having them in the library is a bad idea, especially if they are incomplete. Then you get all those strange errors that you're expecting a method to return something but your types tell it it's just a void.

realwakils commented 3 years ago

@realwakils you should check out the other library. It has at least all the features this library has.

My opinion about the types, it's better to have no types (like at the moment) then have incomplete/incorrect types, like you're proposing.

It should be possible to generate those, but creating them manually and not having them in the library is a bad idea, especially if they are incomplete. The. You get all those strange errors that you're expecting a method to return something but your types tell it it's just a void.

Cool I will check it out. Yes I see your concerns in how the DefinetlyTyped library would have to follow along with this one or it could cause huge confusion. Personally though I think that adding types should be a large priority because for me (and I know for many others), Typescript speeds up my workflow alot and having to look at typings online in different docs is really just pain.

Anyways I will keep this issue open in case anyone wants to continue the discussion.

svrooij commented 3 years ago

One more question, what is the reason to put the types in a different repository? That makes it harder to manage.

This should be top priority

The maintainers make up the priority, you could also contribute these types and they will probably added to the package. Next to that there is a typescript library available, so the priority to add them to this library probably isn't very high.

But as said having incomplete/incorrect typings is even worse then no types. So this should be incorporated into maintaining this library.

svrooij commented 3 years ago

@realwakils I just found a typescript reference on how to generate types from JavaScript. It requires to add jsdoc comments to a lot of items.

https://www.typescriptlang.org/docs/handbook/declaration-files/dts-from-js.html

I would be happy to add all the scripting and the stuff in the package.json, but annotating all the variables will be a tedious task. I could definitely use some help with that.

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 1.15.0-alpha.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket: