Adding enum values with a number after the prefix result in breaking change to previously generated values.

[mikeder]

When adding a new enum value that contains a number after the prefix, generated enums names will change to use the full "ENUM_PREFIX_NAME" style rather than the "Prefix.Name" value style.

Observed with both buf.build/bufbuild/es:v2.2.0 and current buf.build/bufbuild/es:v2.10.0, did not test in between versions.

Example:

Start with minimum basic enum:

syntax = "proto3";

package enum.test.v1;

enum ActionType {
  ACTION_TYPE_INVALID = 0;
  ACTION_TYPE_ONE = 1;
}

Results in this being generated:

/**
 * @generated from enum enum.test.v1.ActionType
 */
export enum ActionType {
  /**
   * @generated from enum value: ACTION_TYPE_INVALID = 0;
   */
  INVALID = 0,

  /**
   * @generated from enum value: ACTION_TYPE_ONE = 1;
   */
  ONE = 1,
}

Add a new enum value containing a number:

syntax = "proto3";

package enum.test.v1;

enum ActionType {
  ACTION_TYPE_INVALID = 0;
  ACTION_TYPE_ONE = 1;
  ACTION_TYPE_2 = 2;
}

Notice all of the generated values change format:

/**
 * @generated from enum enum.test.v1.ActionType
 */
export enum ActionType {
  /**
   * @generated from enum value: ACTION_TYPE_INVALID = 0;
   */
  ACTION_TYPE_INVALID = 0,

  /**
   * @generated from enum value: ACTION_TYPE_ONE = 1;
   */
  ACTION_TYPE_ONE = 1,

  /**
   * @generated from enum value: ACTION_TYPE_2 = 2;
   */
  ACTION_TYPE_2 = 2,
}

I know this is a bad example, since you shouldn't mix enum name styles in proto to begin with, but we recently had a somewhat justified reason for doing so and were surprised by a breaking change in our generated code, despite breaking change detection on the proto itself.

[timostamm]

Hey Mike,

the code generator follows this logic:

If all enum values share a prefix that corresponds with the enum's name, the prefix is dropped from all enum value names.

What you're running into is an edge case that isn't documented. We can't strip the prefix from ACTION_TYPE_2 because we would generate:

enum ActionType {
  INVALID = 0,
  ONE = 1,
  2,
}

That's a syntax error because identifiers cannot start with numbers. To avoid generating code that doesn't compile, the code generator bails, and doesn't strip the prefix.

Alternatively, the code generator could decide per enum value whether to drop the prefix. We should consider this (breaking) change for the next major version.

Please note there's a similar problem with nested types. Consider:

syntax="proto3";
message Foo {
  string foo = 1;
  message Bar {
    string foo_nested_bar = 1;
  }
}
message Foo_Bar {
  string foo_bar = 1;
}

Because the nested message Foo.Bar is generated with the name Foo_Bar, it would clash with message Foo_Bar. We avoid that by appending $ to the name. But since we don't know which one came first, such an addition can also lead to a breaking change.

[mikeder]

Thanks @timostamm for the follow up!

I do think that a note somewhere in the documentation to caution against this may be a good idea but I'm also wondering if it may be possible to catch with a linter? We generally don't scrutinize generated code during PR so I'm wondering if there are options for automated checks to catch this type of thing.

[jcready]

Too late now, but the nested types issue could've been avoided by using $ instead of _ to replace the . (e.g. Foo$Bar) since $ isn't allowed in protobuf identifiers, but is allowed in JS identifiers.

Also probably too late now, but the enum prefix stripping probably could've used the same trick by prepending $.

enum ActionType {
  INVALID = 0,
  ONE = 1,
  $2,
}

Or by just retaining the prefix for enum labels which would be an invalid TS enum label otherwise.

enum ActionType {
  INVALID = 0,
  ONE = 1,
  ACTION_TYPE_2,
}

To be fair, the enum prefix stripping (while convenient) is prone to breaking changes regardless. All it would take is for some enum value to be added with a non-conforming enum prefix. e.g.

 syntax = "proto3";

 package enum.test.v1;

 enum ActionType {
   ACTION_TYPE_INVALID = 0;
   ACTION_TYPE_ONE = 1;
+  FACTION_TYPE_FOO = 2;
 }

[timostamm]

wondering if it may be possible to catch with a linter?

@mikeder, buf lint supports custom lint plugins. It should be possible to implement a lint rule to catch this issue. For more info, see https://buf.build/blog/buf-custom-lint-breaking-change-plugins