ipfs / protons

Protocol Buffers for Node.js and the browser without eval
Other
32 stars 23 forks source link

Handling of default values do not conform with proto3 language guide #43

Closed D4nte closed 2 years ago

D4nte commented 2 years ago

Proto3 language guide states:

When a message is parsed, if the encoded message does not contain a particular singular element, the corresponding field in the parsed object is set to the default value for that field.

Looking at protons's code and behaviour, it is clear that:

Default values are type-specific:

  • For strings, the default value is the empty string.
  • For bytes, the default value is empty bytes.
  • For bools, the default value is false.
  • For numeric types, the default value is zero.
  • For enums, the default value is the first defined enum value, which must be 0.
  • For message fields, the field is not set. Its exact value is language-dependent. See the generated code guide for details.

I see that the tests are done against pbjs, this package states:

Unlike other JavaScript implementations, this library doesn't write out default values.

Is the choice to not follow the proto3 language guide regarding default value on purpose in protons?

Keen to open a PR, I can see two ways forward:

  1. Follow proto3 language guide: do not encode a field when it's set to a default value, set a default value when a field is absent as decoding
  2. Enable some way to parameterize this behavior (guidance would be welcome here).
D4nte commented 2 years ago

Note that I am doing a PR where I add a CLI option --no-default-on-wire that conforms to the proto3 language guide behaviour when passed. should be ready by EOW.

BigLep commented 2 years ago

2022-09-02 triage conversation:

Next steps: we need to agree on how we're denoting:

  1. presence of values and
  2. default values

Maintainers need to look at what's been described in this issue and cross reference with protobuff documentation and other implementations.

fryorcraken commented 2 years ago

This should help : https://buf.build/blog/protobuf-language-specification

https://www.protobuf.com/docs/language-spec#field-presence-and-default-values

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version protons-runtime-v4.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version protons-v6.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: