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

[ajv.d.ts] Add missing argument for validateSchema function #1216

Closed
wants to merge 1 commit into from

Conversation

brussee
Copy link

@brussee brussee commented May 13, 2020

Fix types according to implementation and documentation: https://github.com/ajv-validator/ajv/blob/master/lib/ajv.js#L164

@brussee
Copy link
Author

brussee commented May 13, 2020

For people using patch-package you may find the following gist useful until this PR is merged: https://gist.github.com/brussee/0011732e20dbcfd4c4e9d33748fa23f4

@epoberezkin
Copy link
Member

epoberezkin commented May 26, 2020

This wasn't documented in readme, let me please think if I want to legitimise this parameter - it was quite ad-hoc. Doc in code was needed mainly to appease linter about "this" (I think).

Why do you need this parameter becoming public api? Its name is quite confusing tbh...

@brussee
Copy link
Author

brussee commented Jun 19, 2020

@epoberezkin Thanks for your reply and sorry for the delay.

My use case is that I want to know the reason if schema is invalid. The only way to get that information back to the caller is to have the validateSchema function throw an error with the message (is there another way?), which it only does right now if you set the throwOrLogError parameter to true.

I agree the name is confusing and I propose you choose between what I consider the C way vs. the C++ way of returning (status/exception) information from a function, e.g.:
func foo(in, *err) { err = 'foo'; return -1; }
func foo(in) { throw Error('foo'); }
If you want to support both, like you do now, maybe you can make a global setting instead of a parameter per function, because consistency is preferred in my opinion.

Thanks for your consideration!

@brussee
Copy link
Author

brussee commented Jun 19, 2020

I just noticed the errors and errorsText are public API already, so I can use that instead.

@brussee brussee closed this Jun 19, 2020
@epoberezkin
Copy link
Member

You can indeed get the errors from the instance - thank you.

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

Successfully merging this pull request may close these issues.

2 participants