Skip to content
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

Add @exodus/schemasafe #46

Merged
merged 1 commit into from
Jul 5, 2020
Merged

Add @exodus/schemasafe #46

merged 1 commit into from
Jul 5, 2020

Conversation

ChALkeR
Copy link
Contributor

@ChALkeR ChALkeR commented Jun 17, 2020

Things to note:

  1. allowUnusedKeywords is required because that's what specification is expecting, and there are tests for that. By default, @exodus/schemasafe instead refuses to compile a schema with unused keywords to prevent failing in open state on e.g. schema author mistakes -- similar to how ajv behaves with strictKeywords option, but with deeper checks.

    That option does not affect the produced code or performance, it just affects whether the schema would be refused to compile or not.

  2. Structured reporting is disabled by default, instead just a boolean validation status is reported — because if the user wants errors, they would enable them, and if they don't need errors and need just the result — they might forget to disable it for performance.

    I'm not sure how fair is that to other validators in this benchmark. If it's not -- just add includeErrors: true, to the options ;-)

  3. Default $schema value is needed because $schema is not specified in all tests, and behavior differs between various spec versions. Specifically, $ref behavior changed in draft2019-09, and there is a test for that. Out of those two, draft2019-09 behavior is more secure and makes more sense, hence that's the default in schemasafe unless $schema is specified in the schema or $schemaDefault is specified in options.

@ebdrup
Copy link
Owner

ebdrup commented Jul 5, 2020

  1. In general I've tried to run all validators with their default settings, so as to avoid the benchmark being run with weird perfomance tweaks that do not make sense in the real world. This setting seems to be ok to use as per your explanation.
  2. Following the principle of running the validator with default settings, this seems fine.
  3. That's ok as far as I can see.

@ebdrup ebdrup merged commit 2410c2b into ebdrup:master Jul 5, 2020
@@ -4,6 +4,7 @@
"license": "MIT",
"dependencies": {
"@cfworker/json-schema": "*",
"@exodus/schemasafe": "^1.0.0-alpha.4",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I general I use * for the latest stable version. This should be changed once you reach a stable v1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants