Skip to content
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

Open
JaneJeon opened this issue May 28, 2023 · 1 comment
Open

dnsCache.lookup types might be wrong? #79

JaneJeon opened this issue May 28, 2023 · 1 comment

Comments

@JaneJeon
Copy link

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 the dnsCache.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 as CacheableLookup['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.

@tolu
Copy link

tolu commented Nov 10, 2023

Agreed, I just bumped into this using express-http-proxy that pulls types directly from import("node:http").RequestOptions['lookup'] where IPFamily is defined as

  • family?: number | undefined; vs types in this module that are
  • family?: 4 | 6;

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants