Don't set defaults for optional fields

[rakyll]

Protobuf.js shouldn't set defaults for optional fields while encoding a message, rather set defaults during decoding if not present -- and provide has('fieldname') to help users to distingush if it's an explicit set or set by default. I'm not able to use this module to talk to Google's Datastore API.

[dcodeIO]

Hope this helps. It now presets the default values for required fields only, leaving optional fields with a value of null. On decode, optional defaults are populated if the field is not present.

I believe that checking if there is an optional default should be done via reflection, though. Like looking for the default option and testing against null.

[rakyll]

Thanks for the quick fix, is working for my case at the moment.

[deepakmerugu]

Can you please explain how to check if a field has been set. Currently, I'm observing that if a field has a default value and is not set, then on decoding, the object contains the default value. How can we distinguish this from the case where the field was indeed set with the same value as the default?

Thanks!

[dcodeIO]

See: #200

[AoJ]

Documentation on https://developers.google.com/protocol-buffers/docs/proto#optional say:

Optional Fields And Default Values

As mentioned above, elements in a message description can be labeled optional. A well-formed message may or may not contain an optional element. When a message is parsed, if it does not contain an optional element, the corresponding field in the parsed object is set to the default value for that field. The default value can be specified as part of the message description. For example, let's say you want to provide a default value of 10 for a SearchRequest's result_per_page value.

optional int32 result_per_page = 3 [default = 10];

If the default value is not specified for an optional element, a type-specific default value is used instead: for strings, the default value is the empty string. For bools, the default value is false. For numeric types, the default value is zero. For enums, the default value is the first value listed in the enum's type definition.

[dcodeIO]

I somewhat feel like setting optional fields without a default value to something else than null or undefined in JS is not the right thing to do as it neglects the advantages of the dynamic type system in place. However, the official documentation clearly states to do so.

What do you prefer and why?

[AoJ]

If I set the default value, I expect to always get a string/number/whatever not null. The same behavior has an official java implementation.

Subjectively, I have a Messages with several hundred Fields, adding "null" causes unnecessary overhead. It causes complications if you want to convert to json output for example. Usually, the application only works with small subset of data.

[dougrad]

Primitive accessors should always return a value even for optional fields.

The ProtoBuf.js implemention of default values violates this compared to the C++ and Go protobuf libs. It seems like the problem is trying to make direct field accesses give the same result as calling the accessor function; in C++/Go they do not.

The actual fields should always store the explicit value if present during deserialize or stored by an setter. The spec does not say that the default value should be stored -- just that it should be returned by the accessor if there was a null value stored.

In addition, ProtoBuf.js violates the last paragraph of the spec that AoJ quoted in above. For primitive fields without a default value, the accessor does not return the type-specific default.

To be true to ProtoBuf author's intentions, getters/accessors should be generated as:

get_xyz() {
   if ('xyz' in this and this.xyz != null)
      return this['xyz'];
   if (proto has default value for xyz) 
      return proto default;
   return type-specific default.
}

[jeremyong]

Out of curiosity, why was this closed? As far as I can tell, this is still an issue. I'm not sure I understand the design decision to not inject default values if not present (and subsequently ignore default values when encoding). Or at least provide the default value in the message prototype.

[Cristy94]

Any updates?

This is very annoying when used with TypeScript, because the type definition says it can be undefined (being an optional field), but it is never undefined as the default value will be (for strings) empty string. Currently multiple bugs occurred because I was checking stringVariable !== undefined instead of the (inintuitive) stringVariable !== ''. It's even worse when trying to send optional integers (trying to send both 0 and undefined will both be 0 on the receiving end).

[dcodeIO]

Well, this is from 2014. Things have changed a lot since then. If you have a specific issue with the current version, please open a new issue.

[Cristy94]

Ok, thanks! I think it is strange that this issue was closed even though the problem was never fixed. On Tue, Dec 26, 2017 at 1:43, Daniel Wirtz<notifications@github.com> wrote: Well, this is from 2014. Things have changed a lot since then. If you have a specific issue with the current version, please open a new issue. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.