-
-
Notifications
You must be signed in to change notification settings - Fork 877
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
only emit equal
when needed: addresses #1852
#1853
Conversation
I had to add a |
(Once more with feeling) An explicit check for Array.isArray, and now |
@@ -20,7 +21,11 @@ const def: CodeKeywordDefinition = { | |||
const {gen, data, $data, schema, schemaCode, it} = cxt | |||
if (!$data && schema.length === 0) throw new Error("enum must have non-empty array") | |||
const useLoop = schema.length >= it.opts.loopEnum | |||
const eql = useFunc(gen, equal) | |||
const needsEql = |
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've made a small correction, but in general this is a brittle approach - if the condition of usage changes, these conditions would have to be updated too, and there may be edge cases that would not be caught by the tests.
A much better approach would be to simply call useFunc in the place where eql is actually used.
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.
so probably just make a memoized getEql function and replace eql with getEql() in two places.
closing in favour of #1922 |
* fix: emit equal when needed - alternative to #1853 * fix: prettier errors * fix: tslint errors * Update lib/vocabularies/validation/enum.ts * Update lib/vocabularies/validation/enum.ts Co-authored-by: Evgeny Poberezkin <2769109+epoberezkin@users.noreply.github.com>
What issue does this pull request resolve?
This addresses issue #1852 .
What changes did you make?
I added a check to the enum types before emitting the requirement for
equal
.Is there anything that requires more attention while reviewing?
Not that I'm aware of.