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

Feature/ajv-async - Supporting Asynchronous Validations #53

Open
wants to merge 3 commits into
base: feature/dynamic-properties
Choose a base branch
from

Conversation

dangrima90
Copy link

Hi! Following the discussion we had on #51. I did some research for other libraries however I couldn't find a good solution to what I was looking for.

Main reason why I wanted to stick with this library is its UI schema as it reflects the properties of Vue components. Anyway I did this code change about a week and a half ago been testing it a bit and so far so good, and now finally I found the time to create a pull request. I took a branch from the dynamic-properties branch so that I could test that as well.

Changes made basically all resolve around the ajv.validate() function. Any dependencies of this function were changed to be asynchronous so that there is support for asynchronous keywords and formats.

@dangrima90 dangrima90 changed the title Feature/ajv async - Supporting Asynchronous Validations Feature/ajv-async - Supporting Asynchronous Validations May 13, 2020
@dangrima90 dangrima90 changed the base branch from master to feature/dynamic-required May 13, 2020 16:54
@dangrima90 dangrima90 changed the base branch from feature/dynamic-required to feature/dynamic-properties May 13, 2020 16:55
@jarvelov
Copy link
Owner

Wow!

Thanks for this pull request. At first glance this looks good, but I need to test it out and see if something breaks with it seeing as async would be the new default.

I'll go over the pull request this weekend and let you know if I find any issues that would prevent this from being merged.

Again, many thanks!

@dangrima90
Copy link
Author

You're welcome! One small suggestion I would try to have babel set up to enable async/await. It would make the code a bit more readable when it comes to asynchronous functions. I didn't change any configuration with regards to babel so as not to change too many things. :)

@jarvelov
Copy link
Owner

jarvelov commented May 26, 2020

Hi,

I finally got time to sit down and check out this pull request, I apologize that it's taken a while.

There are some breaking changes which I've started to look into. For example, if one omits $async: true in the schema value then the code will still try to use the return value of ajv.validate() as a promise. You can see this behavior by omitting the $async property.

I've begun work on fixing that by replacing calls to ajv.validate with a helper method to wrap the validate call in and return the value in a promise. Hopefully that should take care of the problems. I will have to test things out to ensure everything works as intended, but that will have to be tomorrow.

Also, regarding async/await, I will introduce this in version 3.0, but as of version 2.0 I aim to support IE 11. In order to retain that support it would require to include the regenerator runtime polyfill, however that would break things for people using the ESM build. Also, if I bundle the polyfills it would severely increase the bundle size for the UMD build. So unfortunately that will have to wait until next version when I drop IE 11 support.

@dangrima90
Copy link
Author

Hello, thanks for the update. Yes sure I understand your points! I look forward to eventually seeing this as part of the library. Hope the pull request at least helps you out for future releases.

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