-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Golang: Omitempty when field is required by some oneOf variants #2515
Comments
@vbqz could you elaborate on your expectations of what should be done? Seems to me that this would be expected. You have two events, both of which will have a non nil "kind", both of these events can have a "B" but only one of them has it being required. Thereby making it optional. To me this makes sense but perhaps I misunderstand you or perhaps it is I who is mistaken. |
I think you have the right idea of the schema, B is (conditionally) optional. The problem lies in the generated Go code. If I try to create a valid event like:
then the
|
@vbqz Okay then why use the omit-empty flag if you don't want it? Couldn't you just run: npx quicktype --src-lang schema -o ./event.go --just-types-and-package def.json |
@rekram1-node Sure, that is what we're doing right now. However I opened this issue since it seems like using --omit-empty generated incorrect code. The upside of --omit-empty would be that truly optional fields could be omitted. Avoiding null value noise for example seeing all fields of variants A or B when the items is actually variant C. |
@vbqz I see... I assume your expected generated code would be the following correct?: package main
type Event struct {
B *string `json:"b"`
Kind Kind `json:"kind"`
}
type Kind string
const (
One Kind = "one"
Two Kind = "two"
) |
@vbqz After looking at the omit empty flag description: And given that B is not required in all cases, I think that the behavior you are experiencing is the expected outcome at least in the current iteration. Perhaps it is too opinionated but given that description this would fall under that behavior. |
@rekram1-node Yes I think that would be the code. Yes I was just about to post the flag description myself. But I think we've exhausted this topic, it boils down to a difference of interpretation. |
Hi,
I've bumped into an unexpected interaction between oneOf and the new
--omit-empty
flag.I tried to condense the problem down to the example below. We're using
oneOf
to express variants, where the"b"
field is only required in some of them. I expected this "partially required" to overrule the flag, since makingB
omitempty
makes it hard to generate valid events of kind "one".Does this make any sense or am I missing some nuance in the schema rules?
Schema
Command (
quicktype version 23.0.104
):Outcome:
Thanks!
The text was updated successfully, but these errors were encountered: