WIP Add types - Githubissues

alexbosworth / ln-service

Node.js interface to LND

MIT License

318 stars 61 forks source link

WIP Add types #135

Closed bkiac closed 3 years ago

bkiac commented 3 years ago

I started working on TypeScript definitions for this package based on the documentation.

I think I reached a solid stage and I would like to get your input on how to move forward. I've noticed there are few types in the new https://github.com/alexbosworth/lightning package, but they are incomplete, and because I needed a few methods from this library I decided to work on this repo for now.

The AuthenticatedLND and UnauthenticatedLND objects are unfinished, but the rest of the methods should have a type:

[ ] AuthenticatedLND API Object properties
[ ] UnauthenticatedLND API Object properties

I have a few notes, questions about how to continue:

Some of the methods are just reexported from lightning, should their type definitions be moved there also?
I defined all of the types in a huge index.d.ts, but it might make sense to split them up, what do you think?
I refrained from creating common types and relied solely on the documentation, but in later versions reusable types for channels, policies, routes, hops, etc. could be defined.

A few technical questions:

I wasn't confident what the return types are of some of the functions which have only @returns via cbk or Promise in the docs. I assumed their return type to be void, is this correct?
In subscribeToChainAddress the lnd arg is a Chain RPC LND gRPC API Object, is this different from the Authenticated LND API Object which is used in the other methods?

(I also fixed a few typos and missing details in the documentation.)

Related to #112

alexbosworth commented 3 years ago

This looks great but can you add it to the alexbosworth/lightning repo? I'd like to make that repo the more typescript friendly one

If a method doesn't have a return object, yes it should return cbk() so undefined/nothing

Chain RPC LND is the same as Authenticated LND, it just means that the LND must be built with the ChainRPC flag

alexbosworth commented 3 years ago

Most of the methods of ln-service are already imported from alexbosworth/lightning - look in lightning/lnd-methods

If they have types I think I can bump them up to the top level index.js

alexbosworth commented 3 years ago

I guess I do agree that if it is possible to split up the type definitions a bit that could be easier to deal with

bkiac commented 3 years ago

This looks great but can you add it to the alexbosworth/lightning repo? I'd like to make that repo the more typescript friendly one

Sure, I will open a PR there soon as well. I didn't start there because I wasn't sure what is the scope of that library and I'm currently using ln-service. Will lightning replace this one eventually?

alexbosworth commented 3 years ago

This looks great but can you add it to the alexbosworth/lightning repo? I'd like to make that repo the more typescript friendly one

Sure, I will open a PR there soon as well. I didn't start there because I wasn't sure what is the scope of that library and I'm currently using ln-service. Will lightning replace this one eventually?

Cool, yes I am moving all the lnd methods out of this repo, but I am trying to maintain higher unit-test coverage on that repository, and I want typescript types there as well. This repo will focus more on integration tests and aggregating a methods from various libraries, like I already moved out the invoice parsing and creation to https://github.com/alexbosworth/invoices

alexbosworth commented 3 years ago

typescript typed methods live in https://www.npmjs.com/package/lightning now