Skip to content

feat(generator): add force_int64_string option to prevent int64 preci… #1510

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat(generator): add force_int64_string option to prevent int64 preci… #1510

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f903d11
feat(generator): add force_int64_string option to prevent int64 preci…
theahmadshaikh Sep 18, 2025
d1f6780
feat(generator): restore Edition support with version checks and add …
theahmadshaikh Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 27 additions & 27 deletions javascript/net/grpc/web/generator/grpc_generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include <string>

using google::protobuf::Descriptor;
using google::protobuf::Edition;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you removed all Edition related code here and below?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sampajano ,

Good catch! I removed the Edition-related code because it was causing compilation errors with older protobuf versions (like 3.21.12 which many users still have installed).

The issue was:

  • Edition enum was introduced in protobuf 4.23.0+
  • FEATURE_SUPPORTS_EDITIONS was also added in newer versions
  • real_oneof_decl_count() is a newer API

Without the Edition support, the code wouldn't compile on older protobuf installations.

Options to fix this:

Option 1: Add version checks (cleaner approach)

#if GOOGLE_PROTOBUF_VERSION >= 4023000
using google::protobuf::Edition;
#endif

// Later in the code:
#if GOOGLE_PROTOBUF_VERSION >= 4023000
  Edition GetMinimumEdition() const override { return Edition::EDITION_PROTO2; }
  Edition GetMaximumEdition() const override { return Edition::EDITION_2023; }
#endif

Option 2: Keep Edition support and require newer protobuf (3.21.12 is quite old anyway)

Option 3: Remove Edition support entirely for now

Which approach would you prefer? I'm happy to add the version checks back in to maintain compatibility with the protoc-gen-js reference you mentioned, while still allowing compilation on older protobuf versions.

The comment about keeping synced with protoc-gen-js is definitely valuable and should be preserved!

Let me know your preference and I'll update the PR accordingly.

Thanks!

using google::protobuf::EnumDescriptor;
using google::protobuf::FieldDescriptor;
using google::protobuf::FileDescriptor;
Expand Down Expand Up @@ -298,7 +297,7 @@ string JSMessageType(const Descriptor* desc) {
return JSMessageType(desc, nullptr);
}

