search

einride / protoc-gen-typescript-http

Generate types and service clients from protobuf definitions annotated with http rules.
MIT License
44 stars 11 forks source link

fix(type): Correctly map proto to json according to spec #68

Open paulip1792 opened 2 years ago

paulip1792 commented 2 years ago

In protobuf definition (https://developers.google.com/protocol-buffers/docs/proto3#json), 64 bits numeric types should be interpreted as string in JSON.

ericwenn commented 2 years ago

Hello @paulip1792, thanks for contributing. I ran the CI pipeline, and youre missing the changes to generated code in your commit.

For my understand have you seen any isues with the current mapping in your applications?

paulip1792 commented 2 years ago

@ericwenn I am using protojson (https://pkg.go.dev/google.golang.org/protobuf/encoding/protojson) to marshal the response and 64 bits integer will be quoted as string value. I guess it's because JS Number can only represent 53 bits integer, so it's defined as string in the spec. In my opinion, in order to make sure it's all type-safe, 64 bits integer should be represented in string. I will make the CI happy if it is accepted change since I know it's a breaking change.

ericwenn commented 2 years ago

I agree we should address this. As it turns out we rarely use int64 in our protobuf schemas, so that is likely why we have not noticed it.

Thanks for the PR and reading through the spec!

ericwenn commented 2 years ago

@paulip1792 Is this still something you want to work on fixing?