feat: support proto2 optional and default value fields

[lukealvoeiro]

Description

This PR aims to implement the features necessary to enable proto2 optional fields and support proto2 default values, requested by #973

The following changes are present in this PR:

• Code changes to main.ts and types.ts that implement optional fields, as well as default values.
• Creates two new options: disableProto2Optionals and disableProto2DefaultValues. By default these are set to false.
• New integration tests for proto3 and proto2 syntax types, intended to catch any regressions
• Modifications to existing integration tests - this mainly involves:
  • Updating message interfaces to use optional variables
  • Updating the createBaseMessage() function to use default values provided by the proto2 syntax.
  • The following two changes to the encode function of a message
  1. When checking boolean default values, following the other default checks by checking if not equals instead of equals:

     // BEFORE
     message.truth === true
     // AFTER
     message.truth !== false

  2. When default checking Long types using the forceLong=true option, always check via a .equals() instead of .isZero(). This could be reverted, however, I find it to be cleaner; regardless of whether there is a default value provided or not this check will always work.

     // BEFORE
     !message.key.isZero()
     // AFTER
     !message.key.equals(Long.ZERO)

Outstanding work

In a subsequent future PRs, we need to handle the following:

• Updating to ts-proto-descriptors to correctly render fields as optional. This involves running changing the parameters for protos/build.sh to using the build code from ts-protos. I tried doing this as part of this PR, but the updated proto descriptors did not work well with ts-proto. Going to re-attempt this at a later date.
• Fix the last of the default values: currently all but the Bytes type is handled correctly. I have added a todo(proto2) label around the places that need updating.

Past Discussion

Please ignore everything below, as it is out of date. I am keeping it within the PR description so below comments make sense to future readers.

Broken TypeScript in src

@lukealvoeiro: The code in src/ is currently broken as a result of the ts-proto-descriptors update, which made a lot of the fields typically available in interfaces such as ProtoFileDescriptor optional.

I'm planning on updating all the source code to use the optional syntax (not actually that hard, there are like 60 TS errors which I can probably knock out quickly), and then adding a new option that disables proto2 optional fields (e.g. disableProto2Optionals=true). This gives us better maintainability in the long run, brings us in line with proto2 conventions, and improves our parsing of proto files. It also allows customers who are used to ts-proto's existing output for proto2 files to continue to codegen the same TS output via the disableProto2Optionals flag.

As I mentioned above, I tried do this but didn't get very far down this path. I think in general it makes sense to update ts-proto-descriptors to use the optional fields (and default values), however, it was harder than expected to get the updated descriptors working with ts-proto

File-specific context

@lukealvoeiro: One of the not-so-elegant aspects of this PR is visible in main.ts, where I am passing a parameter isProto3File: boolean around a lot. Wanted to open up a discussion as to whether it would be helpful to insert file-specific context such as isProto3File into the Context object. After we process each file, we could then overwrite this portion of the context with the next file's metadata.

I implemented this change, folks can continue adding to the file context whenever appropriate.

Testing performed

• Ran unit tests, changes to files are expected and compile / pass tests
• Ran on my own codebase that has proto2 and proto3 files and noticed no issues there either.

[lukealvoeiro]

@stephenh Would like to get your thoughts on the Questions / Considerations section above.

[stephenh]

Hi @lukealvoeiro ; apologies for taking awhile to get back to you. I knew this PR would take more-than-a-few-minutes to give intelligent feedback on :-), so has been hard to find the time for it.

Everything in your plan sounds good to me!

In terms of passing around isProto3File, yeah, I had also thought of "would be nice to use Context". but had assumed we couldn't do that since Context is global. But you're right, we only handle one file at a time anyway, so we could mutate a context.isProto3File flag (or make a copy of Context for each file if we really had to), so I like that idea!

If you can continue on this path, I think it all sounds great and I'll merge everything when you're ready! Thank you!

[stephenh]

Just sanity checking my understanding here:

The code in src/ is currently broken as a result of the
ts-proto-descriptors update, which made a lot of the fields
typically available in interfaces such as ProtoFileDescriptor optional.

You're saying that like, on main today, the code src/ is fine, but if we merge this PR, and bump ts-proto-descriptors, because the protoc *.proto files are all proto2-based, then the next version of ts-proto-descriptors will break a bunch of src/ code, b/c we'll see optional fields as actually optional.

If I'm following, that makes sense.

I'm planning on updating all the source code to use the optional syntax
(not actually that hard, there are like 60 TS errors which I can probably
knock out quickly)

Right, cool!

and then adding a new option that disables proto2 optional fields (e.g.
disableProto2Optionals=true)

Ah okay, so you're thinking this would basically be a "force proto3 mode" type flag, where *.proto files that are technically proto2, would get ts-proto types that matches the proto3 behavior of optional fields being non-optional/look required?

Fwiw I could kind of go back/forth on this one; part of me thinks that, well, if you're using proto2, this is just how proto2 is supposed to work. And also proto3 ended up walking back their "fields are always set" and re-introduced optional fields.

But I don't really feel strongly about it either. Like if this is just about making ts-proto's own use of ts-proto-descriptors easier, I don't think it's that important.

But if you've personally got a lot of proto2 files that you'd prefer their optional fields to be "more like proto3", then that sounds good to me.

[lukealvoeiro]

Thanks for the feedback @stephenh! This PR should be ready to merge, unless you think I should modify parts of it. I updated the description above to better match the recent commits. Replying to some of your questions:

You're saying that like, on main today, the code src/ is fine, but if we merge this PR, and bump ts-proto-descriptors, because the protoc *.proto files are all proto2-based, then the next version of ts-proto-descriptors will break a bunch of src/ code, b/c we'll see optional fields as actually optional.

If I'm following, that makes sense.

Yup, thats exactly it. I tried updating the ts-proto-descriptors like I mentioned but ran into more difficulties than anticipated. As a result, I've left the ts-proto-descriptors as is by generating them using the "force proto3 mode" described below.

Ah okay, so you're thinking this would basically be a "force proto3 mode" type flag, where *.proto files that are technically proto2, would get ts-proto types that matches the proto3 behavior of optional fields being non-optional/look required?

Fwiw I could kind of go back/forth on this one; part of me thinks that, well, if you're using proto2, this is just how proto2 is supposed to work. And also proto3 ended up walking back their "fields are always set" and re-introduced optional fields.

But I don't really feel strongly about it either. Like if this is just about making ts-proto's own use of ts-proto-descriptors easier, I don't think it's that important.

But if you've personally got a lot of proto2 files that you'd prefer their optional fields to be "more like proto3", then that sounds good to me.

I don't think I have strong feelings either way, just didn't want to introduce breaking changes to folks who are used to proto2 files behaving like proto3 ones. For the time being, since we need to force proto3 for the ts-proto-descriptors, I've kept the flag.

[lukealvoeiro]

This changed because the file imports at proto2 message, hence the correct default value is being picked up.

[stephenh]

This looks great @lukealvoeiro !

One really minor question about the readme updates, but otherwise looks great to merge, thank you!

[stephenh]

This again looks great @lukealvoeiro ! Thank you!

[stephenh]

🎉 This PR is included in version 1.169.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release
• v1.169.0

Your semantic-release bot 📦🚀