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

feat: support $unknownFields #1912

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

indutny-signal
Copy link

This add $unknownFields property to generated code. It is populated on .decode() when decoder encounters currently unknown field, and similarly used by encoder to serialize unknown fields as they are into a binary data.

If user stores unknown fields and then upgrades their protobufs to the latest spec - there is a chance that some of the previously unknown fields will become known. This is handled in the encoder by attempting to decode the fields first.

The name of the field starts with $ because it is not a valid character for a field name according to protobuf v3 specification.

This add `$unknownFields` property to generated code. It is populated on
`.decode()` when decoder encounters currently unknown field, and
similarly used by encoder to serialize unknown fields as they are into a
binary data.

If user stores unknown fields and then upgrades their protobufs to the
latest spec - there is a chance that some of the previously unknown
fields will become known. This is handled in the encoder by attempting
to decode the fields first.

The name of the field starts with `$` because it is not a valid
character for a field name according to protobuf v3 specification.
@alexander-fenster
Copy link
Contributor

Thanks @indutny-signal! We'll need some time to review this one but we'll try to do it soon.

@indutny-signal
Copy link
Author

Thanks a lot! FWIW, most of the changes are just generated code in tests. I can probably rip it out of the Pull Request if you'd like!

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

Successfully merging this pull request may close these issues.

None yet

2 participants