-
-
Notifications
You must be signed in to change notification settings - Fork 208
feat: add support for const value #276
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
322fd60
to
a5189a6
Compare
index.js
Outdated
return (/"if":{/.test(str) && /"then":{/.test(str)) || | ||
/"(anyOf|oneOf)":\[/.test(str) || | ||
/"const":/.test(str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zekth here's the same what hasOf
, hasIf
and hasConst
does, do you see any tricky cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't the first condition be factorized by :
/"if":{.*"then":{/.test(str)
WDYT?
Also is there any impact on perfs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried run benchmark, seems there no impact almost at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I joined all regexp into one
a5189a6
to
70faaeb
Compare
Windows 10 20H2 x64, AMD Ryzen 3500, 32gb RAM, NodeJS 14.15.2 master:
this branch:
P.S. I wouldn't really trust this benchmark results, there're lot of things which may accidentically affect them |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
regarding benchmarks, i just wanted to see if there is an impact from looping to regex.
Adds support for
const
keywordFixes #275
References: http://json-schema.org/understanding-json-schema/reference/generic.html#id5
Checklist
npm run test
andnpm run benchmark
and the Code of conduct