ts-proto icon indicating copy to clipboard operation
ts-proto copied to clipboard

Server streaming method outputs observable interface but promise client implementation

Open protango opened this issue 4 years ago • 2 comments

Consider the following proto

service MathService {
    rpc Add(MathServiceAddRequest) returns (stream MathServiceAddResponse) {}
}

Running ts-proto on this will generate the following:

export interface MathService {
  Add(request: MathServiceAddRequest): Observable<MathServiceAddResponse>;
}
export class MathServiceClientImpl implements MathService {
  // ...
  Add(request: MathServiceAddRequest): Promise<MathServiceAddResponse> {
    // ...
  }
}

Notice that the MathServiceClientImpl.Add method returns a Promise, but the interface specifies it should be an Observable, thus the following error is generated by typescript:

error TS2416: Property 'Add' in type 'MathServiceClientImpl' is not assignable to the same property in base type 'MathService'.

protango avatar Jun 07 '21 00:06 protango

Huh, thanks for the report; I kinda thought this code should already handle that:

https://github.com/stephenh/ts-proto/blob/84060e204d0e42688b8da85d434fe3d24788813b/src/generate-grpc-web.ts#L54

function generateRpcMethod(ctx: Context, serviceDesc: ServiceDescriptorProto, methodDesc: MethodDescriptorProto) {
  const { options, utils } = ctx;
  const inputType = requestType(ctx, methodDesc);
  const partialInputType = code`${utils.DeepPartial}<${inputType}>`;
  const returns =
    options.returnObservable || methodDesc.serverStreaming
      ? responseObservable(ctx, methodDesc)
      : responsePromise(ctx, methodDesc);

And there's an integration test that I'm pretty sure is basically the same proto here:

https://github.com/stephenh/ts-proto/blob/main/integration/grpc-web/example.ts#L750

Can you tell what's different in your setup vs. that integration test?

stephenh avatar Jun 07 '21 01:06 stephenh

generate-services.ts (not grpc-web) had a bug here where it missed the check for options.returnObservable || methodDesc.serverStreaming.

I fixed it with the first commit of https://github.com/stephenh/ts-proto/pull/373.

Jille avatar Oct 26 '21 19:10 Jille