-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
fix(dns) Fix resolve4 and resolve6 behavior #2144
Conversation
src/bun.js/node-dns.exports.js
Outdated
@@ -535,15 +548,21 @@ export const promises = { | |||
} | |||
|
|||
resolve(hostname, rrtype) { | |||
return dns.resolve(hostname, rrtype).then(promisifyResolve); | |||
return dns.resolve(hostname, rrtype).then(promisifyResolve(rrtype)); |
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.
can we generate each of these functions at the top somewhere to avoid the extra function allocation
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.
done in 9aeb64f
src/bun.js/node-dns.exports.js
Outdated
if (options?.ttl) { | ||
return dns.lookup(hostname, { family: 4 }); | ||
} | ||
return dns.lookup(hostname, { family: 4 }).then((res)=> res?.map(a => a.address)) |
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.
can we generate each function at the top to avoid the allocation
also make sure you have Prettier configured to run on save (you might need to run bun install
somewhere)
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.
done in 9aeb64f
Thank you |
* fix #2098 * fix last promisifyResolve call * avoid some functions alloc on dns.exports
fix #2098