-
-
Notifications
You must be signed in to change notification settings - Fork 309
Suggestion: optional keyword to complement required #1112
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
Comments
This seems like a duplicate/variant of #1096. |
I can see how this could work. It would work similarly to Something like this has been requested for a long time, but this is the first proposal of this type that I think fits well into the JSON Schema architecture and I think we should consider it. Requiring properties by default (#1096) and Although I hate to add more keywords that break keyword independence, I think this is a good solution. In particular, this wouldn't have the same kind of problems with combining schemas that |
Agreed. But this also means we need to work out what happens when they are used together: for example if one schema pulls in another with edit: a subschema keyword overriding a superschema keyword isn't even doable, since both are evaluated, each in their own contexts. keywords generally need to stand on their own when not siblings, unless we explicitly define an annotation behaviour that can carry state. |
When they are used together, they each assert independently just like any other keyword. That does mean it's possible for them to contradict each other, but not in any way that isn't possible already in JSON Schema. They don't need to interact or override each other. |
Grateful to hear my idea has legs! Happy you agree that it fits in without obviously breaking anything. The only edge case I can think of is accidentally adding a property to a schema, forgetting that |
I implemented Note: Things in my |
I don't believe doing multiple things is a problem as such. If there is a keyword does two things (an "authoring convenience keyword"), then as long as we also have keywords that do each of those things separately, there is no issue. The problem would be if keywords do multiple things and there's no way around it. For example, suppose "minLength" (which only applies when the input is a string), also required that the value be a string, or if it were a number, applied to numbers as well. We would have to craft a more complicated if/then statement (if type="string" then minLength: 1), which would be more difficult to reason about. |
Fair enough. I think your example is a bit of a stretch, but I agree with your point. However, I would still prefer a solution with a keyword that only does one thing. Requiring properties by default is off the table. A |
I would say that an "optional" keyword does at least two things. It's presence means changes the behavior of "properties" so that it requires the listed properties; except the ones listed by "optional". This seems more complicated to me from both an authoring perspective as well as an implementation perspective. If I understand correctly, these schemas have the same behavior: {
properties: {
"name": { type: "string" },
"phone": { type: "string" }
},
optional: [ "phone" ]
} Versus: {
requiredProperties: {
"name": { type: "string" }
},
properties: {
"phone": { type: "string" }
}
} The only advantage I see to an "optional" keyword is it allows you to group/order properties the way you want, instead of forcing you to group them by if it's required/optional. But this has no effect on the validation, it's purely an authoring choice. |
Not necessarily. |
I do like the idea of enforcing that both cannot be present. This can be enforced in the meta-schema. |
Yes, this is the correct way forward.
I agree with this too (but since the specification text itself will mention this restriction, it is not out of the question for an implementation to enforce this with code as well). |
The whole reason I implemented this keyword was to explore things like this. I can assure you that As for making authoring complicated, I don't see how that would be, but having a web based implementation available makes it easy for us all to try it out and evaluate the developer experience implications.
It could be done, but I consider this kind of thing out of scope. I see it as a linting responsibility. I'm sure there's no good reason to use both at once, but there's no reason they couldn't both be used. |
Brilliant. If I ever get back onto that project in a reasonable time frame I'll update the code to pull this schema and try it out. Hopefully, others will have an impetus to try it as well.
Happy to stay on the sidelines on this point, but I would observe that if one allows a situation, one should handle it; therefore it should be defined what happens if you do use both at once, or defined that you cannot. Or it should at least be defined that it is undefined, which hedges against someone in future coming up with a genius idea that we can put in here. |
Just to be clear, the
Each keyword would behave independently, just like any other keyword. If you had I thought of an example where it would sense to use both {
"$id": "https://example.com/schemas/base-object",
"type": "object",
"properties": {
"aaa": { "type": "string" },
"bbb": { "type": "string" }
},
"optional": ["bbb"]
} We want to extend this schema add a required property "ccc" and make the "bbb" property required. {
"$id": "https://example.com/extended-object",
"allOf": [{ "$ref": "/schemas/base-object" }],
"properties": {
"ccc": { "type": "string" }
},
"required": ["bbb"],
"optional": []
} The "bbb" property isn't declared in this schema's |
I thought of these two cases as well. Suppose
I do like the usage of having {
"properties": {
"bbb": true,
}
} |
This isn't correct. This is what I meant when I said it reads like a contradiction, but it's not.
I don't think |
This isn't intuitive to me, but it makes a lot more sense than what I was thinking at first. |
If this is confusing to us, it's going to be much more so for users. Maybe we can come up with a better name. Something like |
That behavior is also unintuitive to me, as illustrated by my examples. |
It's too bad that |
It's confusing framed like this, but it seems to me the outcome of applying rules like this basically means "required overrides optional", which is easy to understand.
If we expand this further:
We consider this to be:
Obviously, repeating that is invalid, but I see no sensible objection not to consider
As a result, it seems to be emergent behaviour that " |
The problem isn't that it's difficult to understand or to explain. The problem is that people will see the word "optional", assume they know what that means, and skip over the explanation of what it really means. The unintuitive part is that it doesn't mean what it sounds like it should mean. |
Yeah it’s a good idea, but calling it There’s got to be some other option like |
It's ironic that |
The best suggestion I have is |
I've seen people trying to do it this way though. |
I agree this is the best we have. I suggest we leave it open for a week, and barign further comments, call consensus and move to PR. |
My 2 cents is I greatly prefer requiredProperties vs requireAllExcept. I think it is a simpler concept to understand and makes reading and writing schemes much easier and more straight forward. For example, to say that all properties are required (a very common case) would require either:
In addition, one of the common mistakes that I see developer's make when writing schemas is forgetting to specify if something is required or not. This wouldn't solve that issue, it would just have the same problem with a different default behavior. Considering this addition alone, I think that requireAllExcept is the clearer of the names proposed. |
A proposal doc for this should be created. See #1450 for an example. |
In my opinion adding this option adds complexity without adding much value. These new optional keywords may not have schema value definitions in properties. If one really wants separate buckets for required and optional properties, my suggestion is to:
Which could be done in openapi v4.0.0 (codenamed moonwalk) referring to a new later version of the json schema spec which contains the above suggested changes Right now optional properties can be describe with properties defined only in properties and not in required. One can fully describe optional use cases right now, so in my opinion this new optional keyword is unneeded. |
Please be aware that OpenAPI is not the only consumer of JSON Schema. We have to consider how changes like this impact all consumers. Changes that benefit OpenAPI could make JSON Schema very painful for another consumer. |
Sorry, my mistake. The next major (breaking) release of json schema then is when I would propose adding optionalProperties + requiredProperties to meet the needs of the author of this issue |
any update? |
Progress on this issue ended up stalling out due to lack of consensus on a solution. I expect discussion to to resume once we get some other things worked out, but I wouldn't expect anything to be available in the near future. |
Now that we have the feature life cycle and proposal mechanism in place, development of this feature will need to continue per that process. Removing from the stable release development milestone and project. |
I don't write objects with optional properties most of the time. I tend to prefer to use polymorphism to allow one of several data structures. When I do have an optional field, it's often something like "comments" or "extra info", and even then I don't have any philosophical objection to simply making it required but allowing it to contain the empty string.
An
optional
keyword to complementrequired
would greatly benefit me (and hopefully others), without treading on the toes of people who are writing schemata with many optional values.My proposition is thus:
required
oroptional
may be providedrequired
with an empty arrayoptional
is provided, anything not mentioned is now requiredoptional
means everything is requiredHope that's both clear and a useful idea.
The text was updated successfully, but these errors were encountered: