-
-
Notifications
You must be signed in to change notification settings - Fork 299
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
Move "definitions" to core (as "$defs"?) #512
Comments
Moving this into core sounds like a good idea. Core makes much more sense as a logical home for the definitions keyword, as the keyword itself doesn't actually imply any action - it's simply a well-known location where schema definitions may be stored. In order to maintain consistency with other core keywords, it seems sensible to rename it to |
@erayd I suppose there wouldn't be any harm in officially reserving |
@handrews That sounds like a good idea. |
On a related note, would it also be possible to specify that arbitrary schema fragments SHOULD NOT appear outside of The particular use-case I'm thinking of is a validation implementation which compiles a schema definition into procedural logic for direct execution against a document. Any circumstance which results in an inbound remote reference to a nonstandard location would necessarily imply lazy compilation, which adds significant implementation complexity and may be highly undesirable - in such cases, this would allow the implementation to ignore inbound references to a nonstandard location. |
@erayd I don't think we should forbid it (although the thought has actually crossed my mind before, so I get your point here). But there's nothing preventing an implementation from saying "if you only put I definitely think that reaching into sub-pieces of other schemas (like referencing a particular property schema) is bad form- factor the subschema out to Also, someone writing an extension vocabulary may come up with a good reason to have another location for schemas. |
"definitions" is basically the same thing as a comment. I don't see a problem with this. |
@handrews funny how similar this thread is to what I was discussing with you earlier and I hadn't seen this at that point :) |
Sounds like the main question is whether to add the I had thought not, but am warming to the idea. It would place |
Having it in core makes sense. Not so sure about renaming to $definitions. That means that the references should become {
"properties": {
"foo": {"$ref": "#foo"}
},
"definitions": {
"foo": {
"$id": "#foo"
}
}
} In any case I don't like renaming to $definitions because refs will become uglier... It does make sense from the consistency point of view though. Maybe let's move to core to begin with and think longer about renaming. Unlike other keywords with $, definitions does not have any direct effect on the schema processing, it just provides the location for other schemas. It's the argument not to rename. |
I agree with this. Technically someone could have a reference like |
Further thoughts on naming: As @epoberezkin Then again, Python uses
I'm not dead set on the |
I like the idea of |
If you're worried about obviousness of an abbreviated word, |
@handrews to be honest, I don't like the look of "#/$", it looks very bash-like. Given that definitions on their own have no impact on validation, they just mark the location in the schema, I'm not sure why we need to rename anything. Users are free to use whatever they choose, really. |
I'd like to repeat my comment on issue #505 here: IMHO, as far as
With current Yet, they are somewhat different:
Basically I agree with @handrews that one should not cross-reference from one schema into others - this results IMHO in an ugly spaghetti thing. Yet, factoring out any and all schema parts, that are used more than once, into a separate file definitely rips apart closely coupled things. I think it's fine to put such building blocks into a "global" location whitin the same schema file - this is what As a result I'd like to suggest the following for consideration (applicable aspect in square brackets):
Pros:
Cons:
Effects on implementation I can't assess because I'm not familiar with it. |
@semanino Please don't copy paste walls of text which are asking for things not about this issue. Please keep on topic where possible, and reference your comment using a link rather than pasting. Please edit your comment to reflect this. I can see it's slightly different. |
Decision:
Renaming to shorthand fits with $refs. |
I'm a little wary of renaming things if we don't have to. We could keep I think this is more impactful than "$id" since this seems to explect people to change their URIs. And of course, Cool URIs Don't Change. |
@awwright it's more of a recommendation than a requirement. I think it would go something like this:
Or something like that. We definitely want to avoid the appearance of telling people to reorganize everything Just Because. I must confess, I hate typing |
This was done as PR #570, but I forgot to close the issue at the time. |
In #505 I commented that
definitions
fits better into core than validation (and @Anthropic agreed).The point of
definitions
is simply to reserve a keyword under which you can safely collect named schemas, without worrying that they will get accidentally interpreted by an extended vocabulary.definitions
also allows packaging multiple schemas in a file for re-use as$ref
targets without having to explicitly assign$id
s. In that sense, it is most closely associated with$ref
and$id
, both of which are part of core.Furthermore, having a designated re-use location has nothing to do with validation or any other particular JSON Schema use case. Reading through the validation spec it seems quite out of place. For the last several drafts, there has even been a specific note in its description that it does not affect validation.
When you have to explain how the keyword has nothing to do with the purpose of the specification, that's probably a sign that the keyword is in the wrong place.
Anyone seriously object to moving this?
The text was updated successfully, but these errors were encountered: