peter-murray / node-hue-api

Node.js Library for interacting with the Philips Hue Bridge and Lights
Apache License 2.0
1.19k stars 145 forks source link

Typescript support #135

Closed angeloalduino closed 4 years ago

angeloalduino commented 4 years ago

Hey there, just wanted to preface with the fact that this library is great and very convenient! Thank you!

I've been writing some home automation stuff and dabbling in typescript and I'm trying to figure out whether I'm misusing the library. When using the following declaration, I receive the backward compatibility shim warning:

import { HueApi } from 'node-hue-api';

Unfortunately, this is the only way I can seem to use the typed objects that the API provides. In order to avoid the warning, I have to use the require('node-hue-api').v3 declaration, but then I lose all typing and have to treat everything like a generic javascript object.

I believe this might be because the types that I installed via npm install @types/node-hue-api are outdated. Is this correct, and do you have plans to officially expose the type definitions for v4? It seems that these type definitions are potentially made by third parties, so it'd be understandable if you don't plan on supporting them.

In any case, thanks again for your efforts in creating this library!

peter-murray commented 4 years ago

You are correct, the existing types you are using were created by a third party that desired them at the time.

I am contemplating adding some type definitions, but I don’t want to get bogged down in typescript and toolchains as this is a pure JavaScript library. I personally am not a big fan of TypeScript, but I do understand that others prefer it.

The changes in the v3 release should make building and maintaining definitions easier, but there are still areas where the dynamic nature of the underlying hue api objects are exposed, making the types impossible to define or at least to remain current as users upgrade the underlying hue bridge firmware.

I will evaluate what is required for me to build and expose the TypeScript definitions and respond in this issue in the next week or so.

angeloalduino commented 4 years ago

I fully understand not wanting to add ts support, so I really appreciate any time you take if you add it in the future! If that doesn't happen it's no problem, I'll just look into generating types on my end or just making my own slim models and parsing logic for the data that I need. Cheers!

KnisterPeter commented 4 years ago

@peter-murray @angeloalduino Without much effort and with typescript 3.7 it could be done like described here: https://twitter.com/orta/status/1194373218775293952 https://github.com/jamestalmage/supports-hyperlinks/pull/6

peter-murray commented 4 years ago

Thanks, will take a look at that after I finish my current work on the model and resource links

peter-murray commented 4 years ago

I took a few minutes to run the command against the library to see what falls out:

npx typescript index.js --allowJs --declaration --emitDeclarationOnly

It does emit for most of the files, but there are problems with a few private objects in the old API shim and the new Rules for condition operators.

It also then rejects re-runs on if the previous *.d.ts files are there for Rules as that overrides a parent function call.

Looking at the generated definitions, they are not particularly useful other than to give you maybe code completion on the functions available on an object. The parameters are just about always any which to me makes it just about useless.

I have never wanted to develop in TypeScript, so maybe that is useful, but it all seems a little half baked if I was to do this, as if I was to hand code these you would surely not have so many any arguments, which is what I would have thought was the more desirable thing here, not just function name autocompletion?

I would be very happy to be educated on what a TypeScript consumer would be looking for here, as the auto generation does not seem to be enough.

KnisterPeter commented 4 years ago

I think this is a starting point if you write in js and want to provide types. Extend them by writing proper jsdoc and they will get better I think. Then they will be helpful.

peter-murray commented 4 years ago

Its funny as I removed the jsdoc when refactoring/rewriting for v3 as it was not useful, now it would be... 🤦‍♂

I will take a look at what I can do for the 4.x release that I am preparing as it has a number of changes to expose the model objects, which is where the definitions would be useful if they were correctly defined.

peter-murray commented 4 years ago

Ok, so a first swipe at TypeScript definitions being generated are available from npm in the @next tagged version (which is 4.0.0-alpha-2 at the time of writing this).

Note this version has the upcoming changes to the models and removal of the old v2 API,

Run this to install it into a project.

$ npm install node-hue-api@next

It required me to make a number of changes so that the generator would work and not complain on multiple runs, and there is some extra JSDoc added to support the generation of the types.

I am not massively keen on adding screeds of JSDoc to the code base, as I am finding it much harder to read the code where I have added it, although I could tweak the IDE settings to tone it down, it still blows up the files making it harder to work on.

I would appreciate any feedback on whether it is usable and what/where you would desire improvements. I am going to take a look at using it from a TypeScript project tomorrow.

KnisterPeter commented 4 years ago

I will have I look if I find a minute.

peter-murray commented 4 years ago

I have greatly expanded the JSDoc to increase the quality of the typings from the library, looking over them they are considerably better, but have a few warnings in IntelliJ IDEA.

I am yet to try an use them from a TypeScript project, but they are present in version 4.0.0-alpha-3 that I have just released as a preview.

KnisterPeter commented 4 years ago

@peter-murray The current types are a very good start. I'll try to improve them with small PRs from time to time if you second that. Thank you for setting this up.

peter-murray commented 4 years ago

Happy for any help on the types, as I don’t use them directly, so hard to validate them all.

peter-murray commented 4 years ago

Closing this issue as the types are now present in 4.0.0 and if there are other issues, they should be handled in PRs or another issue ticket

jimmywarting commented 11 months ago

Gosh, really don't like this typescript rewrites. it's fully possible to have type support without typescript flavored syntax...