-
Notifications
You must be signed in to change notification settings - Fork 29
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
dnsCache.lookup types might be wrong? #79
Comments
Agreed, I just bumped into this using
that makes the types non compatible for TS unfortunately and even though it is easily remedied by something like this /**
* @returns {import("node:http").RequestOptions['lookup']}
*/
const getCacheableLookup = () => {
// @ts-ignore - cacheable lookup has stricter types on IpFamily (4|6 vs number) why TS complains
return new CacheableLookup().lookup;
}; It would be fantastic to solve here and just import types for options from import("node:http").RequestOptions['lookup'] 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I'm saying 'might' because I'm a typescript n00b.
But from what I understand, the callback of the native dns.lookup, which is supposed to be what this library's types seem to be based on, always has a callback signature of (err, address, family).
Therefore, when you
util.promisify
it, the result will always be the address (the string).However, that is not the case for this library's
lookup
(https://github.com/szmarczak/cacheable-lookup/blob/master/index.d.ts#L109), causing inconsistencies in callback handling.Furthermore, because of the inconsistencies in the callback signatures in this library, when I
util.promisify
thednsCache.lookup
, it thinks the "promisified lookup async" can return a void.Now, you may think "well, why not just use lookupAsync directly anyway?", and that would be an excellent choice if I were directly consuming CacheableLookup; however, as a got plugin author, I have to deal with the fact that got's
dnsLookup
option is typed asCacheableLookup['lookup']
, which, as we've learned, may be incorrect.So the hope is that the wrong types are fixed at the root of the problem (and hopefully it'll "bubble up" to got).
Thank you.
The text was updated successfully, but these errors were encountered: