Translating int64 to TypeScript Number type sacrifices accuracy about ts-protoc-gen HOT 11 CLOSED

Hi @johanbrandhorst I've looked into this in the past and currently there is no official support in the public version of protobuf to use strings to preserve precision. There are a couple of solutions in progress though:

• There's this PR which would fix this bug in the JS implementation: protocolbuffers/protobuf#1832 (which if landed would mean ts-protoc-gen should be updated to accept string | number in int64 setters)
• There's also the field annotation google.protobuf.FieldOptions.JSType which lets you choose per field whether it should use strings or numbers (unfortunately there's no plan to make string the default representation).

Both of these would fix the underlying problem that, although you can use the field setters with a string, just before they're serialised the values are converted to numbers and so precision is lost. There are functions to serialise string encoded int64s but these aren't used in the public version of protobuf (the Google internal version support the JSType annotation and if set to string uses writeInt64String instead of writeInt64)

Although it's not used in generated code, the writeInt64String method is exposed in the protobuf library so there is a hack to make this work:

import { BinaryWriter, BinaryReader } from 'google-protobuf';

Object.defineProperty(BinaryWriter.prototype, 'writeInt64', {
  value: function(field: number, value: number | string) {
    return BinaryWriter.prototype.writeInt64String.call(this, field, value.toString());
  },
});
Object.defineProperty(BinaryReader.prototype, 'readInt64', { value: BinaryReader.prototype.readInt64String });

This is ugly (and doesn't work with the ts-protoc-gen generated types without using something like pb.setMyInt64Field('1309150421589' as any)) but does mean that values are correctly serialised and deserialised as strings

from ts-protoc-gen.

From protocolbuffers/protobuf#3666 (comment):

message dummy {
    uint64 bigInt = 1 [jstype = JS_STRING];
}

It makes use of the various FromString methods: https://github.com/google/protobuf/blob/dd2dc0f14f9e5dc4e8343cc5f78d5f0bddd8991c/js/binary/arith.js#L289, https://github.com/google/protobuf/blob/dd2dc0f14f9e5dc4e8343cc5f78d5f0bddd8991c/js/binary/encoder.js#L356 etc. I implemented support for this in my GopherJS gRPC-Web library recently: johanbrandhorst/protobuf@7e36e40.

from ts-protoc-gen.

The protoc compiler therefore uses strings when expressing large numeric types in JSON.

I was under the impression that protoc only ever used javascript's Number type to express int64 values. protocolbuffers/protobuf#2214 looks to address this but has not landed.

from ts-protoc-gen.

I'm talking specifically about the JSON representation of int64 integers. The JS library may well be using the Number type, so this might be impossible to remedy here, but I thought it'd be good to have this discussion anyway.

from ts-protoc-gen.

@improbable-bradley that's a great writeup, I'll reference this in the future, thanks!

from ts-protoc-gen.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

from ts-protoc-gen.

protoc has supported this since 3.4.0: https://github.com/google/protobuf/releases/tag/v3.4.0

from ts-protoc-gen.

from ts-protoc-gen.

Thanks @johanbrandhorst; it would be good to add a test-case which uses this field option and provide a mention to it in the README at which point this issue can be closed :)

cc @MarcusLongmuir

from ts-protoc-gen.

Thanks to @easyCZ's efforts in #80 we are now consistently using protoc v3.5.1 so we should be in a position to test and document this.

from ts-protoc-gen.

We use the jstype = JS_STRING field annotation extensively and are currently working to translate a project from flow to typescript. I forked the protoc-gen-flow project to simply output string instead of number for these cases and it worked fine. The generated JS will use the string variants of the conversion functions, so the *.d.ts files will actually be correct when they say string.

Here is a link of the changes required for the flow project. We'll be attempting the same for this project.
steckel/protoc-gen-flow@master...andcostello:master

from ts-protoc-gen.