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

Handling well-known types #4

Closed
atreya2011 opened this issue Apr 10, 2021 · 7 comments
Closed

Handling well-known types #4

atreya2011 opened this issue Apr 10, 2021 · 7 comments

Comments

@atreya2011
Copy link
Contributor

atreya2011 commented Apr 10, 2021

Given the following proto message:

syntax = "proto3";

import "google/protobuf/timestamp.proto";
import "google/protobuf/any.proto";

message User {
  string                    id       = 1;
  string                    name     = 2;
  google.protobuf.Timestamp my_field = 3;
}

// https://developers.google.com/protocol-buffers/docs/proto3#any
message ErrorStatus {
  string   message                     = 1;
  repeated google.protobuf.Any details = 2;
}

The following TypeScript is being generated which throws a type error because neither timestamp.pb.ts nor any.pb exist.

/*
* This file is a generated Typescript file for GRPC Gateway, DO NOT MODIFY
*/

import * as GoogleProtobufAny from "../google/protobuf/any.pb"
import * as GoogleProtobufTimestamp from "../google/protobuf/timestamp.pb"

export type User = {
  id?: string
  name?: string
  myField?: GoogleProtobufTimestamp.Timestamp
}

export type ErrorStatus = {
  message?: string
  details?: GoogleProtobufAny.Any[]
}

Please let me know if I am missing any steps in the generation process 🙏🏼
I used the following command to generate the above TypeScript file.

protoc -I. --grpc-gateway-ts_out=. --grpc-gateway-ts_opt logtostderr=true --grpc-gateway-ts_opt loglevel=debug ./proto/example.proto

@atreya2011 atreya2011 changed the title Handling well known types Handling well-known types Apr 10, 2021
@atreya2011
Copy link
Contributor Author

atreya2011 commented Apr 11, 2021

It looks like there is no support for handling protobuf well-known types. I added support for handling the following well-known types and have confirmed it working locally.

  1. google.protobuf.Empty => corresponding TypeScript type: Record<never, never>
  2. google.protobuf.Any => corresponding TypeScript type: any
  3. google.protobuf.Timestamp => corresponding TypeScript type: Date

Is it ok to create a PR?

@lyonlai
Copy link
Collaborator

lyonlai commented Apr 11, 2021

The following TypeScript is being generated which throws a type error because neither timestamp.pb.ts nor any.pb exist.

^^ you need to run the protoc-gen-grpc-gateway-ts on timestamp.pb.ts and any.pb. That's how we did it.

@atreya2011
Copy link
Contributor Author

atreya2011 commented Apr 12, 2021

Ah I see... Is the following fix acceptable as a PR? We don't have to use protoc-gen-gateway-ts on timestamp or any everytime.

atreya2011@a1e3d5c

@lyonlai
Copy link
Collaborator

lyonlai commented Apr 12, 2021

Generate typescript for the depending proto files works out nicely and it is a natural requirement like all other protoc plugins. So I don't think making a special case here make sense.

@atreya2011
Copy link
Contributor Author

Ok I understand. Thank you for your kind response 🙇🏼

@lixin9311
Copy link

The following TypeScript is being generated which throws a type error because neither timestamp.pb.ts nor any.pb exist.

^^ you need to run the protoc-gen-grpc-gateway-ts on timestamp.pb.ts and any.pb. That's how we did it.

Hi, if we run the protoc-gen-grpc-gateway-ts on timestamp.pb, we will get the definition of the underlying timestamp.

// example message
export type Message = {
  value?: string
  waitPeriod?: GoogleProtobufDuration.Duration
  ts?: GoogleProtobufTimestamp.Timestamp
}
// the definition of GoogleProtobufTimestamp.Timestamp
export type Timestamp = {
  seconds?: string
  nanos?: number
}

Which will be rejected by grpc-gateway.
The correct json mapping of the timestamp is a datetime in RFC3339(nano) format, which can be fulfilled by the default behavior of JSON.stringify a Date object in JavaScript.

I think we still need to set up exception rules for parsing those WKTs, at least for Timestamp and Duration.
https://developers.google.com/protocol-buffers/docs/proto3#json

@atreya2011
Copy link
Contributor Author

@lixin9311 Since this issue is closed, you can reference this and open a new issue 🙏🏼

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

No branches or pull requests

3 participants