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

Add ClientOptions in constructor for grpc-js clients #62

Merged
merged 1 commit into from
Jul 31, 2020

Conversation

RMHonor
Copy link
Contributor

@RMHonor RMHonor commented Jul 31, 2020

The @grpc/grpc-js has types for the options in client constructors, this template change conditionally adds these types to generated constructors.

@agreatfool
Copy link
Owner

I found a strange issue.

See the client codes in example:

const BookServiceClient = grpc.makeClientConstructor(bookGrpcPb["com.book.BookService"], "BookService");
const client = new BookServiceClient("127.0.0.1:50051", grpc.credentials.createInsecure()) as any as bookGrpcPb.BookServiceClient;

Client class BookServiceClient comes from makeClientConstructor, which is:

export function makeClientConstructor(
  methods: ServiceDefinition,
  serviceName: string,
  classOptions?: {}
): ServiceClientConstructor {

  class ServiceClientImpl extends Client implements ServiceClient {
  }

  // ...

  return ServiceClientImpl;
}

See, the return type of function makeClientConstructor is ServiceClientConstructor, which is:

export interface ServiceClientConstructor {
  new (
    address: string,
    credentials: ChannelCredentials,
    options?: Partial<ChannelOptions>
  ): ServiceClient;
  service: ServiceDefinition;
}

Type of options here is ChannelOptions.

But, actually in the source codes, which really returned from function makeClientConstructor is class ServiceClientImpl who extends Client, and see here:

export class Client {

  constructor(
    address: string,
    credentials: ChannelCredentials,
    options: ClientOptions = {}
  ) {
    //...
  }
  //...
}

Type of options here is ClientOptions. Which is:

export type ClientOptions = Partial<ChannelOptions> & {
  channelOverride?: Channel;
  channelFactoryOverride?: (
    address: string,
    credentials: ChannelCredentials,
    options: ClientOptions
  ) => Channel;
  interceptors?: Interceptor[];
  interceptor_providers?: InterceptorProvider[];
  callInvocationTransformer?: CallInvocationTransformer;
};

They are not consistent.

@agreatfool
Copy link
Owner

And since the returned object is class ServiceClientImpl and which extends class Client, I think your pr shall be OK.

@agreatfool agreatfool merged commit e2191cf into agreatfool:master Jul 31, 2020
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

Successfully merging this pull request may close these issues.

2 participants