sanketbajoria / ssh2-promise

ssh with promise/async await and typescript support
https://www.npmjs.com/package/ssh2-promise
MIT License
148 stars 25 forks source link

TypeScript: SSHConnections Missing Properties #12

Closed MutableLoss closed 6 years ago

MutableLoss commented 6 years ago

Version: 0.1.1

I was checking this library out for a TS project, and noticed the ssh.shell method prop could not be found:

screen shot 2018-10-21 at 20 41 25

By the way, this happens with both ssh.exec, and ssh.shell (naturally).

While I'm very use to JS, I'm not too use to TS so this could be user error, in which I apologize.

I did manually reference the sshConnections.d file, to which nothing changed leading me to believe the above was correct. While it may not be needed, I do have the @types/ssh2 installed.

I'm curious, is there a reason the type defs aren't being run through DefinitelyTyped? It seems everything else works regardless, but I guess to me it's nice to know where they are and if there is anything I need to add to the compiler types. Is there a possibility of adding a note about this in the README?

Anyways, thanks for this library! It looks great so far. 👍

martindandanell commented 6 years ago

If you use new SSH2Promise.SSH instead of new SSH2Promise directly, it will give you an instance of the SSHConnection object in which the exec and shell functions exists - at least this worked for me.

sanketbajoria commented 6 years ago

@3DEsprit , ah, i can see typescript property is missing. I am fixing this up. Will be fixed by next release

@martindandanell Actually, if we use in above manner, then it won't be providing the feature of connection hopping, which this library intend to do.

MutableLoss commented 6 years ago

Thanks @sanketbajoria !

MutableLoss commented 6 years ago

It looks like there are a few other missing type definitions here, like shell, spawn, getSocksPort, addTunnel, getTunnel, x11, and subsys. Adding each method's signature definition to the index.d.ts has at least allowed me to use each without much issue.

Also, it seems the current signature for the shell method in sshConnections is set with mandatory options, instead of the optional options parameter setup referenced in the docs. Did this change?

sanketbajoria commented 6 years ago

@3DEsprit Fixed this issue, now, it should provide typescript support for all missing type definition.

MutableLoss commented 6 years ago

I'm guessing you already know, but doesn't look like the change was applied. I'm not seeing the update in the repo, or the npm package yet.

sanketbajoria commented 6 years ago

@3DEsprit Yes, i have created a patch on a separate branch. Haven't pushed it yet on master branch or npm repository. Can you please test it at your end, by just specifying github ssh2-promise branch entry in package.json file. Once, you confirm it, i will push the changes branch name -- issue12

MutableLoss commented 6 years ago

Looks good! It looks like Shell is requiring the options, instead of being optional.

sanketbajoria commented 6 years ago

Ah, good catch.. fixed it.

MutableLoss commented 6 years ago

Looking good on this end. Thanks! 👍

sanketbajoria commented 6 years ago

Published to master repository and npm repository Thanks

MutableLoss commented 6 years ago

Any time! Closing.