Skip to content

Commit

Permalink
feat!: stop accepting Promise constructor (#737)
Browse files Browse the repository at this point in the history
BREAKING CHANGE

It won't be possible to pass a third-party Promise constructor (e.g. bluebird promise) to gax-managed client libraries.
  • Loading branch information
alexander-fenster authored Mar 5, 2020
1 parent d6e36c5 commit 816bf9b
Show file tree
Hide file tree
Showing 15 changed files with 23 additions and 186 deletions.
9 changes: 1 addition & 8 deletions src/apiCaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 3 additions & 6 deletions src/bundlingCalls/bundleApiCaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions src/call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>;
Expand Down
2 changes: 1 addition & 1 deletion src/createApiCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 1 addition & 8 deletions src/fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ interface FallbackServiceStub {
export class GrpcClient {
auth?: OAuth2Client | GoogleAuth;
authClient?: OAuth2Client | Compute | JWT | UserRefreshClient;
promise?: PromiseConstructor;
fallback: boolean;
grpcVersion: string;

Expand All @@ -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
*/

Expand All @@ -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
}
Expand Down Expand Up @@ -202,8 +198,7 @@ export class GrpcClient {
clientConfig,
configOverrides,
Status,
{metadataBuilder: buildMetadata},
this.promise
{metadataBuilder: buildMetadata}
);
}

Expand Down Expand Up @@ -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) {
Expand Down
21 changes: 1 addition & 20 deletions src/gax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -127,7 +124,6 @@ export interface CallOptions {
bundleOptions?: BundleOptions | null;
isBundling?: boolean;
longrunning?: BackoffSettings;
promise?: PromiseConstructor;
}

export class CallSettings {
Expand All @@ -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.
Expand All @@ -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
*/
Expand All @@ -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;
}

/**
Expand All @@ -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!;
}
Expand Down Expand Up @@ -252,10 +242,6 @@ export class CallSettings {
longrunning = options.longrunning;
}

if ('promise' in options) {
promise = options.promise!;
}

return new CallSettings({
timeout,
retry,
Expand All @@ -267,7 +253,6 @@ export class CallSettings {
maxResults,
otherArgs,
isBundling,
promise,
});
}
}
Expand Down Expand Up @@ -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.
*/
Expand All @@ -623,8 +606,7 @@ export function constructSettings(
clientConfig: ClientConfig,
configOverrides: ClientConfig,
retryNames: {},
otherArgs?: {},
promise?: PromiseConstructor
otherArgs?: {}
) {
otherArgs = otherArgs || {};
// tslint:disable-next-line no-any
Expand Down Expand Up @@ -679,7 +661,6 @@ export function constructSettings(
? createBundleOptions(bundlingConfig)
: null,
otherArgs,
promise: promise || Promise,
});
}

Expand Down
9 changes: 1 addition & 8 deletions src/grpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ const COMMON_PROTO_FILES = walk

export interface GrpcClientOptions extends GoogleAuthOptions {
auth?: GoogleAuth;
promise?: PromiseConstructor;
grpc?: GrpcModule;
}

Expand Down Expand Up @@ -76,7 +75,6 @@ export class ClientStub extends grpc.Client {

export class GrpcClient {
auth: GoogleAuth;
promise: PromiseConstructor;
grpc: GrpcModule;
grpcVersion: string;
fallback: boolean;
Expand All @@ -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) {
Expand Down Expand Up @@ -258,8 +252,7 @@ export class GrpcClient {
clientConfig,
configOverrides,
this.grpc.status,
{metadataBuilder: this.metadataBuilder(headers)},
this.promise
{metadataBuilder: this.metadataBuilder(headers)}
);
}

Expand Down
9 changes: 3 additions & 6 deletions src/longRunningCalls/longRunningApiCaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions src/longRunningCalls/longrunning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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!;
Expand Down Expand Up @@ -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) => {
Expand Down
9 changes: 3 additions & 6 deletions src/normalCalls/normalApiCaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions src/paginationCalls/pagedApiCaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {APICaller, ApiCallerSettings} from '../apiCaller';
import {APICaller} from '../apiCaller';
import {
GRPCCall,
NextPageRequestType,
Expand Down Expand Up @@ -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();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/streamingCalls/streamingApiCaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {APICaller, ApiCallerSettings} from '../apiCaller';
import {APICaller} from '../apiCaller';
import {
APICallback,
BiDiStreamingCall,
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Loading

0 comments on commit 816bf9b

Please sign in to comment.