This repository has been archived by the owner on Jan 8, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat!: support DNS over HTTPS and DNS-JSON over HTTPS #55
feat!: support DNS over HTTPS and DNS-JSON over HTTPS #55
Changes from 5 commits
d20f584
b111802
0d9c747
14ac953
b87b9e3
1834420
78788f6
188c6ed
f8d3153
cac6ba9
b7bf7bc
2d8c695
5dc5f8d
5864b31
7c3c808
1fd5150
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 17 in packages/ipns/src/dns-resolvers/default.ts
Codecov / codecov/patch
packages/ipns/src/dns-resolvers/default.ts#L16-L17
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 think this can be DRYed.
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.
fwiw there is dns-over-http-resolver which is already used by js-mutiaddr which is already pulled in (i would assume) by Helia users.
Perhaps DoH code should be improved there, so we reuse it for all DNS lookup needs, and here we only import it?
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.
Added the logic here: vasco-santos/dns-over-http-resolver#102
FWIW, I am not sure how much of a performance hit this is, I think having one good way of doing this should be acceptable,
dns-over-https-resolver
can prolly be refactored further to export a lighter version ESM for browser use.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.
Do we need the
status
field? If so, what for? it's not part of rfc8427 nor rfc1035