From bd4b9d1f07aeab412e1fd514ba63afa96fc66c77 Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Mon, 7 Sep 2026 21:04:17 +0000 Subject: [PATCH 1/7] fix: quadratic error filtering and dropped sibling errors `filterErrors` re-scanned every already collected error for each new error and re-copied the accumulated children on every step, so the cost grew quadratically with the amount of errors reported by ajv. Since `allErrors` is enabled, an invalid configuration produces one error per offending value, which makes it cheap to turn a large invalid options object into a long busy loop: validating an object with 40000 unknown properties took ~8.5s, and 40000 bad array items took ~90s. Errors are now indexed by their instance path in a prefix tree, and the children of an absorbed error are adopted instead of copied, which makes the same inputs take ~70ms and ~460ms. The lookup used `oldError.instancePath.includes(instancePath)`, which also matched instance paths that merely contain the path as a substring, so an error was nested under - and for most keywords silently swallowed by - an unrelated error. Two sibling properties where one name is a prefix of the other were enough to lose an error: validate( { type: "object", properties: { foobar: { type: "string" }, foo: { type: "string" } } }, { foobar: 1, foo: 1 }, ); only reported `configuration.foo`. The prefix tree matches json pointer segments, so nesting now happens only for errors that really are reported for the instance path or for something inside it. `findAllChildren` sliced the children array on every recursion, which was quadratic as well; it now takes the end of the range to look at instead. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013wqLmVHkAGBsWQgXknEQCK --- declarations/validate.d.ts | 18 ++++ src/ValidationError.js | 23 +++--- src/validate.js | 153 ++++++++++++++++++++++++++++++---- test/filter-errors.test.js | 163 +++++++++++++++++++++++++++++++++++++ 4 files changed, 333 insertions(+), 24 deletions(-) create mode 100644 test/filter-errors.test.js diff --git a/declarations/validate.d.ts b/declarations/validate.d.ts index 081f2fe..42738c1 100644 --- a/declarations/validate.d.ts +++ b/declarations/validate.d.ts @@ -52,6 +52,24 @@ export type ValidationErrorConfiguration = { */ postFormatter?: PostFormatter | undefined; }; +/** + * A node of the prefix tree used by `filterErrors` to look up already reported errors by their + * instance path. + */ +export type ErrorPathNode = { + /** + * positions (in the result array) of the errors reported for exactly this instance path + */ + indexes: number[]; + /** + * nodes of nested instance paths, keyed by json pointer segment + */ + children: Map; + /** + * amount of errors stored in this node and in all its descendants + */ + size: number; +}; /** * @returns {void} */ diff --git a/src/ValidationError.js b/src/ValidationError.js index fb1796d..f6b10ea 100644 --- a/src/ValidationError.js +++ b/src/ValidationError.js @@ -113,10 +113,11 @@ function extractRefs(error) { * Find all children errors * @param {SchemaUtilErrorObject[]} children children * @param {string[]} schemaPaths schema paths + * @param {number=} end amount of children to look at, i.e. only `children[0..end - 1]` are visited * @returns {number} returns index of first child */ -function findAllChildren(children, schemaPaths) { - let i = children.length - 1; +function findAllChildren(children, schemaPaths, end = children.length) { + let i = end - 1; const predicate = /** * @param {string} schemaPath schema path @@ -127,10 +128,11 @@ function findAllChildren(children, schemaPaths) { while (i > -1 && !schemaPaths.every(predicate)) { if (children[i].keyword === "anyOf" || children[i].keyword === "oneOf") { const refs = extractRefs(children[i]); - const childrenStart = findAllChildren(children.slice(0, i), [ - ...refs, - children[i].schemaPath, - ]); + const childrenStart = findAllChildren( + children, + [...refs, children[i].schemaPath], + i, + ); i = childrenStart - 1; } else { @@ -155,10 +157,11 @@ function groupChildrenByFirstChild(children) { if (child.keyword === "anyOf" || child.keyword === "oneOf") { const refs = extractRefs(child); - const childrenStart = findAllChildren(children.slice(0, i), [ - ...refs, - child.schemaPath, - ]); + const childrenStart = findAllChildren( + children, + [...refs, child.schemaPath], + i, + ); if (childrenStart !== i) { result.push({ ...child, children: children.slice(childrenStart, i) }); diff --git a/src/validate.js b/src/validate.js index 89c4473..50adcfb 100644 --- a/src/validate.js +++ b/src/validate.js @@ -148,41 +148,166 @@ function needValidate() { } /** + * A node of the prefix tree used by `filterErrors` to look up already reported errors by their + * instance path. + * @typedef {object} ErrorPathNode + * @property {number[]} indexes positions (in the result array) of the errors reported for exactly this instance path + * @property {Map} children nodes of nested instance paths, keyed by json pointer segment + * @property {number} size amount of errors stored in this node and in all its descendants + */ + +/** + * @returns {ErrorPathNode} empty node + */ +function createErrorPathNode() { + return { indexes: [], children: new Map(), size: 0 }; +} + +/** + * Splits an instance path (a json pointer) into its segments, i.e. `"/rules/0"` into `["rules", "0"]`. + * @param {string} instancePath instance path + * @returns {string[]} json pointer segments + */ +function parseInstancePath(instancePath) { + // A json pointer is either empty or starts with a separator, so the leading separator is dropped + // instead of splitting off an empty first segment + return instancePath === "" ? [] : instancePath.slice(1).split("/"); +} + +/** + * @param {ErrorPathNode} root root node + * @param {string[]} segments json pointer segments of the error instance path + * @param {number} index position of the error in the result array + * @returns {void} + */ +function addErrorPath(root, segments, index) { + let node = root; + + node.size += 1; + + for (const segment of segments) { + let child = node.children.get(segment); + + if (!child) { + child = createErrorPathNode(); + node.children.set(segment, child); + } + + node = child; + node.size += 1; + } + + node.indexes.push(index); +} + +/** + * Removes and returns every error reported for the given instance path or for anything nested + * inside it, in the order the errors were reported. + * @param {ErrorPathNode} root root node + * @param {string[]} segments json pointer segments of the error instance path + * @returns {number[]} positions (in the result array) of the removed errors + */ +function takeErrorPaths(root, segments) { + /** @type {ErrorPathNode[]} */ + const ancestors = [root]; + let node = root; + + for (const segment of segments) { + const child = node.children.get(segment); + + // Nothing was reported below this instance path + if (!child) { + return []; + } + + node = child; + ancestors.push(node); + } + + /** @type {number[]} */ + const indexes = []; + /** @type {ErrorPathNode[]} */ + const stack = [node]; + + while (stack.length > 0) { + const current = /** @type {ErrorPathNode} */ (stack.pop()); + + for (const index of current.indexes) { + indexes.push(index); + } + + for (const child of current.children.values()) { + stack.push(child); + } + } + + // The whole subtree has been consumed, so detach it and drop the ancestors it left empty, + // otherwise later lookups would keep walking over nodes without errors + const removed = indexes.length; + + node.indexes = []; + node.children.clear(); + node.size = 0; + + for (let i = ancestors.length - 2; i >= 0; i--) { + ancestors[i].size -= removed; + + if (ancestors[i + 1].size === 0) { + ancestors[i].children.delete(segments[i]); + } + } + + return indexes.sort((a, b) => a - b); +} + +/** + * Nests every error under the last reported error that covers its instance path, so that only the + * outermost errors are left at the top level. * @param {ErrorObject[]} errors array of error objects * @returns {SchemaUtilErrorObject[]} filtered array of objects */ function filterErrors(errors) { - /** @type {SchemaUtilErrorObject[]} */ - let newErrors = []; + /** @type {(SchemaUtilErrorObject | undefined)[]} */ + const newErrors = []; + const root = createErrorPathNode(); for (const error of /** @type {SchemaUtilErrorObject[]} */ (errors)) { - const { instancePath } = error; + const segments = parseInstancePath(error.instancePath); /** @type {SchemaUtilErrorObject[]} */ let children = []; - newErrors = newErrors.filter((oldError) => { - if (oldError.instancePath.includes(instancePath)) { - if (oldError.children) { - children = [...children, ...oldError.children]; - } + for (const index of takeErrorPaths(root, segments)) { + const oldError = /** @type {SchemaUtilErrorObject} */ (newErrors[index]); - oldError.children = undefined; - children.push(oldError); + newErrors[index] = undefined; - return false; + if (oldError.children) { + if (children.length === 0) { + // Adopt the array instead of copying it - a long run of sibling errors re-parents the + // previously collected children on every step, so copying them would be quadratic + children = oldError.children; + } else { + for (const child of oldError.children) { + children.push(child); + } + } } - return true; - }); + oldError.children = undefined; + children.push(oldError); + } if (children.length) { error.children = children; } + addErrorPath(root, segments, newErrors.length); newErrors.push(error); } - return newErrors; + return /** @type {SchemaUtilErrorObject[]} */ ( + newErrors.filter((error) => typeof error !== "undefined") + ); } /** diff --git a/test/filter-errors.test.js b/test/filter-errors.test.js new file mode 100644 index 0000000..c160ce3 --- /dev/null +++ b/test/filter-errors.test.js @@ -0,0 +1,163 @@ +import { validate } from "../src"; + +/** + * @param {object} schema schema + * @param {object} options options + * @returns {import("../src/validate").SchemaUtilErrorObject[]} reported errors + */ +function getErrors(schema, options) { + try { + validate(schema, options); + } catch (error) { + if (error.name !== "ValidationError") { + throw error; + } + + return error.errors; + } + + throw new Error("Validation didn't fail"); +} + +/** + * @param {object} schema schema + * @param {object} options options + * @returns {string} error message + */ +function getMessage(schema, options) { + try { + validate(schema, options); + } catch (error) { + if (error.name !== "ValidationError") { + throw error; + } + + return error.message; + } + + throw new Error("Validation didn't fail"); +} + +describe("filter errors", () => { + it("should not nest errors of a property inside errors of a sibling property with a shorter name", () => { + const schema = { + type: "object", + properties: { + foobar: { type: "string" }, + foo: { type: "string" }, + }, + }; + + const message = getMessage(schema, { foobar: 1, foo: 1 }); + + expect(message).toContain("configuration.foobar should be a string."); + expect(message).toContain("configuration.foo should be a string."); + }); + + it("should not nest errors of a nested property inside errors of an unrelated property", () => { + const schema = { + type: "object", + properties: { + a: { type: "object", properties: { b: { type: "string" } } }, + b: { type: "array", items: { type: "string" } }, + }, + }; + + const message = getMessage(schema, { a: { b: 1 }, b: 1 }); + + expect(message).toContain("configuration.a.b should be a string."); + expect(message).toContain("configuration.b should be an array"); + }); + + it("should nest errors of nested properties inside errors of their parent", () => { + const schema = { + type: "object", + properties: { + a: { + anyOf: [ + { type: "object", properties: { b: { type: "string" } } }, + { type: "string" }, + ], + }, + }, + }; + + const errors = getErrors(schema, { a: { b: 1 } }); + + expect(errors).toHaveLength(1); + expect(errors[0].keyword).toBe("anyOf"); + expect(errors[0].instancePath).toBe("/a"); + expect(errors[0].children.map((error) => error.instancePath)).toContain( + "/a/b", + ); + }); + + it("should nest sibling errors reported for the same instance path", () => { + const schema = { + type: "object", + additionalProperties: false, + properties: { a: { type: "string" } }, + }; + + const options = {}; + + for (let i = 0; i < 10; i++) { + options[`unknown${i}`] = 1; + } + + const errors = getErrors(schema, options); + + expect(errors).toHaveLength(1); + expect(errors[0].children).toHaveLength(9); + expect( + errors[0].children.every( + (error) => typeof error.children === "undefined", + ), + ).toBe(true); + }); + + // `filterErrors` used to be quadratic in the amount of reported errors, so a large invalid + // configuration was enough to lock up the process for minutes + it("should filter a large amount of sibling errors in a reasonable time", () => { + const schema = { + type: "object", + additionalProperties: false, + properties: { a: { type: "string" } }, + }; + + const options = {}; + + for (let i = 0; i < 40000; i++) { + options[`unknown${i}`] = 1; + } + + const start = process.hrtime.bigint(); + const errors = getErrors(schema, options); + const elapsed = Number(process.hrtime.bigint() - start) / 1e6; + + expect(errors).toHaveLength(1); + expect(errors[0].children).toHaveLength(39999); + expect(elapsed).toBeLessThan(5000); + }, 30000); + + it("should filter a large amount of errors with distinct instance paths in a reasonable time", () => { + const schema = { + type: "object", + properties: { + list: { + type: "array", + items: { anyOf: [{ type: "string" }, { type: "boolean" }] }, + }, + }, + }; + + const options = { list: Array.from({ length: 40000 }, () => 1) }; + + const start = process.hrtime.bigint(); + const errors = getErrors(schema, options); + const elapsed = Number(process.hrtime.bigint() - start) / 1e6; + + expect(errors).toHaveLength(40000); + expect(elapsed).toBeLessThan(5000); + }, 30000); +}); From 7e0a5f46574a73217f717ad6363291878a46786b Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Mon, 7 Sep 2026 21:04:18 +0000 Subject: [PATCH 2/7] perf: speed up validation and error formatting - `validate` spread the errors of each entry of an array of options into the result with `push(...errors)`, which throws `RangeError: Maximum call stack size exceeded` instead of a `ValidationError` once an entry reports enough errors. The errors are pushed one by one now, which also drops the intermediate array `map` allocated for every entry. - `needValidate` read `process.env.SKIP_VALIDATION` twice and built both of its regular expressions on every call. Reading a variable from `process.env` is expensive and this runs on every validation, so the value is read once and the regular expressions are hoisted: setting `SKIP_VALIDATION` explicitly made a successful validation ~2.4x slower than leaving it unset, now it costs about the same (582ns -> 365ns). - `filterErrors` walked the instance path prefix tree twice per error, once to collect the errors it nests and once to store the error itself. Both are a single walk now, which also removes the pruning of emptied nodes: the error that is stored keeps the node it is stored in non empty. Nodes allocate their children map only when they get children, and the instance path is split once per run of errors sharing it. - `indent` ran a regular expression with a lookahead over every formatted error, and `formatValidationError` built the pretty instance path through an intermediate array and a per-segment regular expression. Formatting a large amount of errors is ~1.5x faster, a successful validation is unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013wqLmVHkAGBsWQgXknEQCK --- declarations/validate.d.ts | 4 +- src/ValidationError.js | 52 +++++++------ src/validate.js | 149 +++++++++++++++++++++---------------- test/filter-errors.test.js | 18 +++++ 4 files changed, 133 insertions(+), 90 deletions(-) diff --git a/declarations/validate.d.ts b/declarations/validate.d.ts index 42738c1..beed853 100644 --- a/declarations/validate.d.ts +++ b/declarations/validate.d.ts @@ -62,9 +62,9 @@ export type ErrorPathNode = { */ indexes: number[]; /** - * nodes of nested instance paths, keyed by json pointer segment + * nodes of nested instance paths, keyed by json pointer segment, created on demand */ - children: Map; + children: Map | undefined; /** * amount of errors stored in this node and in all its descendants */ diff --git a/src/ValidationError.js b/src/ValidationError.js index f6b10ea..b4e1ab5 100644 --- a/src/ValidationError.js +++ b/src/ValidationError.js @@ -47,12 +47,14 @@ const SPECIFICITY = { absolutePath: 2, }; +const IS_NUMERIC = /^-?\d+$/; + /** * @param {string} value value * @returns {value is number} true when is number, otherwise false */ function isNumeric(value) { - return /^-?\d+$/.test(value); + return IS_NUMERIC.test(value); } /** @@ -184,12 +186,24 @@ function groupChildrenByFirstChild(children) { } /** + * Indents every line of `str` but the first one, a trailing new line is left alone. * @param {string} str string * @param {string} prefix prefix * @returns {string} string with indent and prefix */ function indent(str, prefix) { - return str.replace(/\n(?!$)/g, `\n${prefix}`); + const firstNewLine = str.indexOf("\n"); + + // Most formatted errors are a single line + if (firstNewLine === -1 || firstNewLine === str.length - 1) { + return str; + } + + const separator = `\n${prefix}`; + + return str.charCodeAt(str.length - 1) === 10 /* \n */ + ? `${str.slice(0, -1).split("\n").join(separator)}\n` + : str.split("\n").join(separator); } /** @@ -919,27 +933,21 @@ class ValidationError extends Error { formatValidationError(error) { const { keyword, instancePath: errorInstancePath } = error; - const splittedInstancePath = errorInstancePath.split("/"); - /** - * @type {string[]} - */ - const defaultValue = []; - const prettyInstancePath = splittedInstancePath - .reduce((acc, val) => { - if (val.length > 0) { - if (isNumeric(val)) { - acc.push(`[${val}]`); - } else if (/^\[/.test(val)) { - acc.push(val); - } else { - acc.push(`.${val}`); - } - } + let instancePath = this.baseDataPath; + + for (const part of errorInstancePath.split("/")) { + if (part.length === 0) { + continue; + } - return acc; - }, defaultValue) - .join(""); - const instancePath = `${this.baseDataPath}${prettyInstancePath}`; + if (isNumeric(part)) { + instancePath += `[${part}]`; + } else if (part.charCodeAt(0) === 91 /* [ */) { + instancePath += part; + } else { + instancePath += `.${part}`; + } + } // const { keyword, instancePath: errorInstancePath } = error; // const instancePath = `${this.baseDataPath}${errorInstancePath.replace(/\//g, '.')}`; diff --git a/src/validate.js b/src/validate.js index 50adcfb..57b7dbd 100644 --- a/src/validate.js +++ b/src/validate.js @@ -123,6 +123,9 @@ function disableValidation() { } } +const IS_TRUTHY = /^(?:y|yes|true|1|on)$/i; +const IS_FALSY = /^(?:n|no|false|0|off)$/i; + // Check if we need to confirm /** * @returns {boolean} true when need validate, otherwise false @@ -132,14 +135,18 @@ function needValidate() { return false; } - if (process && process.env && process.env.SKIP_VALIDATION) { - const value = process.env.SKIP_VALIDATION.trim(); + // Reading a variable from `process.env` is expensive and this runs on every validation, + // so it is read only once + const value = process && process.env && process.env.SKIP_VALIDATION; + + if (value) { + const trimmedValue = value.trim(); - if (/^(?:y|yes|true|1|on)$/i.test(value)) { + if (IS_TRUTHY.test(trimmedValue)) { return false; } - if (/^(?:n|no|false|0|off)$/i.test(value)) { + if (IS_FALSY.test(trimmedValue)) { return true; } } @@ -152,7 +159,7 @@ function needValidate() { * instance path. * @typedef {object} ErrorPathNode * @property {number[]} indexes positions (in the result array) of the errors reported for exactly this instance path - * @property {Map} children nodes of nested instance paths, keyed by json pointer segment + * @property {Map | undefined} children nodes of nested instance paths, keyed by json pointer segment, created on demand * @property {number} size amount of errors stored in this node and in all its descendants */ @@ -160,7 +167,7 @@ function needValidate() { * @returns {ErrorPathNode} empty node */ function createErrorPathNode() { - return { indexes: [], children: new Map(), size: 0 }; + return { indexes: [], children: undefined, size: 0 }; } /** @@ -175,89 +182,88 @@ function parseInstancePath(instancePath) { } /** - * @param {ErrorPathNode} root root node - * @param {string[]} segments json pointer segments of the error instance path - * @param {number} index position of the error in the result array - * @returns {void} + * @param {number} a a + * @param {number} b b + * @returns {number} comparison result */ -function addErrorPath(root, segments, index) { - let node = root; - - node.size += 1; - - for (const segment of segments) { - let child = node.children.get(segment); - - if (!child) { - child = createErrorPathNode(); - node.children.set(segment, child); - } - - node = child; - node.size += 1; - } - - node.indexes.push(index); +function compareNumbers(a, b) { + return a - b; } /** - * Removes and returns every error reported for the given instance path or for anything nested - * inside it, in the order the errors were reported. + * Stores an error at the given instance path and removes every error already stored for that path + * or for anything nested inside it, since those become children of the new one. + * + * The new error keeps the node non empty, so no node ever has to be pruned. * @param {ErrorPathNode} root root node * @param {string[]} segments json pointer segments of the error instance path - * @returns {number[]} positions (in the result array) of the removed errors + * @param {number} index position of the error in the result array + * @returns {number[]} positions (in the result array) of the removed errors, in the order they were reported */ -function takeErrorPaths(root, segments) { +function replaceErrorPath(root, segments, index) { /** @type {ErrorPathNode[]} */ - const ancestors = [root]; + const ancestors = []; + /** @type {number[]} */ + const indexes = []; let node = root; + let depth = 0; + // A single walk down, creating the missing nodes on the way for (const segment of segments) { - const child = node.children.get(segment); + ancestors[depth] = node; + depth += 1; + + let { children } = node; + + if (!children) { + children = new Map(); + node.children = children; + } + + let child = children.get(segment); - // Nothing was reported below this instance path if (!child) { - return []; + child = createErrorPathNode(); + children.set(segment, child); } node = child; - ancestors.push(node); } - /** @type {number[]} */ - const indexes = []; - /** @type {ErrorPathNode[]} */ - const stack = [node]; + if (node.size > 0) { + /** @type {ErrorPathNode[]} */ + const stack = [node]; - while (stack.length > 0) { - const current = /** @type {ErrorPathNode} */ (stack.pop()); + while (stack.length > 0) { + const current = /** @type {ErrorPathNode} */ (stack.pop()); - for (const index of current.indexes) { - indexes.push(index); - } + for (const collected of current.indexes) { + indexes.push(collected); + } - for (const child of current.children.values()) { - stack.push(child); + if (current.children) { + for (const child of current.children.values()) { + stack.push(child); + } + } } + + // The subtree has been consumed, so detach it + node.children = undefined; + indexes.sort(compareNumbers); } - // The whole subtree has been consumed, so detach it and drop the ancestors it left empty, - // otherwise later lookups would keep walking over nodes without errors - const removed = indexes.length; + node.indexes = [index]; - node.indexes = []; - node.children.clear(); - node.size = 0; + const delta = 1 - node.size; - for (let i = ancestors.length - 2; i >= 0; i--) { - ancestors[i].size -= removed; + node.size = 1; - if (ancestors[i + 1].size === 0) { - ancestors[i].children.delete(segments[i]); - } + for (let i = 0; i < depth; i++) { + ancestors[i].size += delta; } - return indexes.sort((a, b) => a - b); + return indexes; } /** @@ -270,13 +276,24 @@ function filterErrors(errors) { /** @type {(SchemaUtilErrorObject | undefined)[]} */ const newErrors = []; const root = createErrorPathNode(); + let lastInstancePath; + /** @type {string[]} */ + let segments = []; for (const error of /** @type {SchemaUtilErrorObject[]} */ (errors)) { - const segments = parseInstancePath(error.instancePath); + const { instancePath } = error; + + // Errors reported next to each other usually share the instance path, i.e. the branches of an + // `anyOf`, so the split is worth reusing + if (instancePath !== lastInstancePath) { + lastInstancePath = instancePath; + segments = parseInstancePath(instancePath); + } + /** @type {SchemaUtilErrorObject[]} */ let children = []; - for (const index of takeErrorPaths(root, segments)) { + for (const index of replaceErrorPath(root, segments, newErrors.length)) { const oldError = /** @type {SchemaUtilErrorObject} */ (newErrors[index]); newErrors[index] = undefined; @@ -301,7 +318,6 @@ function filterErrors(errors) { error.children = children; } - addErrorPath(root, segments, newErrors.length); newErrors.push(error); } @@ -340,9 +356,10 @@ function validate(schema, options, configuration) { if (Array.isArray(options)) { for (let i = 0; i <= options.length - 1; i++) { - errors.push( - ...validateObject(schema, options[i]).map((err) => applyPrefix(err, i)), - ); + // Not `errors.push(...)`, a large amount of errors would overflow the call stack + for (const error of validateObject(schema, options[i])) { + errors.push(applyPrefix(error, i)); + } } } else { errors = validateObject(schema, options); diff --git a/test/filter-errors.test.js b/test/filter-errors.test.js index c160ce3..2e6cc98 100644 --- a/test/filter-errors.test.js +++ b/test/filter-errors.test.js @@ -140,6 +140,24 @@ describe("filter errors", () => { expect(elapsed).toBeLessThan(5000); }, 30000); + // The errors of each entry used to be spread into the result with `push(...errors)`, which + // overflows the call stack for a large amount of errors + it("should report a large amount of errors for an array of options", () => { + const schema = { + type: "object", + properties: { + list: { type: "array", items: { type: "string" } }, + }, + }; + + const options = [{ list: Array.from({ length: 200000 }, () => 1) }]; + + const errors = getErrors(schema, options); + + expect(errors).toHaveLength(200000); + expect(errors[0].instancePath).toBe("[0]/list/0"); + }, 30000); + it("should filter a large amount of errors with distinct instance paths in a reasonable time", () => { const schema = { type: "object", From 616d1e844bb09e15c8aaf654cace89682b17f0ae Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Mon, 7 Sep 2026 21:04:19 +0000 Subject: [PATCH 3/7] perf: only list the first errors and build the message lazily A configuration with a lot of invalid values produced an unreadable and very large message: 200000 invalid values ended up as a 8.9MB message, and a single `anyOf` error with 200000 children as a 20MB one, because every error was listed. At most 100 errors of a list are listed now, the rest is only counted: - configuration.list[99] should be a string. - and 199900 more errors `errors` still holds every error, so nothing is lost for consumers that read them instead of the message. The message is also built on the first access of `message` rather than in the constructor. Formatting is by far the most expensive part of an invalid configuration and consumers that only look at `errors` never need it. The accessor is replaced by a plain property once the message is built, so `message` keeps behaving like the `message` of any other error - own, enumerable, writable, and part of the stack. It comes from one shared descriptor because defining it per error allocated a context and two functions for every error object, which was slower than building the message eagerly. For 200000 errors: 513ms and 120MB -> 325ms and 96MB when the message is read, 440ms -> 257ms when only `errors` are read. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013wqLmVHkAGBsWQgXknEQCK --- src/ValidationError.js | 120 ++++++++++++++++++------ test/large-errors.test.js | 189 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 281 insertions(+), 28 deletions(-) create mode 100644 test/large-errors.test.js diff --git a/src/ValidationError.js b/src/ValidationError.js index b4e1ab5..2b917f9 100644 --- a/src/ValidationError.js +++ b/src/ValidationError.js @@ -206,6 +206,35 @@ function indent(str, prefix) { : str.split("\n").join(separator); } +// A list of errors longer than this is not readable anyway and formatting it can take a lot of +// memory, i.e. a configuration with 200000 invalid values used to produce a 20MB long message +const MAX_LISTED_ERRORS = 100; + +/** + * Formats a list of errors, listing at most `MAX_LISTED_ERRORS` of them and only counting the rest. + * @param {SchemaUtilErrorObject[]} errors errors + * @param {string} bullet marker put in front of every entry + * @param {(error: SchemaUtilErrorObject) => string} format formats a single error + * @returns {string} formatted list of errors + */ +function formatErrorList(errors, bullet, format) { + const listed = Math.min(errors.length, MAX_LISTED_ERRORS); + /** @type {string[]} */ + const lines = []; + + for (let i = 0; i < listed; i++) { + lines.push(`${bullet}${indent(format(errors[i]), " ")}`); + } + + const rest = errors.length - listed; + + if (rest > 0) { + lines.push(`${bullet}and ${rest} more error${rest > 1 ? "s" : ""}`); + } + + return lines.join("\n"); +} + /** * @param {Schema} schema schema * @returns {schema is (Schema & { not: Schema })} true when `not` in schema, otherwise false @@ -403,6 +432,55 @@ function formatHints(hints) { const getUtilHints = memoize(() => require("./util/hints")); +/** + * Replaces the lazy `message` accessor by a plain property, so that the message is built once and + * behaves like the `message` of any other error afterwards. + * @param {ValidationError} error error + * @param {string} message message + * @returns {string} message + */ +function setMessage(error, message) { + Object.defineProperty(error, "message", { + configurable: true, + enumerable: true, + writable: true, + value: message, + }); + + return message; +} + +// One shared descriptor, defining the accessor per error would allocate a context and two +// functions for every error object +const LAZY_MESSAGE = { + configurable: true, + enumerable: true, + /** + * @this {ValidationError} + * @returns {string} message + */ + get() { + const header = `Invalid ${this.baseDataPath} object. ${ + this.headerName + } has been initialized using ${getArticle(this.baseDataPath)} ${ + this.baseDataPath + } object that does not match the API schema.\n`; + + return setMessage( + this, + `${header}${this.formatValidationErrors(this.errors)}`, + ); + }, + /** + * @this {ValidationError} + * @param {string} value value + * @returns {void} + */ + set(value) { + setMessage(this, value); + }, +}; + /** * @param {Schema} schema schema * @param {boolean} logic logic @@ -464,14 +542,9 @@ class ValidationError extends Error { /** @type {PostFormatter | null} */ this.postFormatter = configuration.postFormatter || null; - const header = `Invalid ${this.baseDataPath} object. ${ - this.headerName - } has been initialized using ${getArticle(this.baseDataPath)} ${ - this.baseDataPath - } object that does not match the API schema.\n`; - - /** @type {string} */ - this.message = `${header}${this.formatValidationErrors(errors)}`; + // Formatting the errors is by far the most expensive part of an invalid configuration and + // consumers that only look at `errors` never need it, so the message is built on first access + Object.defineProperty(this, "message", LAZY_MESSAGE); } /** @@ -1328,16 +1401,11 @@ class ValidationError extends Error { return `${instancePath} should be one of these:\n${this.getSchemaPartText( parentSchema, - )}\nDetails:\n${filteredChildren - .map( - /** - * @param {SchemaUtilErrorObject} nestedError nested error - * @returns {string} formatted errors - */ - (nestedError) => - ` * ${indent(this.formatValidationError(nestedError), " ")}`, - ) - .join("\n")}`; + )}\nDetails:\n${formatErrorList( + filteredChildren, + " * ", + (nestedError) => this.formatValidationError(nestedError), + )}`; } return `${instancePath} should be one of these:\n${this.getSchemaPartText( @@ -1380,17 +1448,13 @@ class ValidationError extends Error { * @returns {string} formatted errors */ formatValidationErrors(errors) { - return errors - .map((error) => { - let formattedError = this.formatValidationError(error); - - if (this.postFormatter) { - formattedError = this.postFormatter(formattedError, error); - } + return formatErrorList(errors, " - ", (error) => { + const formattedError = this.formatValidationError(error); - return ` - ${indent(formattedError, " ")}`; - }) - .join("\n"); + return this.postFormatter + ? this.postFormatter(formattedError, error) + : formattedError; + }); } } diff --git a/test/large-errors.test.js b/test/large-errors.test.js new file mode 100644 index 0000000..c30b4a2 --- /dev/null +++ b/test/large-errors.test.js @@ -0,0 +1,189 @@ +import { validate } from "../src"; + +// eslint-disable-next-line jsdoc/reject-any-type +/** @typedef {any} EXPECTED_ANY */ + +/** + * @param {object} schema schema + * @param {object} options options + * @returns {Error & { errors: EXPECTED_ANY[] }} thrown error + */ +function getError(schema, options) { + try { + validate(schema, options); + } catch (error) { + if (error.name !== "ValidationError") { + throw error; + } + + return error; + } + + throw new Error("Validation didn't fail"); +} + +/** + * @param {number} length length + * @returns {Record} object with unknown properties + */ +function unknownProperties(length) { + /** @type {Record} */ + const options = {}; + + for (let i = 0; i < length; i++) { + options[`unknown${i}`] = 1; + } + + return options; +} + +describe("large amount of errors", () => { + it("should list at most 100 errors and count the rest", () => { + const schema = { + type: "object", + properties: { list: { type: "array", items: { type: "string" } } }, + }; + + const { message, errors } = getError(schema, { + list: Array.from({ length: 5000 }, () => 1), + }); + const listed = message.split("\n").filter((line) => line.startsWith(" - ")); + + expect(errors).toHaveLength(5000); + expect(listed).toHaveLength(101); + expect(listed[0]).toBe(" - configuration.list[0] should be a string."); + expect(listed[100]).toBe(" - and 4900 more errors"); + }); + + it("should list at most 100 errors of a `anyOf` and count the rest", () => { + const schema = { + type: "object", + properties: { + a: { + anyOf: [ + { + type: "object", + additionalProperties: false, + properties: { x: { type: "string" } }, + }, + { type: "string" }, + ], + }, + }, + }; + + const { message } = getError(schema, { a: unknownProperties(5000) }); + const listed = message.split("\n").filter((line) => line.includes(" * ")); + + expect(listed).toHaveLength(101); + expect(listed[100]).toContain("* and 4900 more errors"); + }); + + it("should not list a count when nothing was left out", () => { + const schema = { + type: "object", + properties: { list: { type: "array", items: { type: "string" } } }, + }; + + const { message } = getError(schema, { list: [1, 2, 3] }); + + expect(message).not.toContain("more error"); + expect( + message.split("\n").filter((line) => line.startsWith(" - ")), + ).toHaveLength(3); + }); + + it("should use the singular form for a single left out error", () => { + const schema = { + type: "object", + properties: { list: { type: "array", items: { type: "string" } } }, + }; + + const { message } = getError(schema, { + list: Array.from({ length: 101 }, () => 1), + }); + + expect(message).toContain(" - and 1 more error"); + expect(message).not.toContain("more errors"); + }); + + it("should keep the message small for a huge amount of errors", () => { + const schema = { + type: "object", + properties: { list: { type: "array", items: { type: "string" } } }, + }; + + const { message, errors } = getError(schema, { + list: Array.from({ length: 200000 }, () => 1), + }); + + expect(errors).toHaveLength(200000); + expect(message.length).toBeLessThan(100 * 1024); + }, 30000); +}); + +describe("lazy message", () => { + const schema = { + type: "object", + properties: { a: { type: "string" }, b: { type: "number" } }, + }; + const options = { a: 1, b: "x" }; + + it("should not build the message before it is read", () => { + const error = getError(schema, options); + const descriptor = Object.getOwnPropertyDescriptor(error, "message"); + + expect(typeof descriptor.get).toBe("function"); + expect(descriptor.enumerable).toBe(true); + expect(descriptor.configurable).toBe(true); + }); + + it("should build the message on read and keep it as a plain property", () => { + const error = getError(schema, options); + const { message } = error; + + expect(message).toContain("configuration.a should be a string."); + expect(message).toContain("configuration.b should be a number."); + expect(error.message).toBe(message); + expect(Object.getOwnPropertyDescriptor(error, "message")).toStrictEqual({ + value: message, + writable: true, + enumerable: true, + configurable: true, + }); + }); + + it("should allow to overwrite the message", () => { + const error = getError(schema, options); + + error.message = "custom"; + + expect(error.message).toBe("custom"); + expect(Object.getOwnPropertyDescriptor(error, "message").value).toBe( + "custom", + ); + }); + + it("should keep the message an own enumerable property", () => { + const error = getError(schema, options); + + expect(Object.keys(error)).toContain("message"); + expect(JSON.parse(JSON.stringify(error)).message).toBe(error.message); + }); + + it("should not share the message between errors", () => { + const first = getError(schema, options); + const second = getError(schema, { a: 1 }); + + first.message = "custom"; + + expect(second.message).not.toBe("custom"); + expect(second.message).toContain("configuration.a should be a string."); + }); + + it("should include the message in the stack", () => { + expect(getError(schema, options).stack).toContain( + "Invalid configuration object", + ); + }); +}); From 46e4533b37439a37d6614c736726bfb8543d3e09 Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Mon, 7 Sep 2026 21:04:20 +0000 Subject: [PATCH 4/7] perf: build the message eagerly again Benchmarking the real `sass-loader` schema showed that building the message on the first access of `message` costs more than it saves. Defining the accessor and replacing it by a plain property once the message is read adds about 2us to every reported error, which is a 3-15% regression for the amount of errors a loader actually reports: invalid option name 14.0us -> 17.2us invalid option types 18.1us -> 19.8us Listing only the first errors of a list already removed most of the formatting it was meant to avoid, so what is left to save is small and only applies to consumers that never read `message` - `webpack` prints it, so it always does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013wqLmVHkAGBsWQgXknEQCK --- src/ValidationError.js | 60 +++++------------------------------ test/large-errors.test.js | 66 --------------------------------------- 2 files changed, 8 insertions(+), 118 deletions(-) diff --git a/src/ValidationError.js b/src/ValidationError.js index 2b917f9..d8c58a0 100644 --- a/src/ValidationError.js +++ b/src/ValidationError.js @@ -432,55 +432,6 @@ function formatHints(hints) { const getUtilHints = memoize(() => require("./util/hints")); -/** - * Replaces the lazy `message` accessor by a plain property, so that the message is built once and - * behaves like the `message` of any other error afterwards. - * @param {ValidationError} error error - * @param {string} message message - * @returns {string} message - */ -function setMessage(error, message) { - Object.defineProperty(error, "message", { - configurable: true, - enumerable: true, - writable: true, - value: message, - }); - - return message; -} - -// One shared descriptor, defining the accessor per error would allocate a context and two -// functions for every error object -const LAZY_MESSAGE = { - configurable: true, - enumerable: true, - /** - * @this {ValidationError} - * @returns {string} message - */ - get() { - const header = `Invalid ${this.baseDataPath} object. ${ - this.headerName - } has been initialized using ${getArticle(this.baseDataPath)} ${ - this.baseDataPath - } object that does not match the API schema.\n`; - - return setMessage( - this, - `${header}${this.formatValidationErrors(this.errors)}`, - ); - }, - /** - * @this {ValidationError} - * @param {string} value value - * @returns {void} - */ - set(value) { - setMessage(this, value); - }, -}; - /** * @param {Schema} schema schema * @param {boolean} logic logic @@ -542,9 +493,14 @@ class ValidationError extends Error { /** @type {PostFormatter | null} */ this.postFormatter = configuration.postFormatter || null; - // Formatting the errors is by far the most expensive part of an invalid configuration and - // consumers that only look at `errors` never need it, so the message is built on first access - Object.defineProperty(this, "message", LAZY_MESSAGE); + const header = `Invalid ${this.baseDataPath} object. ${ + this.headerName + } has been initialized using ${getArticle(this.baseDataPath)} ${ + this.baseDataPath + } object that does not match the API schema.\n`; + + /** @type {string} */ + this.message = `${header}${this.formatValidationErrors(errors)}`; } /** diff --git a/test/large-errors.test.js b/test/large-errors.test.js index c30b4a2..0e6ecb6 100644 --- a/test/large-errors.test.js +++ b/test/large-errors.test.js @@ -121,69 +121,3 @@ describe("large amount of errors", () => { expect(message.length).toBeLessThan(100 * 1024); }, 30000); }); - -describe("lazy message", () => { - const schema = { - type: "object", - properties: { a: { type: "string" }, b: { type: "number" } }, - }; - const options = { a: 1, b: "x" }; - - it("should not build the message before it is read", () => { - const error = getError(schema, options); - const descriptor = Object.getOwnPropertyDescriptor(error, "message"); - - expect(typeof descriptor.get).toBe("function"); - expect(descriptor.enumerable).toBe(true); - expect(descriptor.configurable).toBe(true); - }); - - it("should build the message on read and keep it as a plain property", () => { - const error = getError(schema, options); - const { message } = error; - - expect(message).toContain("configuration.a should be a string."); - expect(message).toContain("configuration.b should be a number."); - expect(error.message).toBe(message); - expect(Object.getOwnPropertyDescriptor(error, "message")).toStrictEqual({ - value: message, - writable: true, - enumerable: true, - configurable: true, - }); - }); - - it("should allow to overwrite the message", () => { - const error = getError(schema, options); - - error.message = "custom"; - - expect(error.message).toBe("custom"); - expect(Object.getOwnPropertyDescriptor(error, "message").value).toBe( - "custom", - ); - }); - - it("should keep the message an own enumerable property", () => { - const error = getError(schema, options); - - expect(Object.keys(error)).toContain("message"); - expect(JSON.parse(JSON.stringify(error)).message).toBe(error.message); - }); - - it("should not share the message between errors", () => { - const first = getError(schema, options); - const second = getError(schema, { a: 1 }); - - first.message = "custom"; - - expect(second.message).not.toBe("custom"); - expect(second.message).toContain("configuration.a should be a string."); - }); - - it("should include the message in the stack", () => { - expect(getError(schema, options).stack).toContain( - "Invalid configuration object", - ); - }); -}); From 654daaf8223c0b077c9d0257b6303f2ce70ccb88 Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Mon, 7 Sep 2026 21:04:21 +0000 Subject: [PATCH 5/7] perf: scan the collected errors directly for a small amount of errors The instance path index that keeps `filterErrors` from going quadratic costs more than it saves for the handful of errors a loader or plugin usually reports - it allocates a node, a map and a few arrays per error where a direct scan only compares strings. The errors are scanned directly up to `MAX_SCANNED_ERRORS` and indexed above it, which is where the two were measured to cross: errors 1 3 8 24 32 scan 72ns 174ns 557ns 2201ns 5018ns index 232ns 567ns 1530ns 3751ns 5707ns Both nest errors identically, checked over 16400 generated error sets covering 0 to 40 errors and every combination of nested, sibling and prefix sharing instance paths. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013wqLmVHkAGBsWQgXknEQCK --- src/validate.js | 111 ++++++++++++++++++++++++++++++++----- test/filter-errors.test.js | 27 +++++++++ 2 files changed, 123 insertions(+), 15 deletions(-) diff --git a/src/validate.js b/src/validate.js index 57b7dbd..74c828f 100644 --- a/src/validate.js +++ b/src/validate.js @@ -266,6 +266,97 @@ function replaceErrorPath(root, segments, index) { return indexes; } +/** + * Moves an already reported error under `children`, hoisting the children it collected itself. + * @param {SchemaUtilErrorObject[]} children collected children + * @param {SchemaUtilErrorObject} oldError error to nest + * @returns {SchemaUtilErrorObject[]} collected children, which may be a different array + */ +function absorbError(children, oldError) { + let newChildren = children; + + if (oldError.children) { + if (newChildren.length === 0) { + // Adopt the array instead of copying it - a long run of sibling errors re-parents the + // previously collected children on every step, so copying them would be quadratic + newChildren = oldError.children; + } else { + for (const child of oldError.children) { + newChildren.push(child); + } + } + } + + oldError.children = undefined; + newChildren.push(oldError); + + return newChildren; +} + +/** + * Whether an instance path points at `ancestorPath` itself or at something nested inside it, i.e. + * `"/rules/0"` is inside `"/rules"` but `"/rulesets"` is not. + * @param {string} instancePath instance path + * @param {string} ancestorPath ancestor instance path + * @returns {boolean} true when at or below the ancestor path, otherwise false + */ +function isAtOrBelow(instancePath, ancestorPath) { + if (instancePath.length === ancestorPath.length) { + return instancePath === ancestorPath; + } + + return ( + instancePath.length > ancestorPath.length && + // the next character has to be a separator, otherwise it is a sibling with a longer name + instancePath.charCodeAt(ancestorPath.length) === 47 /* / */ && + instancePath.startsWith(ancestorPath) + ); +} + +// Below this amount of errors scanning the collected errors directly is cheaper than indexing +// them, above it the index is what keeps the whole thing from going quadratic +const MAX_SCANNED_ERRORS = 24; + +/** + * Same as `filterErrors`, without the instance path index - for a small amount of errors walking + * the collected errors is cheaper than building one. + * @param {SchemaUtilErrorObject[]} errors array of error objects + * @returns {SchemaUtilErrorObject[]} filtered array of objects + */ +function scanErrors(errors) { + /** @type {SchemaUtilErrorObject[]} */ + const newErrors = []; + + for (const error of errors) { + const { instancePath } = error; + /** @type {SchemaUtilErrorObject[]} */ + let children = []; + let kept = 0; + + for (let i = 0; i < newErrors.length; i++) { + const oldError = newErrors[i]; + + if (!isAtOrBelow(oldError.instancePath, instancePath)) { + newErrors[kept] = oldError; + kept += 1; + continue; + } + + children = absorbError(children, oldError); + } + + newErrors.length = kept; + + if (children.length) { + error.children = children; + } + + newErrors.push(error); + } + + return newErrors; +} + /** * Nests every error under the last reported error that covers its instance path, so that only the * outermost errors are left at the top level. @@ -273,6 +364,10 @@ function replaceErrorPath(root, segments, index) { * @returns {SchemaUtilErrorObject[]} filtered array of objects */ function filterErrors(errors) { + if (errors.length <= MAX_SCANNED_ERRORS) { + return scanErrors(/** @type {SchemaUtilErrorObject[]} */ (errors)); + } + /** @type {(SchemaUtilErrorObject | undefined)[]} */ const newErrors = []; const root = createErrorPathNode(); @@ -297,21 +392,7 @@ function filterErrors(errors) { const oldError = /** @type {SchemaUtilErrorObject} */ (newErrors[index]); newErrors[index] = undefined; - - if (oldError.children) { - if (children.length === 0) { - // Adopt the array instead of copying it - a long run of sibling errors re-parents the - // previously collected children on every step, so copying them would be quadratic - children = oldError.children; - } else { - for (const child of oldError.children) { - children.push(child); - } - } - } - - oldError.children = undefined; - children.push(oldError); + children = absorbError(children, oldError); } if (children.length) { diff --git a/test/filter-errors.test.js b/test/filter-errors.test.js index 2e6cc98..e816099 100644 --- a/test/filter-errors.test.js +++ b/test/filter-errors.test.js @@ -116,6 +116,33 @@ describe("filter errors", () => { ).toBe(true); }); + // Errors are collected by scanning them directly below a threshold and through an instance path + // index above it, both have to nest them the same way + it.each([3, 40])("should nest the same way for %i errors", (length) => { + const properties = { nested: { type: "object", properties: {} } }; + + for (let i = 0; i < length; i++) { + properties.nested.properties[`p${i}`] = { type: "string" }; + } + + const schema = { type: "object", properties }; + const options = { nested: {} }; + + for (let i = 0; i < length; i++) { + options.nested[`p${i}`] = 1; + } + + const errors = getErrors(schema, options); + + expect(errors).toHaveLength(length); + expect(errors.every((error) => typeof error.children === "undefined")).toBe( + true, + ); + expect(errors.map((error) => error.instancePath)).toStrictEqual( + Array.from({ length }, (_, i) => `/nested/p${i}`), + ); + }); + // `filterErrors` used to be quadratic in the amount of reported errors, so a large invalid // configuration was enough to lock up the process for minutes it("should filter a large amount of sibling errors in a reasonable time", () => { From 4a6fb30c8c9018ebfc441820f3d4c175216347c1 Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Mon, 7 Sep 2026 21:04:22 +0000 Subject: [PATCH 6/7] perf: read `process.env.SKIP_VALIDATION` when loaded instead of on every validation Reading a variable from `process.env` goes through an interceptor and costs about 250ns, which was 84% of a successful validation against the `sass-loader` schema - the validation itself only takes ~20ns. It is read once now, when the module is loaded, which is how it is used in practice: the variable is set before the process starts. valid options, sass-loader schema 345ns -> 41ns valid options, all options set 399ns -> 98ns `enableValidation()`/`disableValidation()` still take effect immediately. They keep writing `process.env` for copies of `schema-utils` too old to know about it, and share the resolved state through the global object, so copies of different versions still turn each other on and off - without the `process.env` round trip they needed before. Writing `process.env.SKIP_VALIDATION` after `schema-utils` has been loaded no longer has an effect on the copies that are already loaded, which the README now says. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013wqLmVHkAGBsWQgXknEQCK --- README.md | 9 +++ declarations/validate.d.ts | 9 +++ src/validate.js | 84 ++++++++++++++--------- test/api.test.js | 132 +++++++++++++++++++++++++++---------- 4 files changed, 171 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 6ff5176..a364b1f 100644 --- a/README.md +++ b/README.md @@ -291,6 +291,15 @@ Supported values (case insensitive): - `yes`/`y`/`true`/`1`/`on` - `no`/`n`/`false`/`0`/`off` +The variable is read when `schema-utils` is loaded, so set it before starting the process: + +```console +SKIP_VALIDATION=y webpack +``` + +Use `enableValidation()`/`disableValidation()` to change it while the process is running - they +take effect immediately and apply to every copy of `schema-utils` in the process. + ## Contributing Please take a moment to read our contributing guidelines if you haven't yet done so. diff --git a/declarations/validate.d.ts b/declarations/validate.d.ts index beed853..8b5077d 100644 --- a/declarations/validate.d.ts +++ b/declarations/validate.d.ts @@ -70,6 +70,15 @@ export type ErrorPathNode = { */ size: number; }; +/** + * Whether validation is skipped, shared by every `schema-utils` in the process. + */ +export type SkipValidationState = { + /** + * true when validation is disabled + */ + skip: boolean; +}; /** * @returns {void} */ diff --git a/src/validate.js b/src/validate.js index 74c828f..a38ebdd 100644 --- a/src/validate.js +++ b/src/validate.js @@ -93,19 +93,66 @@ function applyPrefix(error, idx) { return error; } -let skipValidation = false; +const IS_TRUTHY = /^(?:y|yes|true|1|on)$/i; +const IS_FALSY = /^(?:n|no|false|0|off)$/i; + +/** + * @returns {boolean} true when `process.env.SKIP_VALIDATION` asks to skip validation + */ +function skipValidationFromEnv() { + const value = + process && process.env ? process.env.SKIP_VALIDATION : undefined; + + if (value) { + const trimmedValue = value.trim(); + + if (IS_TRUTHY.test(trimmedValue)) { + return true; + } -// We use `process.env.SKIP_VALIDATION` because you can have multiple `schema-utils` with different version, -// so we want to disable it globally, `process.env` doesn't supported by browsers, so we have the local `skipValidation` variables + if (IS_FALSY.test(trimmedValue)) { + return false; + } + } + + return false; +} + +/** + * Whether validation is skipped, shared by every `schema-utils` in the process. + * @typedef {object} SkipValidationState + * @property {boolean} skip true when validation is disabled + */ + +const SKIP_VALIDATION_KEY = Symbol.for("schema-utils/skipValidation"); +const globalObject = + /** @type {Record} */ + ( + /** @type {unknown} */ + // eslint-disable-next-line no-undef + typeof globalThis === "undefined" ? global : globalThis + ); + +// `process.env.SKIP_VALIDATION` is read when this module is loaded, not on every validation - +// reading a variable from `process.env` costs about 250ns, which is most of the time a successful +// validation takes. Later changes go through `enableValidation`/`disableValidation`, which share +// the resolved state through the global object so that `schema-utils` copies of different +// versions still turn each other on and off, and keep writing `process.env` for copies too old +// to know about the shared state. +const sharedState = + globalObject[SKIP_VALIDATION_KEY] || + (globalObject[SKIP_VALIDATION_KEY] = { skip: false }); + +sharedState.skip = skipValidationFromEnv(); // Enable validation /** * @returns {void} */ function enableValidation() { - skipValidation = false; + sharedState.skip = false; - // Disable validation for any versions + // Enable validation for any versions if (process && process.env) { process.env.SKIP_VALIDATION = "n"; } @@ -116,42 +163,19 @@ function enableValidation() { * @returns {void} */ function disableValidation() { - skipValidation = true; + sharedState.skip = true; if (process && process.env) { process.env.SKIP_VALIDATION = "y"; } } -const IS_TRUTHY = /^(?:y|yes|true|1|on)$/i; -const IS_FALSY = /^(?:n|no|false|0|off)$/i; - // Check if we need to confirm /** * @returns {boolean} true when need validate, otherwise false */ function needValidate() { - if (skipValidation) { - return false; - } - - // Reading a variable from `process.env` is expensive and this runs on every validation, - // so it is read only once - const value = process && process.env && process.env.SKIP_VALIDATION; - - if (value) { - const trimmedValue = value.trim(); - - if (IS_TRUTHY.test(trimmedValue)) { - return false; - } - - if (IS_FALSY.test(trimmedValue)) { - return true; - } - } - - return true; + return !sharedState.skip; } /** diff --git a/test/api.test.js b/test/api.test.js index 48920f6..ef15e96 100644 --- a/test/api.test.js +++ b/test/api.test.js @@ -6,11 +6,46 @@ import { validate, } from "../src/index"; +// eslint-disable-next-line jsdoc/reject-any-type +/** @typedef {any} EXPECTED_ANY */ + import schemaTitleBrone from "./fixtures/schema-title-broken.json"; import schemaTitle from "./fixtures/schema-title.json"; import schema from "./fixtures/schema.json"; describe("api", () => { + /** + * Loads a fresh copy of `schema-utils`, as if the process had been started with + * `process.env.SKIP_VALIDATION` set to `value` - the variable is read when the module is loaded. + * @param {string | undefined} value value of `process.env.SKIP_VALIDATION` + * @param {(api: EXPECTED_ANY) => void} fn receives the freshly loaded module + * @returns {void} + */ + function withSkipValidation(value, fn) { + const oldValue = process.env.SKIP_VALIDATION; + const set = (newValue) => { + if (typeof newValue === "undefined") { + delete process.env.SKIP_VALIDATION; + } else { + process.env.SKIP_VALIDATION = newValue; + } + }; + + set(value); + + try { + jest.isolateModules(() => { + fn(require("../src/index")); + }); + } finally { + set(oldValue); + // the state is shared with the already loaded copy, reload so it matches the environment again + jest.isolateModules(() => { + require("../src/index"); + }); + } + } + it("should export validate and ValidateError", () => { expect(typeof validate).toBe("function"); expect(typeof ValidationError).toBe("function"); @@ -212,39 +247,31 @@ describe("api", () => { }); it('should allow to disable validation using "process.env.SKIP_VALIDATION"', () => { - const oldValue = process.env.SKIP_VALIDATION; - - let errored; - - process.env.SKIP_VALIDATION = "y"; - - try { - validate(schemaTitle, { foo: "bar" }, { name: "NAME" }); - } catch (error) { - errored = error; - } + withSkipValidation("y", ({ validate: freshValidate }) => { + let errored; - expect(errored).toBeUndefined(); + try { + freshValidate(schemaTitle, { foo: "bar" }, { name: "NAME" }); + } catch (error) { + errored = error; + } - process.env.SKIP_VALIDATION = oldValue; + expect(errored).toBeUndefined(); + }); }); it('should allow to disable validation using "process.env.SKIP_VALIDATION" #2', () => { - const oldValue = process.env.SKIP_VALIDATION; - - let errored; - - process.env.SKIP_VALIDATION = "YeS"; - - try { - validate(schemaTitle, { foo: "bar" }, { name: "NAME" }); - } catch (error) { - errored = error; - } + withSkipValidation("YeS", ({ validate: freshValidate }) => { + let errored; - expect(errored).toBeUndefined(); + try { + freshValidate(schemaTitle, { foo: "bar" }, { name: "NAME" }); + } catch (error) { + errored = error; + } - process.env.SKIP_VALIDATION = oldValue; + expect(errored).toBeUndefined(); + }); }); it('should allow to enable validation using "process.env.SKIP_VALIDATION"', () => { @@ -314,15 +341,54 @@ describe("api", () => { } }); - it("should allow to enable and disable validation using API", () => { - process.env.SKIP_VALIDATION = "unknown"; - expect(needValidate()).toBe(true); + it('should read "process.env.SKIP_VALIDATION" when loaded, not on every validation', () => { + enableValidation(); - process.env.SKIP_VALIDATION = "no"; - expect(needValidate()).toBe(true); + try { + process.env.SKIP_VALIDATION = "y"; - process.env.SKIP_VALIDATION = "yes"; - expect(needValidate()).toBe(false); + let errored; + + try { + validate(schemaTitle, { foo: "bar" }, { name: "NAME" }); + } catch (error) { + errored = error; + } + + // the already loaded copy keeps the value it read when it was loaded + expect(errored).toBeDefined(); + } finally { + enableValidation(); + } + }); + + it("should share the state with other copies of `schema-utils`", () => { + try { + disableValidation(); + + jest.isolateModules(() => { + // another copy, as if a dependency depended on a different version + + const api = require("../src/index"); + + expect(api.needValidate()).toBe(false); + + api.enableValidation(); + }); + + // turning it back on in the other copy turns it back on here + expect(needValidate()).toBe(true); + } finally { + enableValidation(); + } + }); + + it("should allow to enable and disable validation using API", () => { + withSkipValidation("unknown", (api) => + expect(api.needValidate()).toBe(true), + ); + withSkipValidation("no", (api) => expect(api.needValidate()).toBe(true)); + withSkipValidation("yes", (api) => expect(api.needValidate()).toBe(false)); enableValidation(); expect(process.env.SKIP_VALIDATION).toBe("n"); From 66cc6200eee20fc8a9b2fe9551d4dba49944cda3 Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Mon, 7 Sep 2026 21:07:56 +0000 Subject: [PATCH 7/7] chore: bump `fast-uri` to 3.1.7 `npm audit` reports 4 high severity advisories for `fast-uri` 3.0.0 - 3.1.5, which `ajv` depends on, so the `Security audit` step of the lint job fails. `ajv` accepts `^3.0.0`, so only the lock file changes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013wqLmVHkAGBsWQgXknEQCK --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2d7461f..a87ef49 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7313,9 +7313,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.5", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.5.tgz", - "integrity": "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw==", + "version": "3.1.7", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.7.tgz", + "integrity": "sha512-dOvZVzjdZdz7phd9v6jCbwxrBW3fK6n8Rc0CtdmM4bumzMnxywBYhuph6J819RRw/ku+rLbelwfMunktuzVVHg==", "funding": [ { "type": "github",