protobuf-es icon indicating copy to clipboard operation
protobuf-es copied to clipboard

Issue with protoc-gen-es when using hyphens in json_name

Open nicole0707 opened this issue 1 year ago • 1 comments

Describe the bug

When using protoc-gen-es with json_types=true option, if the value of json_name includes a hyphen(e.g. "x-id"), it causes errors in the generated Typescript or Javascript files.

To Reproduce

 /**
   * @generated from field: string x_id = 101 [json_name = "x-id"];
   */
  x-id?: string;

Additional context The generated code should correctly handle the hyphen without causing syntax errors.

nicole0707 avatar Oct 23 '24 09:10 nicole0707

Great find!

We're already using string literals for names that aren't safe here, but clearly the logic is incomplete. It only uses string literals if the name starts with a digit or contains an @.

MDN says:

In JavaScript, identifiers are commonly made of alphanumeric characters, underscores (_), and dollar signs ($). Identifiers are not allowed to start with numbers.

For these common identifiers, it's most user-friendly to continue generating them as simple property names. I think it's reasonable to simply generate any other name with a string literal.

We should add test cases to packages/protobuf-test/extra/json_types.proto:

  bool json_name_ok = 30 [json_name = "Foo123_bar$"];
  bool json_name_at = 31 [json_name = "foo@"];
  bool json_name_hyphen = 32 [json_name = "foo-bar"];
  bool json_name_start_with_digit = 33 [json_name = "1foo"];
  bool json_name_space = 34 [json_name = "foo bar"];
  bool json_name_tab = 35 [json_name = "foo\tbar"];
  bool json_name_non_ascii = 36 [json_name = "你好"];
  bool json_name_escape = 37 [json_name = "foo\nbar\\n"];

timostamm avatar Oct 28 '24 10:10 timostamm

@timostamm FYI I have raised a PR for the issue https://github.com/bufbuild/protobuf-es/pull/1004

nicole0707 avatar Nov 01 '24 05:11 nicole0707

Fixed by https://github.com/bufbuild/protobuf-es/pull/1004

timostamm avatar Nov 01 '24 11:11 timostamm