Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

feat: add types #16

Merged
merged 7 commits into from
Apr 8, 2021
Merged

feat: add types #16

merged 7 commits into from
Apr 8, 2021

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Mar 26, 2021

This PR adds types to libp2p-utils and changes its CI to github actions

@vasco-santos vasco-santos marked this pull request as ready for review March 26, 2021 17:15
async sink (source) {
if (options.signal) {
// @ts-ignore abortable has no type definitions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I retried it deconstructing the require and it has types indeed. However, types infer that the Uint8Array will be a source with numbers for reasons 🤷🏼 I updated the ignore message to not block on this

tsconfig.json Outdated Show resolved Hide resolved
@@ -21,25 +21,28 @@ function ipPortToMultiaddr (ip, port) {
throw errCode(new Error(`invalid ip provided: ${ip}`), errors.ERR_INVALID_IP_PARAMETER)
}

// @ts-ignore parseInt expects only string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to just wrap with an if clause

return multiaddr(`/ip4/${ip}/tcp/${port}`)
}
} catch (_) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're ignoring the error you can use optional catch binding here:

Suggested change
} catch (_) {}
} catch {}

return ip6.is4()
? multiaddr(`/ip4/${ip6.to4().correctForm()}/tcp/${port}`)
: multiaddr(`/ip6/${ip}/tcp/${port}`)
} catch (err) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this error get logged somewhere?

@@ -35,23 +51,24 @@ function streamToMaConnection ({ stream, remoteAddr, localAddr }, options = {})
}
close()
},

// @ts-ignore abortable has no type definitions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why, I will open an issue in abortable-iterator. It is not exporting correctly the types in the default for some reason. I get abortable is not callable
However, if we do const { source: abortable } = require('abortable-iterator') it will infer the types correctly

Copy link

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems fine to me

@vasco-santos vasco-santos merged commit e0552b5 into master Apr 8, 2021
@vasco-santos vasco-santos deleted the feat/add-types branch April 8, 2021 09:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants