feat!: stop accepting Promise constructor (#737)

alexander-fenster · web-flow · commit 816bf9b28321 · 2020-03-05T13:48:59.000-08:00
BREAKING CHANGE

It won't be possible to pass a third-party Promise constructor (e.g. bluebird promise) to gax-managed client libraries.
diff --git a/src/apiCaller.ts b/src/apiCaller.ts
@@ -28,19 +28,12 @@ import {GoogleError} from './googleError';
 import {NormalApiCaller} from './normalCalls/normalApiCaller';
 import {StreamProxy} from './streamingCalls/streaming';
 
-export interface ApiCallerSettings {
-  promise: PromiseConstructor;
-}
-
 /**
  * An interface for all kinds of API callers (normal, that just calls API, and
  * all special ones: long-running, paginated, bundled, streaming).
  */
 export interface APICaller {
-  init(
-    settings: ApiCallerSettings,
-    callback?: APICallback
-  ): OngoingCallPromise | OngoingCall | StreamProxy;
+  init(callback?: APICallback): OngoingCallPromise | OngoingCall | StreamProxy;
   wrap(func: GRPCCall): GRPCCall;
   call(
     apiCall: SimpleCallbackFunction,
diff --git a/src/bundlingCalls/bundleApiCaller.ts b/src/bundlingCalls/bundleApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {APICallback, GRPCCall, SimpleCallbackFunction} from '../apitypes';
 import {OngoingCall, OngoingCallPromise} from '../call';
 import {CallSettings} from '../gax';
@@ -34,14 +34,11 @@ export class BundleApiCaller implements APICaller {
     this.bundler = bundler;
   }
 
-  init(
-    settings: ApiCallerSettings,
-    callback?: APICallback
-  ): OngoingCallPromise | OngoingCall {
+  init(callback?: APICallback): OngoingCallPromise | OngoingCall {
     if (callback) {
       return new OngoingCall(callback);
     }
-    return new OngoingCallPromise(settings.promise);
+    return new OngoingCallPromise();
   }
 
   wrap(func: GRPCCall): GRPCCall {
diff --git a/src/call.ts b/src/call.ts
@@ -96,13 +96,11 @@ export class OngoingCallPromise extends OngoingCall {
   /**
    * GaxPromise is GRPCCallbackWrapper, but it holds a promise when
    * the API call finishes.
-   * @param {Function} PromiseCtor - A constructor for a promise that implements
-   * the ES6 specification of promise.
    * @constructor
    * @private
    */
   // tslint:disable-next-line variable-name
-  constructor(PromiseCtor: PromiseConstructor) {
+  constructor() {
     let resolveCallback: (
       result: [ResponseType, NextPageRequestType, RawResponseType]
     ) => void;
@@ -121,7 +119,7 @@ export class OngoingCallPromise extends OngoingCall {
         throw new GoogleError('Neither error nor response are defined');
       }
     };
-    const promise = new PromiseCtor((resolve, reject) => {
+    const promise = new Promise((resolve, reject) => {
       resolveCallback = resolve;
       rejectCallback = reject;
     }) as CancellablePromise<ResultTuple>;
diff --git a/src/createApiCall.ts b/src/createApiCall.ts
@@ -78,7 +78,7 @@ export function createApiCall(
       currentApiCaller = createAPICaller(settings, undefined);
     }
 
-    const ongoingCall = currentApiCaller.init(thisSettings, callback);
+    const ongoingCall = currentApiCaller.init(callback);
     funcPromise
       .then((func: GRPCCall) => {
         // Initially, the function is just what gRPC server stub contains.
diff --git a/src/fallback.ts b/src/fallback.ts
@@ -64,7 +64,6 @@ interface FallbackServiceStub {
 export class GrpcClient {
   auth?: OAuth2Client | GoogleAuth;
   authClient?: OAuth2Client | Compute | JWT | UserRefreshClient;
-  promise?: PromiseConstructor;
   fallback: boolean;
   grpcVersion: string;
 
@@ -74,8 +73,6 @@ export class GrpcClient {
    *
    * @param {Object=} options.auth - An instance of OAuth2Client to use in browser, or an instance of GoogleAuth from google-auth-library
    *  to use in Node.js. Required for browser, optional for Node.js.
-   * @param {Function=} options.promise - A constructor for a promise that
-   * implements the ES6 specification of promise.
    * @constructor
    */
 
@@ -93,7 +90,6 @@ export class GrpcClient {
         (options.auth as GoogleAuth) ||
         new GoogleAuth(options as GoogleAuthOptions);
     }
-    this.promise = 'promise' in options ? options.promise! : Promise;
     this.fallback = true;
     this.grpcVersion = 'fallback'; // won't be used anywhere but we need it to exist in the class
   }
@@ -202,8 +198,7 @@ export class GrpcClient {
       clientConfig,
       configOverrides,
       Status,
-      {metadataBuilder: buildMetadata},
-      this.promise
+      {metadataBuilder: buildMetadata}
     );
   }
 
@@ -374,8 +369,6 @@ export class GrpcClient {
  * gRPC-fallback version of lro
  *
  * @param {Object=} options.auth - An instance of google-auth-library.
- * @param {Function=} options.promise - A constructor for a promise that
- * implements the ES6 specification of promise.
  * @return {Object} A OperationsClientBuilder that will return a OperationsClient
  */
 export function lro(options: GrpcClientOptions) {
diff --git a/src/gax.ts b/src/gax.ts
@@ -48,9 +48,6 @@ import {BundleOptions} from './bundlingCalls/bundleExecutor';
  * @property {boolean=} isBundling - If set to false and the call is configured
  *   for bundling, bundling is not performed.
  * @property {BackoffSettings=} longrunning - BackoffSettings used for polling.
- * @property {Function=} promise - A constructor for a promise that implements the ES6
- * specification of promise which will be used to create promises. If not
- * provided, native promises will be used.
  * @example
  * // suppress bundling for bundled method.
  * api.bundlingMethod(
@@ -127,7 +124,6 @@ export interface CallOptions {
   bundleOptions?: BundleOptions | null;
   isBundling?: boolean;
   longrunning?: BackoffSettings;
-  promise?: PromiseConstructor;
 }
 
 export class CallSettings {
@@ -142,7 +138,6 @@ export class CallSettings {
   bundleOptions?: BundleOptions | null;
   isBundling: boolean;
   longrunning?: BackoffSettings;
-  promise: PromiseConstructor;
 
   /**
    * @param {Object} settings - An object containing parameters of this settings.
@@ -159,9 +154,6 @@ export class CallSettings {
    * in the page streaming request.
    * @param {Object} settings.otherArgs - Additional arguments to be passed to
    *   the API calls.
-   * @param {Function=} settings.promise - A constructor for a promise that
-   * implements the ES6 specification of promise. If not provided, native
-   * promises will be used.
    *
    * @constructor
    */
@@ -178,7 +170,6 @@ export class CallSettings {
     this.isBundling = 'isBundling' in settings ? settings.isBundling! : true;
     this.longrunning =
       'longrunning' in settings ? settings.longrunning : undefined;
-    this.promise = 'promise' in settings ? settings.promise! : Promise;
   }
 
   /**
@@ -202,7 +193,6 @@ export class CallSettings {
     let otherArgs = this.otherArgs;
     let isBundling = this.isBundling;
     let longrunning = this.longrunning;
-    let promise = this.promise;
     if ('timeout' in options) {
       timeout = options.timeout!;
     }
@@ -252,10 +242,6 @@ export class CallSettings {
       longrunning = options.longrunning;
     }
 
-    if ('promise' in options) {
-      promise = options.promise!;
-    }
-
     return new CallSettings({
       timeout,
       retry,
@@ -267,7 +253,6 @@ export class CallSettings {
       maxResults,
       otherArgs,
       isBundling,
-      promise,
     });
   }
 }
@@ -613,8 +598,6 @@ export interface ClientConfig {
  *   those codes.
  * @param {Object} otherArgs - the non-request arguments to be passed to the API
  *   calls.
- * @param {Function=} promise - A constructor for a promise that implements the
- * ES6 specification of promise. If not provided, native promises will be used.
  * @return {Object} A mapping from method name to CallSettings, or null if the
  *   service is not found in the config.
  */
@@ -623,8 +606,7 @@ export function constructSettings(
   clientConfig: ClientConfig,
   configOverrides: ClientConfig,
   retryNames: {},
-  otherArgs?: {},
-  promise?: PromiseConstructor
+  otherArgs?: {}
 ) {
   otherArgs = otherArgs || {};
   // tslint:disable-next-line no-any
@@ -679,7 +661,6 @@ export function constructSettings(
         ? createBundleOptions(bundlingConfig)
         : null,
       otherArgs,
-      promise: promise || Promise,
     });
   }
 
diff --git a/src/grpc.ts b/src/grpc.ts
@@ -42,7 +42,6 @@ const COMMON_PROTO_FILES = walk
 
 export interface GrpcClientOptions extends GoogleAuthOptions {
   auth?: GoogleAuth;
-  promise?: PromiseConstructor;
   grpc?: GrpcModule;
 }
 
@@ -76,7 +75,6 @@ export class ClientStub extends grpc.Client {
 
 export class GrpcClient {
   auth: GoogleAuth;
-  promise: PromiseConstructor;
   grpc: GrpcModule;
   grpcVersion: string;
   fallback: boolean;
@@ -93,14 +91,10 @@ export class GrpcClient {
    * @param {Object=} options.grpc - When specified, this will be used
    *   for the 'grpc' module in this context. By default, it will load the grpc
    *   module in the standard way.
-   * @param {Function=} options.promise - A constructor for a promise that
-   * implements the ES6 specification of promise. If not provided, native
-   * promises will be used.
    * @constructor
    */
   constructor(options: GrpcClientOptions = {}) {
     this.auth = options.auth || new GoogleAuth(options);
-    this.promise = options.promise || Promise;
     this.fallback = false;
 
     if ('grpc' in options) {
@@ -258,8 +252,7 @@ export class GrpcClient {
       clientConfig,
       configOverrides,
       this.grpc.status,
-      {metadataBuilder: this.metadataBuilder(headers)},
-      this.promise
+      {metadataBuilder: this.metadataBuilder(headers)}
     );
   }
 
diff --git a/src/longRunningCalls/longRunningApiCaller.ts b/src/longRunningCalls/longRunningApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {APICallback, GRPCCall, SimpleCallbackFunction} from '../apitypes';
 import {OngoingCall, OngoingCallPromise} from '../call';
 import {
@@ -45,14 +45,11 @@ export class LongrunningApiCaller implements APICaller {
     this.longrunningDescriptor = longrunningDescriptor;
   }
 
-  init(
-    settings: ApiCallerSettings,
-    callback?: APICallback
-  ): OngoingCallPromise | OngoingCall {
+  init(callback?: APICallback): OngoingCallPromise | OngoingCall {
     if (callback) {
       return new OngoingCall(callback);
     }
-    return new OngoingCallPromise(settings.promise);
+    return new OngoingCallPromise();
   }
 
   wrap(func: GRPCCall): GRPCCall {
diff --git a/src/longRunningCalls/longrunning.ts b/src/longRunningCalls/longrunning.ts
@@ -163,8 +163,7 @@ export class Operation extends EventEmitter {
     function promisifyResponse() {
       if (!callback) {
         // tslint:disable-next-line variable-name
-        const PromiseCtor = self._callOptions!.promise!;
-        return new PromiseCtor((resolve, reject) => {
+        return new Promise((resolve, reject) => {
           if (self.latestResponse.error) {
             const error = new GoogleError(self.latestResponse.error.message!);
             error.code = self.latestResponse.error.code!;
@@ -339,8 +338,7 @@ export class Operation extends EventEmitter {
    */
   promise() {
     // tslint:disable-next-line variable-name
-    const PromiseCtor = this._callOptions!.promise!;
-    return new PromiseCtor((resolve, reject) => {
+    return new Promise((resolve, reject) => {
       this.on('error', reject).on(
         'complete',
         (result, metadata, rawResponse) => {
diff --git a/src/normalCalls/normalApiCaller.ts b/src/normalCalls/normalApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {APICallback, GRPCCall, SimpleCallbackFunction} from '../apitypes';
 import {OngoingCall, OngoingCallPromise} from '../call';
 import {GoogleError} from '../googleError';
@@ -23,14 +23,11 @@ import {GoogleError} from '../googleError';
  * Creates an API caller for regular unary methods.
  */
 export class NormalApiCaller implements APICaller {
-  init(
-    settings: ApiCallerSettings,
-    callback?: APICallback
-  ): OngoingCallPromise | OngoingCall {
+  init(callback?: APICallback): OngoingCallPromise | OngoingCall {
     if (callback) {
       return new OngoingCall(callback);
     }
-    return new OngoingCallPromise(settings.promise);
+    return new OngoingCallPromise();
   }
 
   wrap(func: GRPCCall): GRPCCall {
diff --git a/src/paginationCalls/pagedApiCaller.ts b/src/paginationCalls/pagedApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {
   GRPCCall,
   NextPageRequestType,
@@ -120,11 +120,11 @@ export class PagedApiCaller implements APICaller {
    * @param settings Call settings. Can only be used to replace Promise with another promise implementation.
    * @param [callback] Callback to be called, if any.
    */
-  init(settings: ApiCallerSettings, callback?: APICallback) {
+  init(callback?: APICallback) {
     if (callback) {
       return new OngoingCall(callback);
     }
-    return new OngoingCallPromise(settings.promise);
+    return new OngoingCallPromise();
   }
 
   /**
diff --git a/src/streamingCalls/streamingApiCaller.ts b/src/streamingCalls/streamingApiCaller.ts
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {APICaller, ApiCallerSettings} from '../apiCaller';
+import {APICaller} from '../apiCaller';
 import {
   APICallback,
   BiDiStreamingCall,
@@ -42,7 +42,7 @@ export class StreamingApiCaller implements APICaller {
     this.descriptor = descriptor;
   }
 
-  init(settings: ApiCallerSettings, callback: APICallback): StreamProxy {
+  init(callback: APICallback): StreamProxy {
     return new StreamProxy(this.descriptor.type, callback);
   }
 
diff --git a/test/fixtures/google-gax-packaging-test-app/src/v1beta1/echo_client.js b/test/fixtures/google-gax-packaging-test-app/src/v1beta1/echo_client.js
@@ -54,8 +54,6 @@ class EchoClient {
    *     app is running in an environment which supports
    *     {@link https://developers.google.com/identity/protocols/application-default-credentials Application Default Credentials},
    *     your project ID will be detected automatically.
-   * @param {function} [options.promise] - Custom promise module to use instead
-   *     of native Promises.
    * @param {string} [options.apiEndpoint] - The domain name of the
    *     API remote host.
    */
diff --git a/test/unit/apiCallable.ts b/test/unit/apiCallable.ts
diff --git a/test/unit/longrunning.ts b/test/unit/longrunning.ts

Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ export function createApiCall(`
`78`	`78`	`currentApiCaller = createAPICaller(settings, undefined);`
`79`	`79`	`}`
`80`	`80`
`81`		`- const ongoingCall = currentApiCaller.init(thisSettings, callback);`
	`81`	`+ const ongoingCall = currentApiCaller.init(callback);`
`82`	`82`	`funcPromise`
`83`	`83`	`.then((func: GRPCCall) => {`
`84`	`84`	`// Initially, the function is just what gRPC server stub contains.`