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

grpc-js & gpc-native-core: Types mismatch for ServerWritableStream #1581

Closed
badsyntax opened this issue Sep 20, 2020 · 4 comments
Closed

grpc-js & gpc-native-core: Types mismatch for ServerWritableStream #1581

badsyntax opened this issue Sep 20, 2020 · 4 comments

Comments

@badsyntax
Copy link
Contributor

badsyntax commented Sep 20, 2020

Problem description

I'm seeing a type difference in grpc-js and gpc-native-core.

export type ServerWriteableStream<RequestType> = ServerWritableStream<RequestType>;

export type ServerWritableStream<
RequestType,
ResponseType
> = ServerSurfaceCall &

ServerWritableStream requires one type argument within grpc-native-core and 2 within grpc-js.

Reproduction steps

I'm playing around with the typescript generator within #1474 which generates following service definition:

export interface ChatHandlers extends grpc.UntypedServiceImplementation {
  join(call: grpc.ServerWritableStream<_chat_package_ClientMessage__Output, _chat_package_ServerMessage>): void;
  send(call: grpc.ServerUnaryCall<_chat_package_ClientMessage__Output, _chat_package_ServerMessage>, callback: grpc.sendUnaryData<_chat_package_ServerMessage>): void;
}
Using this proto definition
syntax = "proto3";

package chat_package;

message ServerMessage {
  string user = 1;
  string text = 2;
}

message ClientMessage {
  string user = 1;
  string text = 2;
}

service Chat {
  rpc join(ClientMessage) returns (stream ServerMessage) {}
  rpc send(ClientMessage) returns (ServerMessage) {}
}

..which is incompatible with the types found in grpc-native-core (the grpc npm package)

@badsyntax
Copy link
Contributor Author

badsyntax commented Sep 25, 2020

@murgatroid99 while #1587 is a step in the right direction, the types are still incompatible.

For example:

interface IChatServer extends grpc.UntypedServiceImplementation {
  join: grpc.handleServerStreamingCall<proto.ClientMessage, proto.ServerMessage>;
}

const chatServer: IChatServer = {
  // type error when using the `grpc` package
  join(call: grpc.ServerWritableStream<proto.ClientMessage, proto.ServerMessage>): void {
    // implementation
  }
};

I think it makes sense to align grpc with @grpc/grpc-js, so we can have typed server stream methods (call.write(chunk: T)). What do you think? I'll try send a PR today.

@murgatroid99
Copy link
Member

I think the definition should probably be something like ServerWritableStream<RequestType, ResponseType=unknown> to get compatibility without breaking anything.

@badsyntax
Copy link
Contributor Author

That's a nice solution. Have added that change here: #1590

@murgatroid99
Copy link
Member

#1590 has been published in grpc version 1.24.4.

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

No branches or pull requests

2 participants