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

Feature request: InternalBinaryWrite configurable to force writing non optional fields #194

Open
ivalduan opened this issue Dec 15, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@ivalduan
Copy link

Looking at the spec of proto3 the behavior of not writing fields when the value is equivalent to the default is expected

Also note that if a scalar message field is set to its default, the value will not be serialized on the wire.

In order to be compatible with proto2 serialization it would be great to have an option that allows the override of this behavior and serializes always non-optional fields, the changes required seem to be fairly small. It could be even automatic detecting the syntax statement when reading proto.

Let me know if a PR related with this would be of interest.

@timostamm
Copy link
Owner

Full proto2 support requires quite a few changes, see https://github.com/timostamm/protobuf-ts/blob/master/MANUAL.md#proto2-support

I think it would translate into new code paths for the plugin, the runtime, and in the generated code as well. This will increase complexity quite a bit, and might also impact performance for the reflection based operations.

Maybe there is a way to selectively only special case proto2 required fields, but I'm not sure what exactly this would entail, and I'm wondering how far it would get us towards full proto2 compatibility. The conformance tests unfortunately only cover proto3.

Do you have a use case for serializing proto2 messages?

@ivalduan
Copy link
Author

ivalduan commented Dec 18, 2021

In this case I need to be compatible with some proto2 schemas between different programming languages. The only incompatibility encountered using this framework was some required fields not being written on serialization when their value is the default, but reading the provided link I see it being a bit more involved. I think this point 4 sentence is not accurate, the framework infers and generates correctly all interface types.

  1. required label is not supported: If a field has the required label, the type will be myField: bool with the default value false.

I was able to move forward by adding some quick and dirty code to InternalBinaryWrite.scalar. If protobuf-ts prefers to stay on proto3 only support what framework would you recommend for proto2 typescript code generations?

@timostamm
Copy link
Owner

It looks like the official implementation and protobuf.js both support proto2. I recommend going with one of them. However, if you have a migration to proto3 on your roadmap somewhere, I would personally use protobuf-ts with a patch.

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

No branches or pull requests

2 participants