Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

getInstances type is incorrect: returns a Promise when callback is unspecified #564

Closed
lukasschwab opened this issue Nov 7, 2019 · 7 comments · Fixed by #579
Closed
Assignees
Labels
api: bigtable Issues related to the googleapis/nodejs-bigtable API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. typescript

Comments

@lukasschwab
Copy link

Environment details

  • OS: macOS 10.14
  • Node.js version: v12.12.0
  • npm version: 6.11.3
  • @google-cloud/bigtable version: 2.2.1

Steps to reproduce

I have code that calls getInstances without defining a callback:

const [instances] = await bigtable.getInstances();

getInstances behaves according to its docstring:

If the callback is omitted, we'll return a Promise.

...but this behavior is not reflected in its generated type, which has return type void and no overloads:

getInstances(gaxOptions?: any, callback?: any): void;

This mismatch causes eslint to complain because void is a non-Promise.

Screen Shot 2019-11-07 at 15 44 55

Proposal

Declare an overload type:

getInstances(gaxOptions?: any, callback: any): void;
getInstances(gaxOptions?: any): Promise<Instance[]>;

Thanks!

@jkwlui jkwlui added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. typescript help wanted We'd love to have community involvement on this issue. labels Nov 8, 2019
@jkwlui
Copy link
Member

jkwlui commented Nov 8, 2019

We need to annotate this client like the way we do with other TypeScript libraries.

@lukasschwab
Copy link
Author

@jkwlui can you link one of the annotated libraries/documentation for the annotation style? I'd be happy to give it a shot this weekend.

@stephenplusplus stephenplusplus removed the help wanted We'd love to have community involvement on this issue. label Nov 22, 2019
@callmehiphop
Copy link
Contributor

@JustinBeckwith @jkwlui looks like we're missing types across the board for the entire client. Is this something that is being prioritize? There isn't an open issue or anything, but I'd be happy to try and take this if some one isn't already.

@bcoe
Copy link
Contributor

bcoe commented Nov 25, 2019

@callmehiphop could we leverage @alexander-fenster's work to auto-generate types with the new micro-generator?

@callmehiphop
Copy link
Contributor

@bcoe it would help in some aspects like having request/response types mapped out for us.

@xiaowei-routific
Copy link

We are currently adopting bigtable right now and having the same problem. We have a thin layer of wrapper of our own right now and really hope that this could be prioritized

@google-cloud-label-sync google-cloud-label-sync bot added the api: bigtable Issues related to the googleapis/nodejs-bigtable API. label Jan 31, 2020
@jgehrcke
Copy link

jgehrcke commented Mar 9, 2020

Can we do the same for createInstance and other entry points?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
api: bigtable Issues related to the googleapis/nodejs-bigtable API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants