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

protobuf.js decode object missing some field when the field have default value #521

Closed
dbskccc opened this Issue Dec 7, 2016 · 8 comments

Comments

Projects
None yet
3 participants
@dbskccc

dbskccc commented Dec 7, 2016

with protobuf 6, all field with default value will not encode to serialized data, but when protobuf.js decode it, the generated object also not have such field, since we don't know which field will have default value,
we have to check every field for undefined and assign a default value to further process(display,calculate), it's somewhat annoying, it should be an parameter on decode function to guide protobuf.js to fill the default field value.

test with protobuf.js 6.0.1

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 7, 2016

With protobuf.js 6, default values are on the respective prototype. Hence, when not present on the wire, accessing the field's value on a runtime message (received from decode, hence not a plain object) will still return the default value, while Object#hasOwnProperty called for the field will however return false.

It's possible that there are still bugs, though, so if you have an example of what is not working, please let me know!

@arthurtoper

This comment has been minimized.

arthurtoper commented Dec 7, 2016

From my testing, calling asJSON() on the decoded message results in a JS object lacking fields with default values. Is this by design?

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 7, 2016

asJSON does not include properties inherited through the prototype chain, just as JSON.stringify does. And, well, default values are on the prototype.

It should be rather trivial to add another option to also return default values, though.

dcodeIO added a commit that referenced this issue Dec 7, 2016

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 7, 2016

Only exception for options.defaults here is repeated fields because we can't use empty arrays on the prototype as a default value (one would modify the array on the prototype instead of creating a new one on the instance through myField.push(...) and similar). For that reason, repeated fields that are not present are always undefined.

@dbskccc

This comment has been minimized.

dbskccc commented Dec 7, 2016

oh, it's my fault not test much, first i encounter undefined on repeated field, and console.log not show normal field with default value, so i concluded that default value field is all undefined.

now, can we have the option to make the repeated empty field has a [], not undefined, since some code thought it as an array and access it's property and just crash.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 7, 2016

Currently trying out to initialize prototype instances with repeated field arrays, but this of course removes the possibility to check for actually present fields through just calling hasOwnProperty when it comes to repeated fields. Need to think about this a bit.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 7, 2016

Array and object defaults are now also on the prototype. This should fix the issue.

Caveat: You have to make sure to set new objects for these defaults on the actual instances. You can't just Array#push or obj.something = 123; on these objects. For example, if your object has a repeated field and you want to set values for it, use myMessage.myRepeatedField = [ "a", "b" ] instead of trying to modify the object. Once you have assigned a new object, you can use it just normally, including pushing more values etc.

Internally, modifications to these objects are prevented by running them through Object.freeze. In case of Array#push, this throws an error, while obj.something = 123; fails silently (such objects are actually only relevant with map fields).

@dbskccc

This comment has been minimized.

dbskccc commented Dec 8, 2016

tests ok with v6.1.0
thanks, now it's more comfortable to process protobuf message.

@dbskccc dbskccc closed this Dec 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment