feat: add createAjvValidatorAsync #522
Conversation
|
Thanks Stefan, I'll review your PR as soon as I can. |
There was a problem hiding this comment.
Thanks Stefan, your PR looks good! I like the way you've refactored both the code and made sure the tests are not duplicated. It makes sense to create a separate function for the async variant since it returnse a Promise, that would be a breaking change and not something you want to deal with in the synchronous case.
I made a few inline comments, can you have a look at those?
| describe('createAjvValidator', () => { | ||
| test('should create a validate function', () => { | ||
| const validate = createAjvValidator({ schema, schemaDefinitions }) | ||
| function sharedAssertions(factoryFn: typeof createAjvValidator | typeof createAjvValidatorAsync) { |
There was a problem hiding this comment.
Can you replace the solution with a sharedAssertions function with using describe.each of vitest? Docs: https://vitest.dev/api/#describe-each
Something like:
describe.each([
{ name: 'createAjvValidator', create: createAjvValidator },
{ name: 'createAjvValidatorAsync', create: createAjvValidatorAsync }
])('$name', ({ create }) => {
// ... the tests ...
})| * } | ||
| * }) | ||
| */ | ||
| export async function createAjvValidatorAsync(options: AjvValidatorOptions): Promise<Validator> { |
There was a problem hiding this comment.
Can you describe the new function createAjvValidatorAsync in the README.md, in the sections "validator" and "Utility functions"?
| * | ||
| * @default ValidationSeverity.warning | ||
| */ | ||
| errorSeverity?: ValidationSeverity |
This PR adds a new function,
createAjvValidatorAsyncto the Ajv plugin.It behaves like
createAjvValidator, but usesajv.compileAsyncon the schema which allows for resolution of remote schemas through theloadSchema(uri)hook of Ajv'soptions.Due to the async nature of
compileAsync, the return format changes, which would be a breaking change. So I opted for a second factory function in order to not break existing consumers.The PR also adds
errorSeverityas an option.As you can see, I have refactored the code a bit s.t. as little code as possible is duplicated. It had a bit of an unfortunate impact on the testing code, where I also tried to not duplicate all the existing tests just be cause there now is a second factory function.
Thanks for your work & please let me know if you want anything changed :)