Add Typescript support

[nazarhussain]

With reference to the issue https://github.com/libp2p/js-libp2p/issues/659 this PR will add typescript support to peer-id

[nazarhussain]

Two questions for the reviewer:

1. I am not sure if we really need the below code. What it's actually doing is creating isPeerId function on the class, but we have our own implementation of isPeerId so we really don't need it.

https://github.com/libp2p/js-peer-id/blob/42efbb57f188e2581f6f721b28bfdb6b58b37ae7/src/index.js#L497-L500

2. The implementation of get pubKey suggests it could be undefined. So if we use the right type fo get pubcKey as PublicKey | undefined it could be breaking change for users. If we force it to not undefined then type will not match the implementaiton. https://github.com/libp2p/js-peer-id/blob/e19da79f08f8dc6924a72a688ce563c7fd7b1989/src/index.js#L48-L67

Please suggest how to handle these two cases.

[vasco-santos]

Thanks for reaching out @nazarhussain

1. class-is

yes, it should be removed. We should have something similar to https://github.com/multiformats/js-multiaddr/blob/master/src/index.js#L550 where we also had before class-is before moving with the types work

2. public key

yes, sadly it can also return undefined. The story here is that not all types of key types allows us to compute the public key with the peerId. And you can create a PeerId just with the id. You mention this will break the manually implemented types in https://github.com/libp2p/js-peer-id/blob/master/src/index.d.ts#L118 right?

The answer here is that we need to make breaking changes as the types do not represent the truth and undefined can happen

[nazarhussain]

@vasco-santos PR is ready for review. Feel free to share your feedback. Just to be sure this PR contains breaking changes in terms of types not behaviour.

[vasco-santos]

@nazarhussain thanks, sorry for taking long to get here. I will review this tomorrow

[nazarhussain]

@vasco-santos Thanks for such a detail feedback. My intention was to make minimum change possible to existing code which does not involve making changes to the tests. Hopefully it looks better now.

Currently let with these linting warning/errors.

index.js:5:1: Unexpected 'todo' comment: 'TODO: Fix missing type'. [Warning/no-warning-comments]
index.js:69:5: Expected an assignment or function call and instead saw an expression. [Error/no-unused-expressions]
index.js:274:5: Unexpected 'todo' comment: 'TODO: needs better checking'. [Warning/no-warning-comments]
index.js:462:5: Unexpected 'todo' comment: 'TODO: val id and pubDigest'. [Warning/no-warning-comments]

I am not sure how to disable warnings for such TODO comments, please suggest.

For the error case it comes because of this line in constructor.

    /**
     * @type {!string}
     */
    this._idCIDString

Linting is expecting to assign some value there, we can disable this linting error. Else ee can assign an empty string and later we can check for empty string in toString case. It will fix the issue, but will introduce a change in this test case to have "_idCIDString": "" field in a peerId which is not stringified with toString().

https://github.com/nazarhussain/js-peer-id/blob/nh-ts-support/test/peer-id.spec.js#L312

[nazarhussain]

@vasco-santos Thanks for the information. Let me know if I can be helpful in creating that interface.

[lidel]

Triage note:

• next step is to create the interface
  • PoC would be to create interface with everything that is not prefixed with _ + factory function
  • @achingbrain will provide more details if time allows :pray:

[nazarhussain]

@lidel @vasco-santos If you guys allow I can create the interface for PeerID in interfaces repo. What do you think?

[lidel]

@nazarhussain yes i believe https://github.com/libp2p/js-libp2p-interfaces is the place. let us know if you still want to work on this or someone else can pick this topic

[BigLep]

@nazarhussain : just checking in - are you planing to add in the interface? Thanks.

[nazarhussain]

@BigLep I was not sure based on discussion, that should I contribute to the interfaces repo or not. But now I will create PR in few coming days.

[BigLep]

@nazarhussain: just checking in to see if you're going to make this PR or not. Thanks!

[nazarhussain]

Hi @BigLep sorry to get late. I got engaged with some other issues. I actually worked on it and created the interfaces. Was writing the test helpers where I left it. Will complete and open the PR this weekend. Then we can discuss on that PR to improve any thing further.

[nazarhussain]

@BigLep I just opened a PR https://github.com/libp2p/js-libp2p-interfaces/pull/107. I will work on its feedback. As soon it's merged will refactor this PR according to it.

[BigLep]

@nazarhussain: given https://github.com/libp2p/js-libp2p-interfaces/pull/107 got merged, are you ok to refactor this PR?

[nazarhussain]

@BigLep I was waiting for the newer release of libp2p-interfaces which includes that interface, so I can use it in this PR. As soon that is released I will refactor this PR and get it ready.

[nazarhussain]

The changes related libp2p-interfaces were released but some unintentional change was introduced. I had to create a PR for that https://github.com/libp2p/js-libp2p-interfaces/pull/126 Now waiting for this to get merged and released.

[BigLep]

Closing because this is being handled by the TypeScript conversion push (being tracked in https://github.com/libp2p/js-libp2p/issues/1021 ).