grpc-web icon indicating copy to clipboard operation
grpc-web copied to clipboard

Missing some class definitions in the generated d.ts files.

Open aberasarte opened this issue 6 years ago • 10 comments

I was trying the new d.ts generation feature (thanks again for adding it) and I found that in the generated definition file some of the types are declared as Objects when they could have been strongly typed. For instance, for the following .proto file:

syntax = "proto3";

service UserService{

    rpc GetUser (GetUserRequest) returns (GetUserResponse);
}

message GetUserRequest {
    string user_name = 1;
}

message GetUserResponse {
    User user = 1;
}

message User {
    string name = 1;
}

The produced _pb_d.ts file looks as follows:

export class GetUserRequest {
  constructor ();
  getUserName(): string;
  setUserName(a: string): void;
  serializeBinary(): Uint8Array;
  static deserializeBinary: (bytes: {}) => GetUserRequest;
}

export class GetUserResponse {
  constructor ();
  getUser(): {};
  setUser(a: {}): void;
  serializeBinary(): Uint8Array;
  static deserializeBinary: (bytes: {}) => GetUserResponse;
}

The getUser() method returns {} when it could be User. The export class User is missing as well. I was expecting something like this:

export class GetUserRequest {
  constructor ();
  getUserName(): string;
  setUserName(a: string): void;
  serializeBinary(): Uint8Array;
  static deserializeBinary: (bytes: {}) => GetUserRequest;
}

export class GetUserResponse {
  constructor ();
  getUser(): User;
  setUser(user: User): void;
  serializeBinary(): Uint8Array;
  static deserializeBinary: (bytes: {}) => GetUserResponse;
}

export class User{
  constructor ();
  getName(): string;
  setName(a: string): void;
  serializeBinary(): Uint8Array;
  static deserializeBinary: (bytes: {}) => User;
}

Looking at the generator's code, I see that only numbers and strings are being taken into account when printing class fields types. Other well known types like array or map aren't being considered either.

aberasarte avatar Sep 07 '18 11:09 aberasarte

I'm experiencing the same issue after generating the ts files with protoc.

n4uti avatar Sep 07 '18 17:09 n4uti

syntax = "proto3";

package books;

service BookService {
  rpc List (Empty) returns (BookList) {}
  rpc Insert (Book) returns (Empty) {}
  rpc Get (BookIdRequest) returns (Book) {}
  rpc Delete (BookIdRequest) returns (Empty) {}
  rpc Watch (Empty) returns (stream Book) {}
}

message Empty {}

message Book {
  int32 id = 1;
  string title = 2;
  string author = 3;
}

message BookList {
  repeated Book books = 1;
}

message BookIdRequest {
  int32 id = 1;
}

Can one generate this book.proto file for me in grpc-web js and ts! I'm having some difficulties in compiling it on my windows.

generalomosco avatar Sep 07 '18 18:09 generalomosco

@aberasarte Thanks for the report. Yes we can certainly do better in the .d.ts typings output. Will work on this soon.

stanley-cheung avatar Sep 07 '18 18:09 stanley-cheung

@Generalomosco The files. Im using hyper v with ubuntu book.zip

nucle avatar Sep 07 '18 18:09 nucle

Just to add on. Can we have ClientReadableStream callback type to be any instead of Array<{}>? I am using the toObject method and it is giving type check errors.

  export class ClientReadableStream {
    on (type: string,
        callback: (...args: Array<{}>) => void): ClientReadableStream;
    cancel (): void;
  }

I feel that using any first will be a better move and it will be easier for us to transition once the typing files get better

weilip1803 avatar Oct 16 '18 07:10 weilip1803

This is also happening with well known types; when using the types from the example and replacing message_interval int32 with google.protobuf.Duration:

export class ServerStreamingEchoRequest {
  constructor ();
  getMessage(): string;
  setMessage(a: string): void;
  getMessageCount(): number;
  setMessageCount(a: number): void;
  getMessageInterval(): {};
  setMessageInterval(a: {}): void;
  serializeBinary(): Uint8Array;
  static deserializeBinary: (bytes: {}) => ServerStreamingEchoRequest;
}

johanbrandhorst avatar Oct 25 '18 18:10 johanbrandhorst

Interestingly, maybe this is a space where collaboration with https://github.com/improbable-eng/ts-protoc-gen could pay off? Their generator already has much better support for TypeScript bindings, with no dependencies other than google-protobuf and any imported types.

johanbrandhorst avatar Oct 25 '18 18:10 johanbrandhorst

Looks like it not fixed.

@stanley-cheung can you provide some ETA for that bug?

pumano avatar Nov 12 '18 15:11 pumano

Looks like it should be supported FieldDescriptor::TYPE_MESSAGE for repeated. Missing entity TypeScript definition as issue author mentioned:

The getUser() method returns {} when it could be User. The export class User is missing as well.

pumano avatar Nov 13 '18 10:11 pumano

This issue has not been fixed yet, has it? I'm facing same error😢 I am waiting eagerly for that this bug fixed.

makoto-developer avatar Dec 09 '21 02:12 makoto-developer