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

fix: enum type returns 'UNRECOGNIZED' or '-1' in xxxToJSON/xxxToNumber #566

Merged
merged 1 commit into from
May 18, 2022

Conversation

xalanq
Copy link
Collaborator

@xalanq xalanq commented May 16, 2022

Is returning 'unknown' or '0' in xxxToJSON/xxxToNumber as expected? It may conflict with the default enum value '0'

I'm not sure, so I correct it to 'UNRECOGNIZED' and '-1'

wdyt?

Copy link
Owner

@stephenh stephenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, @xalanq I think this is a good change!

My vague recollection is that the return UNKNOWN / return 0 code is from a long time ago and just never got updated to know about the unrecognizedEnum setting.

Reading the proto3 docs, I think technically there are use cases where ideally ts-proto / protobuf libraries would leave the unrecognized enum in place, like if a service accepts a message with Color.BLUE, and the service itself is on an older version of the schema w/o BLUE, then it's internal code would see UNRECOGNIZED ... but if it didn't mutate the message, and just re-serialized to some other client (like a database), then BLUE would still be in the serialized JSON.

This is a somewhat niche use-case though, and there would be a lot more work, not just with enums, but all fields iirc, to really support it, so just mentioning it out of thoroughness, but otherwise I think this approach is good and definitely better than what we had been doing.

Thanks!

@stephenh stephenh merged commit 19911a1 into stephenh:main May 18, 2022
stephenh pushed a commit that referenced this pull request May 18, 2022
## [1.112.2](v1.112.1...v1.112.2) (2022-05-18)

### Bug Fixes

* enum type returns 'UNRECOGNIZED' or '-1' in xxxToJSON/xxxToNumber ([#566](#566)) ([19911a1](19911a1))
@stephenh
Copy link
Owner

🎉 This PR is included in version 1.112.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants