Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MethodDescriptor type issue in TypeScript #1351

Open
danias opened this issue Jul 31, 2023 · 3 comments
Open

MethodDescriptor type issue in TypeScript #1351

danias opened this issue Jul 31, 2023 · 3 comments

Comments

@danias
Copy link

danias commented Jul 31, 2023

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] 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
Copy link
Collaborator

@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
Copy link

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
Copy link

PaulFidika commented Dec 15, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants