Open vbqz opened 7 months ago
@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.
Hi @rekram1-node
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:
Event {
Kind: One,
B: nil, // OK since bar can be "string" or "null".
}
then the omitempty
tag on B
will exclude my B: nil
from the marshalled payload, it is simply not there. And then this payload does not fulfil any of the oneOf
variants. In other words
we get -> { "kind": "one" }
we want -> { "kind": "one", "b": null }
@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:
omit-empty If set, all non-required objects will be tagged with
",omitempty" (off by default)
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.
I'd argue the opposite since b
is in a required
list (albeit in a oneOf) it should not count as "non-required".
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!