grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.45k stars 761 forks source link

Typescript: Property 'unaryCall' does not exist on type 'AbstractClientBase' #1331

Closed GabrieleGervasiPEL closed 1 week ago

GabrieleGervasiPEL commented 1 year ago

Running protoc with grp-web plugin (1.4.2) generated a file *ServiceClientPb.ts. Such file contains a call to this.client_.unaryCall. Meantime, npm module grp-web (1.4.2) doesn't contain such method in AbstractClientBase class.

Protoc configuration for plugin: --grpc-web_out=import_style=typescript,mode=grpcweb

sampajano commented 1 week ago

@GabrieleGervasiPEL Hi there :) Thanks for bringing up this question!

You're indeed right that unaryCall is not exposed on the AbstractClientBase class.. https://github.com/grpc/grpc-web/blob/master/packages/grpc-web/index.d.ts

Although, they do exist on the actual implementation of the class, which should work when it's actually called.. https://github.com/grpc/grpc-web/blob/8ab32b945ca0530222593127e9431a455eff24a8/javascript/net/grpc/web/grpcwebclientbase.js#L158-L161

(As of now, unaryCall() is just an alias for thenableCall(), where it simply calls the latter.)

I agree this is not ideal, and I will explore ways to close the gap (e.g. https://github.com/grpc/grpc-web/pull/1444). The right long-term fix is probably to get rid of thenableCall() in favor of unaryCall(), as the latter sounds like a better API (more clear) :)


Question: Although, I'm wondering what exact issue are you seeing in your runtime?

If you use grpc-web npm package, it would have exposed unaryCall method on the client implementation, even though it's not present in the index.d.ts file :)

GabrieleGervasiPEL commented 1 week ago

Sorry @sampajano It's been over a year since the question and we dropped the use of grpc web. Actually i don't remember what the problem was. Probably, reading my question, the unaryCall was not present in typings (index.d.ts) file and the generated client *ServiceClientPb.ts had a call to unaryCall, marked as red because not recognized. Maybe it's already been fixed.

sampajano commented 1 week ago

@GabrieleGervasiPEL Hi thank you for the context! No worries at all.. And my apologies for address this late.. :)

Yeah it makes sense. :) unaryCall is indeed not present from index.d.ts but at runtime the javascript should work.. :)

I'll close it now since it is no longer an issue for you. But will keep an eye on how to reduce the confusions in the APIs. (Especially as we migrate codebase to TypeScript.) Thank you for filing the issue in the first place! :)