grpc / grpc-web

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

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

Open aberasarte opened 6 years ago

aberasarte commented 6 years ago

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.

n4uti commented 6 years ago

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

generalomosco commented 6 years ago
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.

stanley-cheung commented 6 years ago

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

nucle commented 6 years ago

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

weilip1803 commented 6 years ago

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

johanbrandhorst commented 6 years ago

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 commented 6 years ago

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.

pumano commented 5 years ago

Looks like it not fixed.

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

pumano commented 5 years ago

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.

makoto-developer commented 2 years ago

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