google-cloud-node: Returning arrays for everything makes for awkward and confusing code

Environment details

  • google-cloud-node version: 1.2.0

Steps to reproduce

  1. require google-cloud
  2. Do lots of operations using the API
  3. Need to unpack an array with a [thing, ApiResponse] from every promise.

ex. Almost every API looks like this:

    download(options?: DownloadOptions): Promise<[Buffer]>;
    exists(): Promise<[boolean]>;
    get(): Promise<[File, ApiResponse]>;
    getMetadata(): Promise<[FileMetadata, ApiResponse]>;

but that means you end up doing [0] all over the place. The extra ApiResponse should be returned through some out of band system (ex. another argument that requests it, or a separate callback) instead of in an array with every response (and sometimes it seems there’s an array even when there is no other data returned.

I end up having to write wrappers around every Google Cloud API function like this:

const fileExists = async (file: gcs.File) => {
  return (await file.exists())[0];
};

since doing:

if (!(await file.exists())[0])) ...

is hard to read and confusing. In general all the arrays all over the API don’t feel like idiomatic Promise code. You end up leaving comments all over explaining what the arrays are about.

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 1
  • Comments: 16 (14 by maintainers)

Most upvoted comments

For those interested, I want to lay out a plan of how we are thinking of approaching it. Thoughts welcomed!

  1. We add new Async methods for all functions that return a Promise<Array<T>>. For example, bucket.exists() becomes bucket.existsAsync(). We make this change across all of the available libraries.

  2. We update all of the documentation and samples for all the libraries to point folks towards the new Async variants.

  3. We add process.emitWarning warnings for anyone using the old functions in a way that would return an array of values from the promise.

  4. We wait for a year.

  5. We remove the support for promise-arrays from the non-Async methods, and only support callbacks.

  6. We wait another year 😱

  7. We add support for async with the original methods back. We update all the docs, and the samples again, back to the original method, but with the new syntax.

  8. We add deprecation notices to the Async methods.

  9. We wait another year.

  10. We remove the Async methods.

Thoughts? @ofrobots @crwilcox @alexander-fenster @kinwa91 @stephenplusplus @callmehiphop

@JustinBeckwith now that we aren’t as scared of semver-major bumps anymore, does that change your preferred approach from your last suggestion?