fix: grpc-web issue#239 where client streaming was producing incorr…

[samelie]

fixing grpc-web issue#239 where client streaming was producing incorect requestType

Based on this issue https://github.com/stephenh/ts-proto/issues/329

Hopefully this fix isn't overly naive

Thanks for all the hardwork putting this together; it's really cool

[stephenh]

Fwiw @samelie I pushed a "re-codegen" commit to this PR that undoes a lot of *.bin file changes you'd had in here that come from having different versions of protoc. Ideally you shouldn't need to include those changes in the PR.

[samelie]

Hey @stephenh hopefully this approach is better. the goal of not generating rpc ChangeUserSettingsStream (stream Cred) returns (stream DashUserSettingsState) {} has been accomplished for grpc-web

Also, off topic, any thoughts on bumping rxjs to 7.3.0? Saw nestjs is on it too. v7 has native ts support and is really good

[boukeversteegh]

Hi @samelie Is this PR still under development?

[samelie]

@stephenh @boukeversteegh

I'm really not familiar with client streaming in grpc-web. I saw a post on their issues about it saying you needed the websocket transport. Makes sense. One day I'll do an experiment (we like to say) , but I've been holding out for streamsAPI https://github.com/improbable-eng/grpc-web#client-side-streaming All that to say I don't have an easy way to setup a websockets and write the rxjs push code we'd need.

For me, this wouldn't be an issue (am fine with it not working), however I'm in a work situation where we need to run tsc on the generated output that produces errors like these for clientStreaming

  error TS1005: '(' expected.
         Observable<WriteVerifyFileRequest>.fromPartial(request),

tsc is really annoying in the way there no way to silence errors. For me since this is all in CI, this is a problem as I can't swallow them either (bash noob?)

The easiest solution in the vein of here be dragons is just to treat clientSide in the case of grpc-web && !nestJs like a normal Promise rpc

I'd be happy to propose my forked fix for this that changes requestType in types/types.ts to

export function requestType(ctx: Context, methodDesc: MethodDescriptorProto): Code {
  let typeName = rawRequestType(ctx, methodDesc);
  if (methodDesc.clientStreaming && ctx.options.nestJs) {
    return code`${imp('Observable@rxjs')}<${typeName}>`;
  }
  return typeName;
}

Less magical solutions would be better. Perhaps some ctx option to ignore all clientStreaming?