grpc / grpc-node

gRPC for Node.js
https://grpc.io
Apache License 2.0
4.47k stars 647 forks source link

proto-loader + grpc-js + typescript without type casts #1901

Open glenjamin opened 3 years ago

glenjamin commented 3 years ago

Is your feature request related to a problem? Please describe.

I've been struggling to get my head around the exact interactions between proto-loader, grpc-js and typescript.

From what I can tell, the typescript types need to be static - so that the typescript compiler can see them. Meanwhile, proto-loader only appears to have a dynamic mode where it reads from a .proto file.

This leads to a situation where the runtime code to load the .proto source requires explicit type casts to apply the correct typescript types.

Describe the solution you'd like

It would be helpful if there was a way to generate the static equivalent of the proto-loader result in a way that matches the typescript definitions, so no typescript type casting is required, and the types and the loaded protobufs are kept in sync.

Describe alternatives you've considered

From what I can tell, grpc-tools has a protobuf compilation option which can use grpc-js, but it's unclear to me if this produces code compatible with the types generated by proto-loader.

Additional context

n/a

murgatroid99 commented 3 years ago

To be specific, the current code requires a type assertion on the object returned by calling grpc.loadPackageDefinition with the output of protoLoader.load, and the combination of those creates two levels of type weakening. Generating that object statically in a type-safe way would be a whole different problem than just generating the object that the proto-loader library outputs. It would be a lot of work just to avoid one type assertion.

grpc-tools is a completely separate code generator, and it has no API compatibility with proto-loader.

glenjamin commented 3 years ago

Generating that object statically in a type-safe way would be a whole different problem than just generating the object that the proto-loader library outputs.

Oh, that's a great idea!

I was under the impression that proto-loader was using the code-gen code from protobuf.js under the hood, so might be able to switch to a static-mode, but I hadn't looked into it very deeply yet.

murgatroid99 commented 3 years ago

I think I was not clear in my comment. proto-loader is in fact using protobuf.js under the hood, though it does its own modifications to the output, so there would probably need to be a separate layer of translation code for the statically generated code.

But that's not even the problem. The output of that library is not where the type needs to be asserted, so generating that code statically would not allow for removing that type assertion. Static, type safe generation of the object that currently requires a type assertion would be an even harder problem.

glenjamin commented 3 years ago

I think a reasonable workaround for me is to add a step into my own build process which generates a typescript stub with appropriate assertions for protoloader.loadSync + grpc.loadPackageDefinition for each .proto file, and then have the rest of my code only access the resulting generated module.

I'll give that a go and post whatever I end up with here for posterity, but it sounds like this might not be worth adding to the core tooling itself?

murgatroid99 commented 3 years ago

I think someone did actually make a similar suggestion before, but I can't find the issue now. It should be possible to generate that function, or probably two to account for synchronous and asynchronous loading, and include it with the other generated code in the top-level file. I don't remember if there was a reason we didn't add it. I didn't think of it until just now because it wouldn't exactly be "without type casts", it would just move the type cast into generated code. But that is probably better for most users.