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

openApi formats #9

Closed
seriousme opened this issue Dec 31, 2020 · 8 comments
Closed

openApi formats #9

seriousme opened this issue Dec 31, 2020 · 8 comments

Comments

@seriousme
Copy link
Contributor

Hi,

Thanks for all the work on AJV !

I'm the author of fastify-openapi-glue and I have created formats for

  • int32
  • int64
  • float
  • double
  • binary

as defined at https://swagger.io/docs/specification/data-models/data-types/

I presume that more AJV users will be using AJV for the parsing of openapi specs, would you be interested in a PR to add these formats to ajv-formats ?

Kind regards,
Hans

@epoberezkin
Copy link
Member

epoberezkin commented Mar 27, 2021

Yes, particularly that ajv added discriminator support in v8, it makes sense to add these formats.

I'm about to release v2 of this package today, a shame I didn't spot it earlier, but it can be added to v2.1 if you can't add it now-ish :)

btw, binary and password that are no-op can be just defined as true (as opposed to () => true), the difference is that it would generate no validation code at all in case it is true. Also - you don't need to explicitly pass data type "string" to format definition, it is the default, so string format can be either function, regex or true.

@epoberezkin
Copy link
Member

I wasn't even watching this repo - that's why I missed...

@seriousme
Copy link
Contributor Author

I can probably do add all formats in the next few hours.
Do you want to wait for #8 or should I do a PR myself ?

And a second question:
My current code relies on decimal.js-light but I noticed that BigInt is natively supported as of Node 10.4.0 so I'd prefer to use BigInt instead. Do you agree ?

Kind regards,
Hans
ps. I forgot about raising this issue too ;-)

@epoberezkin
Copy link
Member

yes - please do it - you have it more-or-less done in the link :)

@seriousme
Copy link
Contributor Author

Working on it, code is in place, now working on the tests ;-)

@epoberezkin
Copy link
Member

My current code relies on decimal.js-light but I noticed that BigInt is natively supported as of Node 10.4.0 so I'd prefer to use BigInt instead

I think it's already updated to use BigInt? Either way, even node 10 will end by May, so it's ok to use BigInt I think (it doesn't require that the number itself is BigInt, does it?)

@seriousme
Copy link
Contributor Author

Just updated it to BigInt after my comment, also as an excercise for myself ;-)
BigInt also works with small numbers :-)

@seriousme
Copy link
Contributor Author

Thx !

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

No branches or pull requests

2 participants