ts-proto icon indicating copy to clipboard operation
ts-proto copied to clipboard

Drop shared enum prefixes

Open fgblomqvist opened this issue 3 years ago • 6 comments

It's very common to write your protobuf enums like this:

enum Foo {
   FOO_BAR = 0;
   FOO_BAZ = 1;
}

It would be nice if there was an option (and later the default) to drop the shared prefix (if there is one), so that the TS enums can be generated like this:

enum Foo {
   BAR = 0;
   BAZ = 1;
}

fgblomqvist avatar Mar 02 '21 18:03 fgblomqvist

Ah sure, sounds good I guess. I haven't worked in a schema with that convention, but seems useful, especially for longer enum names.

I'd be open for yet-another-config-flag to turn that on if you'd like to submit a PR.

stephenh avatar Mar 02 '21 21:03 stephenh

That convention is enforced by default by the Buf linter/style guide: https://docs.buf.build/style-guide/#enums But yes okay, sounds good. We're currently evaluating using this library, so we'll look into making a PR if the choice falls on ts-proto. But regardless, excellent work so far, this looks like one of the best libs for proto TS codegen out there🥇

fgblomqvist avatar Mar 02 '21 22:03 fgblomqvist

This would be a very welcome feature for .proto files that define multiple enums. The protobuf system disallows enums in the same scope that have values of the same name, which is what leads many to use the shared prefix approach. Otherwise you're met with this error:

Note that enum values use C++ scoping rules, meaning that enum values are siblings of their type, not children of it.  Therefore, "VALUE" must be unique within "package.service", not just within "EnumName".

TS does not make this enum scoping distinction, so being able to strip the shared enum prefixes would really clean up the generated code.

Jake-RoundrockIO avatar Aug 06 '21 15:08 Jake-RoundrockIO

I guess using of custom enum option is more idiomatic solution for this case

extend google.protobuf.EnumOptions {
    string json_omit_prefix = 61210;
}

enum MyEnum {
    option (json_omit_prefix) = "STATUS_";

    STATUS_UNKNOWN = 0;
    STATUS_ACTIVE = 1;
    STATUS_ERROR = 2;
}

I can try to find out how to implement it

doochik avatar Aug 06 '21 16:08 doochik

Fwiw I wouldn't be against some sort of cute flag like --ts-opt=dedupEnumPrefixes which would, for each enum:

  • Split a name like AAA_BBB_CCC on _
  • Find the longest common AAA_ or AAA_BBB_ prefix across all values and
  • Dropped that prefix from all enum values...

I agree @doochik that custom options would be more explicit, and custom option support in general would be great to have ... I've glanced at a few times but never had a strong enough reason to really figure it out/build it.

And, even with custom options, I personally kind of like "rule of thumb" conventions like a dedupEnumPrefix that would "just work" across all enums in a potentially large schema...

@fgblomqvist are you interested in working on a PR for such a flag?

stephenh avatar Aug 22 '21 20:08 stephenh

Sorry totally missed this message! Unfortunately I don't really have the bandwidth at the moment. Worth noting is that I did end up exploring https://github.com/timostamm/protobuf-ts as well, which feels very similar to this project. Don't mean to discourage you from working on this though, you clearly started this way before and have come a long way 🙂.

fgblomqvist avatar Oct 14 '21 09:10 fgblomqvist

This got implemented as an removeEnumPrefix option ~awhile ago.

stephenh avatar Aug 22 '23 02:08 stephenh