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 z-schema validator plugin #1157

Merged
merged 6 commits into from
Jul 4, 2019

Conversation

phil-lgr
Copy link
Contributor

This PR contains:

  • A NEW FEATURE

Describe the problem you have without this PR

Both is-my-json-valid and ajv-validate use eval() to perform validation which might not be wanted when 'unsafe-eval' is not allowed in Content Security Policies. This one is using z-schema as validator which doesn't use eval.

Todos

  • Tests
  • Documentation
  • Typings
  • Changelog

Related to #243, actually closes #235

@phil-lgr
Copy link
Contributor Author

@pubkey I took the same tests as the ajv-validator plugin, does that makes sense to you?

* @
*/
function _getValidator() {
const schemaPath = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : '';
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason why you do not use javascript-defaults like here?

@pubkey
Copy link
Owner

pubkey commented May 29, 2019

@phil-lgr thank you for that great PR.
I added one comment. I will review the rest tomorrow, looks good. 👍

@phil-lgr
Copy link
Contributor Author

there is a couple of issues:

In my app I get:

"RxError: RxError: Sub-schema not found, does the schemaPath exists in your schema? Given parameters: { schemaPath:""} 

@phil-lgr phil-lgr force-pushed the add-z-schema-validator-plugin branch from de9af96 to d3a9471 Compare May 29, 2019 02:15
const validatorsOfHash = validatorsCache[hash];

if (!validatorsOfHash[schemaPath]) {
const schemaPart = schemaPath === '' ? rxSchema.jsonID : rxSchema.getSchemaByObjectPath(schemaPath);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand where does the getSchemaByObjectPath comes from? @pubkey mmmm

@phil-lgr
Copy link
Contributor Author

Ok so I'm working on this again now

@phil-lgr phil-lgr force-pushed the add-z-schema-validator-plugin branch from d3a9471 to 3479066 Compare May 29, 2019 16:23

const runAfterSchemaCreated = rxSchema => {
// pre-generate the validator-z-schema from the schema
requestIdleCallbackIfAvailable(() => _getValidator.bind(rxSchema, rxSchema));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why but I had to .bind here... otherwise the test would work, but in the browser it wouldn't.. if you have a better way to handle this let me know

@@ -94,7 +94,8 @@
"spark-md5": "^3.0.0",
"unload": "2.1.0",
"url": "^0.11.0",
"util": "^0.12.0"
"util": "^0.12.0",
"z-schema": "^4.0.2"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that 'ajv' is not listed here, are users supposed to install themselves? if so I guess I should remove z-schema from here

Copy link
Owner

Choose a reason for hiding this comment

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

I think ajv is missing. Z-schema schould stay there. Im offline since monday btw, i can help then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what version of ajv should I add?

@pubkey
Copy link
Owner

pubkey commented Jul 4, 2019

What is the state of this?

@phil-lgr
Copy link
Contributor Author

phil-lgr commented Jul 4, 2019

I've been using my fork for sometime, it works fine!

Not sure what we need to add to the PR, if you can review the code style comments above that would be great

@pubkey pubkey merged commit 7bc7d9c into pubkey:master Jul 4, 2019
@pubkey
Copy link
Owner

pubkey commented Jul 4, 2019

@phil-lgr thanks. I merged this for now.
I added an entry to the todo-list for the next major version. See here
I think we should rewrite the jsonschema-validation which I will do then.

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.

Content-Security-Policy Error on validate schema
2 participants