-
-
Notifications
You must be signed in to change notification settings - Fork 209
refactor: create separate schema for Ajv validation #504
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
Conversation
Node: 14
MAIN:
Node: 16
MAIN:
Node: 18
MAIN:
|
It makes sense to me. |
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.
lgtm
787dd9a
to
b2d55a0
Compare
b2d55a0
to
e6fcbe9
Compare
There are two updates.
|
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.
The code itself look good to me and the changes on debug
output is breaking.
@ivan-tymoshenko
You are the one focusing on this package. Can I ask how many breaking changes are you expected in the near future? I am considering should we group a bunch of them before another major.
@@ -11,8 +11,10 @@ class RefResolver { | |||
if (schema.$id !== undefined && schema.$id.charAt(0) !== '#') { | |||
schemaId = schema.$id | |||
} | |||
this.insertSchemaBySchemaId(schema, schemaId) | |||
this.insertSchemaSubschemas(schema, schemaId) | |||
if (this.getSchema(schemaId) === undefined) { |
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.
Is this changes itself self contained?
Do we need a test case for it?
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.
I didn't add this check. I just moved it inside the class.
Line 113 in b0bb1a6
if (refResolver.getSchema(schemaKey) === undefined) { |
@climba03003 I can leave the Ajv instance and remove it in the next major release. |
How it's working now:
It takes a user-defined schema, adds necessary adjustments for Ajv (now it's only for date type), and then works with modified schema.
How it will work:
It will clone a user-defined schema, modify it and pass it to the Ajv. Serialization will work with the original schema. The only requirement we must follow is that the schema must not change its structure during the modification. In other words, the JSON pointers must match in two schemas, but the schemas themselves (type, format, keywords, ... etc.) may differ.
Motivation:
It's too complicated to maintain one syntax for serialization and validation rules. Our code depends on Ajv validation syntax.
Depends on #507