grpc / grpc-web

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

MethodDescriptor type issue in TypeScript #1351

Open danias opened 1 year ago

danias commented 1 year ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch grpc-web@1.4.2 for the project I'm working on.

The autogenerated files are not compatible with the current MethodDescriptor definition that has ...args: unknown[]. As a result my TypeScript project fails to run.

Argument of type 'typeof MyRequest' is not assignable to parameter of type 'new (...args: unknown[]) => MyRequest'.

The command I am running to generate the files is the following:

"proto": "protoc -I=. src/proto/proto.proto --ts_out=. --ts_opt=target=web,json_names,unary_rpc_promise=true,no_namespace --grpc-web_out=import_style=typescript,mode=grpcwebtext:.",

Here is the diff that solved my problem:

diff --git a/node_modules/grpc-web/index.d.ts b/node_modules/grpc-web/index.d.ts
index 09fb671..9b6b233 100644
--- a/node_modules/grpc-web/index.d.ts
+++ b/node_modules/grpc-web/index.d.ts
@@ -71,8 +71,8 @@ declare module "grpc-web" {
   export class MethodDescriptor<REQ, RESP> {
     constructor(name: string,
                 methodType: string,
-                requestType: new (...args: unknown[]) => REQ,
-                responseType: new (...args: unknown[]) => RESP,
+                requestType: new (...args: any) => REQ,
+                responseType: new (...args: any) => RESP,
                 requestSerializeFn: any,
                 responseDeserializeFn: any);
     getName(): string;

This issue body was partially generated by patch-package.

sampajano commented 1 year ago

@danias Hi! Thanks for the report (and providing the example patch how the issue can be fixed)!

The patch you provide relaxes the type check so i understand it would fix the issue but also more potential bugs can slip in.

I wonder if you could provide more details on what exactly does your generated typescript files look like?

I assume that this issue is due to the specific proto you are using. You could confirmed that if you try out our TS example and see if the issue also reproduces there for you.

Thanks!

PaulFidika commented 9 months ago

I ran into this same issue, I'm compiling protoc files with:

https://www.npmjs.com/package/protoc-gen-ts

Using the command

protoc -I=$PROTO_DIR --ts_out=$TMP_DIR --ts_opt=target=web *.proto;

PaulFidika commented 9 months ago

protoc-gen-ts generates files with class-constructors that look like this:

```
export class ComfyRequest extends pb_1.Message {
    #one_of_decls: number[][] = [];
    constructor(data?: any[] | {
        workflows?: Workflow[];
        input_files?: WorkflowFile[];
        save_outputs?: boolean;
        hold_worker?: boolean;
        room_id?: string;
    }) {
    ```

however MethodDescriptor in this library expects them to be of type ...args: unknown[], causing a spurious type conflict. You should really find a way to make the method-descriptor type more broadly compatible.