-
-
Notifications
You must be signed in to change notification settings - Fork 315
Clarify usage of $ref with properties not provided in definitions #1097
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
That is highly dependent on whose schemas you look at. Pretty much every large project I've seen (ranging from internal APIs to a large biomedical information system) uses Do you specifically need draft-04 explanations? The way I'm not quite sure what "canonical response" means here, but if I were working with these schemas they wouldn't look like this at all. Anything re-usable would be factored out under |
Can you provide a link to where they use JSON Schema. That page is an examples site where the only code that's displayed is XML/HTML |
To show an example of |
@gregsdennis there is a tab in the bottom frame of the page labeled |
This is a great way of looking at it, thanks @handrews. My point in the slack discussion was that using While $ref-ing schemas that aren't in $defs/definitions is valid, it should be considered a code smell. Regarding the "Understanding JSON Schema" link, that example with The other sections do explain how the JSON Pointer fragment behavior works with $ref. They just don't include examples with Maybe a section on JSON Pointers could be worth while? It's probably out of scope for that document, but worth considering. |
Thanks for the productive responses team.
Does this need to change then, based on the phrasing in the maintainers responses here?
This is not necessary, as it is not the problem we are attempting to solve. Thanks though.
No, latest is fine.
So it sounds like the docs need to be updated to strongly discourage usage here? As it stands, there are multiple massive codebases by different companies I am responsible for consuming in production environments that do not use Since the spec currently does not require
|
Thanks. I see that now. But what do they do when they want to compose these components into a larger schema, e.g. to represent a form? It looks to me that they've elected to store their component schemas in separate files. My guess is that they'd just
The (8.2.4) The "$defs" keyword reserves a location for schema authors to inline re-usable JSON Schemas into a more general schema. Reserving a location for this purpose is sufficient. We generally try to leave authorship options open. If a schema author wants to (or has good reason to) I'm not sure anything needs to change in the spec. This
is not going to be an acceptable outcome from this issue. If the "how to schema" site could use more clarification in some areas, that's fine. |
This is less straightforward than "not required by the spec" implies, although that is not at all obvious from draft-04. Really, I advise never looking at draft-04 for anything ever again 😝 There was a lot of great work put into that draft, as demonstrated by its longevity. However, many of the choices made in an attempt to clarify turned out to do the opposite, so in some ways it is uniquely confusing. This is all much more clear in 2020-12, so from here on out I will use those keyword names and terminology exclusively. For one thing, fragments are no longer allows in To elaborate on what @gregsdennis said: Keywords fall into one of five classifications. When you load a schema document, you need to scan for Often, you can get away without doing this. You can So, technically you do not need to use And yes, I agree that we don't want to forbid |
Thanks again for the background.
That would be preferable, however one of the massive codebases I'm consuming was primarily authored back in draft-04, with a few draft-07 sprinkled in. I do not have a choice here.
Ok. So it sounds like there should be no spec changes, but the documentation site should have a line something along the lines of:
If we can agree on that, I can make an Issue/PR to the Understanding Schema repo and close this one out. This does end up making things more difficult for a massive library of individual JSON schemas with complex interdependencies like the ones I work with, but we'll have to accept that the approach taken is not considered a primary use case by the maintainers. |
Just as with programming languages or XML, there is a learning curve with JSON Schema, and we all made some sub-optimal choices as we learned.
I would like to understand if you think this is true in an absolute sense, or only because of the way the existing JSON Schemas are designed. I have managed very large schema libraries worked on by globally distributed teams, and in my experience proper use of reserved locations like |
Yes, although making the recommended usage more obvious through examples on the website will help with adoption. Right now, according to the Understanding JSON Schema site, only "complex schemas" use definitions for reuse, when in the real world I would suggest based on my own experience that most schemas are likely intended for re-use elsewhere in the system eventually. To fit with the above recommended method for reuse, we'd have to refactor thousands of schema files (most of which are re-used or use another schema) to use definitions: Before: {
"$schema": "http://json-schema.org/draft-04/schema#",
"category": "atom",
"title": "Link",
"entity": "link",
"type": "object",
"format": "grid",
"properties": {
"href": {
"title": "URL",
"type": "string",
"format": "url",
"options": {
"grid_columns": 4
}
},
"title": {
"title": "Title attribute",
"description": "Shown on mouseover.",
"type": "string",
"options": {
"grid_columns": 4
}
}
}
} After: {
"$schema": "https://json-schema.org/draft/2020-12/schema",
"category": "atom",
"title": "Link",
"entity": "link",
"type": "object",
"format": "grid",
"$defs": {
"href": {
"title": "URL",
"type": "string",
"format": "url",
"options": {
"grid_columns": 4
}
},
"title": {
"title": "Title attribute",
"description": "Shown on mouseover.",
"type": "string",
"options": {
"grid_columns": 4
}
}
},
"properties": {
"href": { "$ref": "#/$defs/href" },
"title": { "$ref": "#/$defs/title" }
}
} A second example just for completions sake: {
"$schema": "https://json-schema.org/draft/2020-12/schema",
"title": "Image embed",
"description": "A method for displaying images",
"category": "component",
"type": "object",
"format": "grid",
"$defs": {
"link": {
"entity": "link",
"type": "object",
"format": "grid",
"options": {
"grid_columns": 6
},
"properties": {
"href": {
"$ref": "link.json#/href",
"options": {
"grid_columns": 3
}
},
"title": {
"$ref": "link.json#/title",
"options": {
"grid_columns": 3
}
}
},
"required": ["href"]
}
},
"properties": {
"link": { "$ref": "#/$defs/link" }
}
} |
I think this is really a developer decision of what schemas are worth being factored out for reuse. Typically, the Rule of Three is followed. The idea is that you extract functionality when it's been proven to be reused. Premature factorization results in wasted effort. If you want to extract all of your subschemas into definitions, then you're welcome to. We won't stop you. But that's your philosophy of how you want to organize your schemas. We have to remain open to different development patterns. I personally wouldn't suggest extracting all of your subschemas into definitions. I'd follow the Ro3 mentioned above. In fact, I have an extension for my library which uses a "rule of two" to refactor common subschemas ( |
@cybtachyon yeah what @gregsdennis said. I'll also add, regarding:
Please keep in mind that we inherited Understanding JSON Schema, which was a completely independent project. The authors generously donated it to us when they were no longer in a position to keep it up to date, but we have not had time to do much more than some minor tweaks to add draft-07 notes. It was originally written for draft-04, which was very different (and also not written by the current spec team). So the book is not the official view of the current JSON Schema spec team, and every once in a while we find something we really would have done differently. We are finally in a position where someone is dedicating substantial time to updating and reworking it, which is exciting! But that's a very recent development. On the plus side, now is the perfect time to get this sort of feedback in! |
BTW, there are existing tests of $refs to locations not under $defs, here: https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/master/tests/draft2020-12/ref.json#L38 ..and there are tests of $refs to locations under keywords that do not contain subschemas, here: https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/master/tests/draft2020-12/optional/refOfUnknownKeyword.json#L4-L9 -- if the spec is explicit that this usecase should be allowed, then we should move this test out of optional/. |
@karenetheridge testing Regarding unknown keywords, it's complicated. Given the following schema (with totally unknown keywords, not a vocabulary): (YAML because I'm lazy) $id: https://example.com/foo
unknown1:
$id: bar
$anchor: bbb
unknown2:
type: string You can You cannot You can try to Although if your implementation scans and pre-calculates which possible JSON Pointers point to schemas and only allow those to be dereferenced, then none of these "unknown" cases would work. Optional seems like a good place for it. Probably a compromise solution to a past discussion, I'm guessing. |
I didn't mean to suggest that we should be allowing $refs to identifiers defined under unknown keywords, nor are we testing for that now, either in the main test section or in optional/ (in fact I added tests that check that we DON'T allow this). We should definitely NOT be scanning for identifiers under unknown keywords, because we don't know where the subschemas are, or even if there are any. The only $refs that would work to that area of the document would be using json pointer fragments using identifiers that are already known outside that section. |
@karenetheridge oops, sorry to misinterpret! I think I don't really understand what that test is doing, then, but I am quite happy to take your word for it that it's doing the appropriate thing 🙂 My understanding of how the test suite works is a little vague. |
Both the tests I linked to above use fragment-only json pointer uri references (that's a mouthful). ..and I take it back, there aren't tests for $refs to locations below unknown keywords.. but there are similar tests that check that identifiers aren't extracted from non-keyword locations (https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/master/tests/draft2020-12/id.json#L208-L257 |
@karenetheridge thank you! |
@cybtachyon is there anything more here that needs to be answered or clarified, or can this be closed? |
@handrews We need to create an issue for the docs, and I need to finish an issue for https://github.com/json-editor/json-editor , and then this can be closed.
|
@cybtachyon Feel free to create an issue for attention of @jdesrosiers. He is doing major work on it, so I’m not sure if a PR from elsewhere would be counter productive or not. |
@cybtachyon I'm happy to keep this open until you file the Understanding JSON Schema issue as long as that is done soon, but it should not be held open for a PR there (use the issue there to track the PR) or anything else. We don't keep issues open here waiting for other people (JSON Editor) to update their projects. This repository only tracks work we can do on this repository. |
This can now be closed. Thanks all for clarifying and contributing! |
Uh oh!
There was an error while loading. Please reload this page.
From @ThatGuyCND @rjmill and myself:
Examples:
image_embed.json
:link.json
:Why is this important enough to merit changes to the docs?
"definitions"
/"$defs"
are not required by the spec, therefore most developers are unlikely to use them. Just take a look at any of the large libraries that use JSON Schema to define their schema - for example https://explore.fast.design/components/fast-accordionGoal
The text was updated successfully, but these errors were encountered: