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

Support the jstype field option #556

Closed
timostamm opened this issue Aug 30, 2023 · 7 comments · Fixed by #592
Closed

Support the jstype field option #556

timostamm opened this issue Aug 30, 2023 · 7 comments · Fixed by #592
Labels
Feature New feature or request

Comments

@timostamm
Copy link
Member

In protobuf, fields with a 64-bit integral type can be annotated with the jstype field option:

syntax="proto3";

message Example {
    int64 normal = 1;
    int64 string = 2 [jstype = JS_STRING];
    int64 number = 3 [jstype = JS_NUMBER];
}

The reason this option exists is that the JavaScript Number primitive cannot safely represent the full range of 64-bit integers. The option predates BigInt, but since BigInt has drawbacks for some users, we can honor this option to represent 64-bit integers with String.

The buf CLI will support setting this field in managed mode (see this related PR).

@timostamm timostamm added the Feature New feature or request label Aug 30, 2023
@tannerlinsley
Copy link

Is there any consideration going into a global setting that would only change the generated types and not require changes to the schema? My backend team will probably not sign off on this request...

@timostamm
Copy link
Member Author

Definitely considered. You'll be able to set the option for every relevant field - without changing proto files - with managed mode.

Will most likely need just two lines in your buf.gen.yaml:

managed:
  enabled: true
  override:
+   - field_option: jstype
+     value: JS_STRING

I think this should be reasonable?

@tannerlinsley
Copy link

This looks nice, but I'm also curious why there wouldn't be something in the override to denote the field type we're overriding. Without the implementation context, it would appear that every field would be changed to a string, not just int64s. Maybe something like this?

managed:
  enabled: true
  override:
+   - field_type: int64
+   - field_option: jstype
+     value: JS_STRING

@tannerlinsley
Copy link

Or another option (just brainstorming here) would be changing jstype to int64jstype?

@timostamm
Copy link
Member Author

We're working with the field option defined in descriptor.proto here. I agree that it's not the best name, but it is unlikely that we can rename the option.

@derekperkins
Copy link

I didn't realize this was a core protobuf definition, but it explains why the linked PR is for buf and not protobuf-es. We were really surprised to see it there originally. Glad there's a path forward!

@timostamm
Copy link
Member Author

@tannerlinsley, @derekperkins, this feature has just been released in v1.4.0.

The managed mode feature from bufbuild/buf#2311 is not available yet, but the option can of course be manually specified on fields.

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

Successfully merging a pull request may close this issue.

3 participants