bencevans / node-sonos

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

Typescript types #519

Closed svrooij closed 2 years ago

svrooij commented 2 years ago

I've managed to get started with generating Typescript definition files from existing Javascript code I followed this manual.

This way everybody that likes to use the Sonos library can also enjoy the comfort of automatic documentation.

This PR includes a first version of the type definition. The types are also generated in the prepack stage, so they will never become outdated.

Improving the type definition can be done by adding jsdoc comments to the javascript code.

Check this sample

https://github.com/svrooij/node-sonos/blob/9a87007e3aa81886fca274ed301a64f3bb7ff2ab/lib/services/alarm-clock.service.js#L30-L45

Fixed #512 @realwakils What do you think @bencevans

svrooij commented 2 years ago

@pascalopitz can you check if you application still works with all these types now included. I've tried to not break anything and just including a first version of the types.

Installing with npm install svrooij/node-sonos#feature/types --save (I guess, never installed a package from git branch).

github-actions[bot] commented 2 years ago

:tada: This PR is included in version 1.15.0-alpha.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

pascalopitz commented 2 years ago

@svrooij Nope, not looking good for me

image

svrooij commented 2 years ago

@pascalopitz The AsyncDeviceDiscovery is the only thing I've changed. A promise cannot return multiple arguments, so I changed it to return an object with device and model. The model was missing in the async version.

if you change this to:

    async searchForDevices(timeout = 1000) {
        const discovered = await new AsyncDeviceDiscovery().discover({
            timeout,
            port: SONOS_DISCOVERY_PORT,
        });
        await this.connectDevice(discovered.device);
    },

Did this fix your app? Or are you facing tons of troubles?

Related code: https://github.com/bencevans/node-sonos/blob/bea996efa92b8519dc975596424f0c615dec6de7/lib/asyncDeviceDiscovery.js#L9-L21

pascalopitz commented 2 years ago

Confirmed. I just figured out the same ...

svrooij commented 2 years ago

Confirmed. I just figured out the same ...

If you're facing any other issues with the new alpha, could you create a new issue?