-
-
Notifications
You must be signed in to change notification settings - Fork 11
Add draft-06 contains support #151
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
base: main
Are you sure you want to change the base?
Conversation
jdesrosiers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is the best we can do, then it doesn't make sense to use a shared handler. None of the meaningful code is shared, only the boilerplate.
However, I think it can be done in a way that makes sense for it to be shared. In draft-06, there is no minContains/maxContains keywords, so we can be confident that they will never be there. That means it doesn't hurt to check for them. If we just let them be checked regardless of which version of the keyword it is, I'm pretty sure things will just work. That was the entire handler can be the same and the only change needed is the loop to check for both keywor URIs.
|
Thanks for the guidance!
|
jdesrosiers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't reuse the normalization handler. They produce different values. The draft-06 version only has a string (the schema location). The current version is an object containing the schema location and the min/max constraints.
I noticed that you never enabled the draft-06 and draft-07 tests in the test runner. When you do that, you should see test failures from this bug.
|
Thanks understood. |
|
Hi @jdesrosiers when I enable the draft-06/07 tests I get this failure:
So it seems my handler is returning per-item outputs instead of the boolean summary that the current Could you confirm what the correct normalized output should be for draft-06 Should the draft-06 normalization handler:
Once I know the expected structure, I can fix it properly. Thanks |
Hi @jdesrosiers
I made the required updates for draft-06/07
containssupport:Registered only the draft-06 keyword URI:
https://json-schema.org/keyword/draft-06/contains
(since draft-06 and draft-07 share the same keyword URI)
Added a draft-specific normalization handler at:
src/normalization-handlers/draft-06/contains.js
which implements the simpler draft-06 semantics (at least one matching item, no minContains/maxContains).
Updated the shared error handler (src/error-handlers/contains.js) to treat
draft-06/containsas a special case:{ minContains: 1 }All tests are passing locally, and lint/type-check are clean.
Please let me know if anything else should be adjusted.