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

New option suggestion: direct assignment of ajv validate function #94

Closed
wants to merge 1 commit into from

Conversation

hokiedsp
Copy link

While the current support for JSON scheme validation is great, it limits more advanced usage of ajv. To fully unleash ajv capability, I'd love for conf to support an option to let user provide the ready-to-go ajv validator function rather than using the internally generated one via schema option. This PR implements such option, which I tentatively named ajvValidator.

Additional Notes on PR:

  • schema option take precedence over ajvValidator option
  • ajvValidator option is checked to be sure it's a function
  • As far as I can tell, I don't think this PR affects the operation of _validate() unless user specifies a non-ajv validation function.
  • I've added a test for the new option in test.js based on the existing schema option test

@sindresorhus
Copy link
Owner

Could you elaborate on your use-case for needing this? It might help inform us on how we can better improve the defaults too.

@hokiedsp
Copy link
Author

hokiedsp commented Nov 13, 2019

@sindresorhus - Sure, the reason I added that option to my fork is to allow me to use full schema files, specifically, to support $ref and use of multiple schema files. See here for synchronous approach or here for async approach.

how we can better improve the defaults

I'm not sure if there is much room left to improve with the schema option as it provides only the properties property of a full JSON schema object. Expanding it in any way will disrupt its original behavior IMO.

My PR is the most generic one such that I can use ajv's async compilation with, but with your support I have an alternate ideas that I can implement for conf:

  • schemaObjects option to accept an array of full schema objects, and use the first object's URI as the base
  • schemaFiles option to accept an array of schema JSON file paths, read files, and use the $id of the first file as the base URI

These are more complete than the PR's ajvValidator but obviously are more complex and more work. As a side note, schemaValidator might be a better name for ajvValidator.

@hokiedsp
Copy link
Author

@sindresorhus - I just realized that you have an ongoing discussion and reached more-or-so the exact same conclusion: #80 (comment)

As stated above, I'd push for schemaObjects/rootSchema to accept an array of JSON schema objects to support schema spanning multiple schema objects.

@sindresorhus
Copy link
Owner

sindresorhus commented Feb 20, 2020

To be honest, I'm not that well versed in JSON schema. I just added it because there was strong demand. It's not clear to me why you need support for multiple schema objects? It kinda seems to me that to solve all use-cases, there should be a schemaMeta option that accepts both ajv options add schemas. I don't really want to introduce more than one top-level option for this, so we need to figure out a good way to design it.

@sindresorhus
Copy link
Owner

Closing for lack of response.

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