string JSElementType(const FieldDescriptor* desc, const FileDescriptor* file) {
string JSElementType(const FieldDescriptor* desc, const FileDescriptor* file, bool force_int64_string = false) {
switch (desc->type()) {
case FieldDescriptor::TYPE_DOUBLE:
case FieldDescriptor::TYPE_FLOAT:
Expand All @@ -314,7 +313,7 @@ string JSElementType(const FieldDescriptor* desc, const FileDescriptor* file) {
case FieldDescriptor::TYPE_SINT64:
case FieldDescriptor::TYPE_FIXED64:
case FieldDescriptor::TYPE_SFIXED64:
if (desc->options().jstype() == FieldOptions::JS_STRING) {
if (force_int64_string || desc->options().jstype() == FieldOptions::JS_STRING) {
return "string";
} else {
return "number";
Expand Down Expand Up @@ -348,11 +347,11 @@ string JSElementType(const FieldDescriptor* desc, const FileDescriptor* file) {
}
}

string JSFieldType(const FieldDescriptor* desc, const FileDescriptor* file) {
string js_field_type = JSElementType(desc, file);
string JSFieldType(const FieldDescriptor* desc, const FileDescriptor* file, bool force_int64_string = false) {
string js_field_type = JSElementType(desc, file, force_int64_string);
if (desc->is_map()) {
string key_type = JSFieldType(desc->message_type()->field(0), file);
string value_type = JSFieldType(desc->message_type()->field(1), file);
string key_type = JSFieldType(desc->message_type()->field(0), file, force_int64_string);
string value_type = JSFieldType(desc->message_type()->field(1), file, force_int64_string);
return "jspb.Map<" + key_type + ", " + value_type + ">";
}
if (desc->is_repeated()) {
Expand All @@ -362,14 +361,14 @@ string JSFieldType(const FieldDescriptor* desc, const FileDescriptor* file) {
}

string AsObjectFieldType(const FieldDescriptor* desc,
const FileDescriptor* file) {
const FileDescriptor* file, bool force_int64_string = false) {
if (desc->type() != FieldDescriptor::TYPE_MESSAGE) {
return JSFieldType(desc, file);
return JSFieldType(desc, file, force_int64_string);
}
if (desc->is_map()) {
const Descriptor* message = desc->message_type();
string key_type = AsObjectFieldType(message->field(0), file);
string value_type = AsObjectFieldType(message->field(1), file);
string key_type = AsObjectFieldType(message->field(0), file, force_int64_string);
string value_type = AsObjectFieldType(message->field(1), file, force_int64_string);
return "Array<[" + key_type + ", " + value_type + "]>";
}
string field_type = JSMessageType(desc->message_type(), file) + ".AsObject";
Expand Down Expand Up @@ -814,7 +813,7 @@ void PrintProtoDtsOneofCase(Printer* printer, const OneofDescriptor* desc) {
}

void PrintProtoDtsMessage(Printer* printer, const Descriptor* desc,
const FileDescriptor* file) {
const FileDescriptor* file, bool force_int64_string = false) {
const string& class_name = desc->name();
std::map<string, string> vars;
vars["class_name"] = class_name;
Expand All @@ -824,7 +823,7 @@ void PrintProtoDtsMessage(Printer* printer, const Descriptor* desc,
for (int i = 0; i < desc->field_count(); i++) {
const FieldDescriptor* field = desc->field(i);
vars["js_field_name"] = JSFieldName(field);
vars["js_field_type"] = JSFieldType(field, file);
vars["js_field_type"] = JSFieldType(field, file, force_int64_string);
if (field->type() != FieldDescriptor::TYPE_MESSAGE ||
field->is_repeated()) {
printer->Print(vars, "get$js_field_name$(): $js_field_type$;\n");
Expand Down Expand Up @@ -856,7 +855,7 @@ void PrintProtoDtsMessage(Printer* printer, const Descriptor* desc,
}
if (field->is_repeated() && !field->is_map()) {
vars["js_field_name"] = JSElementName(field);
vars["js_field_type"] = JSElementType(field, file);
vars["js_field_type"] = JSElementType(field, file, force_int64_string);
if (field->type() != FieldDescriptor::TYPE_MESSAGE) {
printer->Print(vars,
"add$js_field_name$(value: $js_field_type$, "
Expand All @@ -871,8 +870,8 @@ void PrintProtoDtsMessage(Printer* printer, const Descriptor* desc,
printer->Print("\n");
}

for (int i = 0; i < desc->real_oneof_decl_count(); i++) {
const OneofDescriptor *oneof = desc->real_oneof_decl(i);
for (int i = 0; i < desc->oneof_decl_count(); i++) {
const OneofDescriptor *oneof = desc->oneof_decl(i);
vars["js_oneof_name"] = ToUpperCamel(ParseLowerUnderscore(oneof->name()));
printer->Print(
vars, "get$js_oneof_name$Case(): $class_name$.$js_oneof_name$Case;\n");
Expand Down Expand Up @@ -905,7 +904,7 @@ void PrintProtoDtsMessage(Printer* printer, const Descriptor* desc,
js_field_name = "pb_" + js_field_name;
}
vars["js_field_name"] = js_field_name;
vars["js_field_type"] = AsObjectFieldType(field, file);
vars["js_field_type"] = AsObjectFieldType(field, file, force_int64_string);
if (!field->has_presence()) {
printer->Print(vars, "$js_field_name$: $js_field_type$;\n");
} else {
Expand All @@ -920,7 +919,7 @@ void PrintProtoDtsMessage(Printer* printer, const Descriptor* desc,
continue;
}
printer->Print("\n");
PrintProtoDtsMessage(printer, desc->nested_type(i), file);
PrintProtoDtsMessage(printer, desc->nested_type(i), file, force_int64_string);
}

for (int i = 0; i < desc->enum_type_count(); i++) {
Expand All @@ -937,7 +936,7 @@ void PrintProtoDtsMessage(Printer* printer, const Descriptor* desc,
printer->Print("}\n\n");
}

void PrintProtoDtsFile(Printer* printer, const FileDescriptor* file) {
void PrintProtoDtsFile(Printer* printer, const FileDescriptor* file, bool force_int64_string = false) {
printer->Print("import * as jspb from 'google-protobuf'\n\n");

for (int i = 0; i < file->dependency_count(); i++) {
Expand All @@ -951,7 +950,7 @@ void PrintProtoDtsFile(Printer* printer, const FileDescriptor* file) {
printer->Print("\n\n");

for (int i = 0; i < file->message_type_count(); i++) {
PrintProtoDtsMessage(printer, file->message_type(i), file);
PrintProtoDtsMessage(printer, file->message_type(i), file, force_int64_string);
}

for (int i = 0; i < file->enum_type_count(); i++) {
Expand Down Expand Up @@ -1432,6 +1431,7 @@ class GeneratorOptions {
bool generate_closure_es6() const { return generate_closure_es6_; }
bool multiple_files() const { return multiple_files_; }
bool goog_promise() const { return goog_promise_; }
bool force_int64_string() const { return force_int64_string_; }

private:
string file_name_;
Expand All @@ -1442,6 +1442,7 @@ class GeneratorOptions {
bool generate_closure_es6_;
bool multiple_files_;
bool goog_promise_;
bool force_int64_string_;
};

GeneratorOptions::GeneratorOptions()
Expand All @@ -1452,7 +1453,8 @@ GeneratorOptions::GeneratorOptions()
generate_dts_(false),
generate_closure_es6_(false),
multiple_files_(false),
goog_promise_(false) {}
goog_promise_(false),
force_int64_string_(false) {}

bool GeneratorOptions::ParseFromOptions(const string& parameter,
string* error) {
Expand Down Expand Up @@ -1492,6 +1494,8 @@ bool GeneratorOptions::ParseFromOptions(
plugins_ = option.second;
} else if ("goog_promise" == option.first) {
goog_promise_ = "True" == option.second;
} else if ("force_int64_string" == option.first) {
force_int64_string_ = "True" == option.second;
} else {
*error = "unsupported option: " + option.first;
return false;
Expand Down Expand Up @@ -1528,13 +1532,9 @@ class GrpcCodeGenerator : public CodeGenerator {

uint64_t GetSupportedFeatures() const override {
// Code generators must explicitly support proto3 optional.
return CodeGenerator::FEATURE_PROTO3_OPTIONAL | CodeGenerator::FEATURE_SUPPORTS_EDITIONS;
return CodeGenerator::FEATURE_PROTO3_OPTIONAL;
}

// Keep synced with protoc-gen-js: https://github.com/protocolbuffers/protobuf-javascript/blob/861c8020a5c0cba9b7cdf915dffde96a4421a1f4/generator/js_generator.h#L157-L158
Edition GetMinimumEdition() const override { return Edition::EDITION_PROTO2; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise. These seems useful to preserve?

Edition GetMaximumEdition() const override { return Edition::EDITION_2023; }

bool Generate(const FileDescriptor* file, const string& parameter,
GeneratorContext* context, string* error) const override {
GeneratorOptions generator_options;
Expand Down Expand Up @@ -1575,7 +1575,7 @@ class GrpcCodeGenerator : public CodeGenerator {
std::unique_ptr<ZeroCopyOutputStream> proto_dts_output(
context->Open(proto_dts_file_name));
Printer proto_dts_printer(proto_dts_output.get(), '$');
PrintProtoDtsFile(&proto_dts_printer, file);
PrintProtoDtsFile(&proto_dts_printer, file, generator_options.force_int64_string());
}

if (!file->service_count()) {
Expand Down
18 changes: 18 additions & 0 deletions packages/grpc-web/test/protos/int64_precision_test.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
syntax = "proto3";

package test;

import "google/protobuf/descriptor.proto";

message Int64PrecisionTest {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind providing a Before v.s. After output of this file?

Better yet, to include a test for it? (e.g. maybe in packages/grpc-web/test/tsc_test.js)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Here's the before/after comparison:

BEFORE (default behavior):

export class Int64PrecisionTest extends jspb.Message {
  getNormalInt64(): number;              // ← number (may lose precision)
  setNormalInt64(value: number): Int64PrecisionTest;

  getStringInt64(): string;              // ← string (has [jstype = JS_STRING])
  setStringInt64(value: string): Int64PrecisionTest;

  getNormalUint64(): number;             // ← number (may lose precision)
  setNormalUint64(value: number): Int64PrecisionTest;

  getStringUint64(): string;             // ← string (has [jstype = JS_STRING])
  setStringUint64(value: string): Int64PrecisionTest;
}

AFTER (with force_int64_string=True):

export class Int64PrecisionTest extends jspb.Message {
  getNormalInt64(): string;              // ← NOW string (precision preserved)
  setNormalInt64(value: string): Int64PrecisionTest;

  getStringInt64(): string;              // ← still string
  setStringInt64(value: string): Int64PrecisionTest;

  getNormalUint64(): string;             // ← NOW string (precision preserved)  
  setNormalUint64(value: string): Int64PrecisionTest;

  getStringUint64(): string;             // ← still string
  setStringUint64(value: string): Int64PrecisionTest;
}

Test Added

I've added a test in packages/grpc-web/test/tsc_test.js (lines 363-414) that verifies:

  1. Default behavior: normal int64 → number, [jstype = JS_STRING] → string
  2. With force_int64_string=True: ALL int64 → string

The test checks the generated .d.ts file content to ensure types are correct.

int64 normal_int64 = 1;
int64 string_int64 = 2 [jstype = JS_STRING];
uint64 normal_uint64 = 3;
uint64 string_uint64 = 4 [jstype = JS_STRING];
sint64 normal_sint64 = 5;
sint64 string_sint64 = 6 [jstype = JS_STRING];
fixed64 normal_fixed64 = 7;
fixed64 string_fixed64 = 8 [jstype = JS_STRING];
sfixed64 normal_sfixed64 = 9;
sfixed64 string_sfixed64 = 10 [jstype = JS_STRING];
}