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

undefined instead of null for default values #80

Closed
kjvalencik opened this issue Aug 8, 2017 · 3 comments
Closed

undefined instead of null for default values #80

kjvalencik opened this issue Aug 8, 2017 · 3 comments
Labels

Comments

@kjvalencik
Copy link
Collaborator

Many of our protobuf objects end up being eventually serialized as JSON. Currently, message fields have default values of null, which gets serialized into the result.

This is problematic because most specs for defining schema in JSON (e.g., swagger) define optional fields as being omitted from the result. This forces the need to prune null values before serialization in order to adhere to spec.

What are your thoughts on changing the default value of these types to undefined? What about a compile time option for deciding?

pbf --default-defined ./thing.proto
@mourner
Copy link
Member

mourner commented May 3, 2018

@kjvalencik how much of a perf hit would it be to roll-back our optimization of predefined properties? The issue with just using undefined is that properties would still be enumerable, so e.g. if a consumer does for (const key in obj), it would go over the undefined properties. Maybe we should just accept the perf hit and strive for maximum compatibility here.

@kjvalencik
Copy link
Collaborator Author

That's an interesting thought and worth benchmarking. I don't think it would be substantial for small messages, but ones with many keys would go through a lot of hidden class transitions.

@mourner
Copy link
Member

mourner commented Jul 4, 2024

This was fixed by #112 as far as I understand.

@mourner mourner closed this as completed Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants