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

[Enhancement] Make lookup() method accept named params #27

Open
prescience-data opened this issue Mar 25, 2021 · 3 comments
Open

[Enhancement] Make lookup() method accept named params #27

prescience-data opened this issue Mar 25, 2021 · 3 comments

Comments

@prescience-data
Copy link

Currently, the main lookup method is configured to accept a series of params:

async lookup(ip?: string, selectField?: string, fields?: string[])

The issue is if you are wanting to set only one of these, you need to pass undefined to the preceding params.

A common pattern to help solve this scenario would be:

export interface LookupParams {
  ip?: string
  selectField?: string
  fields?: string[]
}

// ... main class
{
  async lookup({ ip, selectField, fields }: LookupParams = {}) {
   // ... lookup logic
  }
}

Naturally, changing the shape of the method signature is a tricky undertaking, so potentially you could run a type check on the first param and keep the existing ones to allow backward compatibility?

@Berand-cod
Copy link

Great!

@jonathan-kosgei
Copy link
Collaborator

@thomasconner thoughts?

@thomasconner
Copy link
Collaborator

Yes this would be much better. I think a major version bump would be better to help people opt in rather then trying to maintain backwards compatibility. I can work on making this change this week.

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

4 participants