New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
client: update multiaddr dep #3384
Conversation
packages/devp2p/src/dns/enr.ts
Outdated
@@ -30,6 +22,13 @@ type ENRTreeValues = { | |||
domain: string | |||
} | |||
|
|||
// Copied over from the multiaddr repo: https://github.com/multiformats/js-multiaddr/blob/main/src/convert.ts | |||
// TODO: Can we import this directly from the multiaddr repo? Doing so makes CI fail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can import the convert
method from import { convert } from '@multiformats/multiaddr/convert
(since multiaddr
now uses an exports map in package.json
) but I'm all for reducing our exposure to upstream dependencies, especially when it's for a 3 line helper. Will suggest we just remove this TODO and move forward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did try doing that but it seemed to error in the CI still, anyhow I agree that removing the dep altogether is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
483f16c
to
0fad9ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR updates to the latest multiaddr version.
Resolves #3370