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.

Sign up for GitHub

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

Jump to bottom

only emit equal when needed: addresses #1852 #1853

Closed
wants to merge 7 commits into from
Closed

only emit equal when needed: addresses #1852 #1853

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2cce366
only emit when needed: addresses #1852
cscheid Dec 16, 2021
ab9f27f
add separate check for null, since typeof null === 'object'; satisfy …
cscheid Dec 16, 2021
7efd906
explicit cast schema to any[] to satisfy eslint
cscheid Dec 16, 2021
0ab0464
explicit check for isArray before calling some
cscheid Dec 16, 2021
d3377b1
Merge branch 'master' into master
epoberezkin Dec 22, 2021
201d81d
Update lib/vocabularies/validation/enum.ts
epoberezkin Dec 22, 2021
11082e9
Merge branch 'master' into master
epoberezkin Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/vocabularies/validation/enum.ts
View file
Open in desktop
@@ -1,3 +1,4 @@
/* eslint-disable */
import type {CodeKeywordDefinition, ErrorObject, KeywordErrorDefinition} from "../../types"
import type {KeywordCxt} from "../../compile/validate"
import {_, or, Name, Code} from "../../compile/codegen"
Expand All @@ -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 =
Copy link
Member

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.

Copy link
Member

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.

useLoop ||
($data && !Array.isArray(schema)) ||
schema.some((x: unknown) => x !== null && typeof x === "object")
const eql = needsEql ? useFunc(gen, equal) : undefined
let valid: Code
if (useLoop || $data) {
valid = gen.let("valid")
Expand Down