Skip to content

Commit

Permalink
#2043 performance improvement (#2072)
Browse files Browse the repository at this point in the history
* fix: performance improvement (#2043)

* Update src/ValidationError.ts

Co-authored-by: Jason Quense <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jason Quense <[email protected]>

* fix PR comments

* #2043 add Error toStringTag to custom ValidationError class

---------

Co-authored-by: Jason Quense <[email protected]>
  • Loading branch information
tedeschia and jquense authored Aug 28, 2023
1 parent 41b9c58 commit 4de9ea0
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 10 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const parsedUser = await userSchema.validate(
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->


- [Schema basics](#schema-basics)
- [Parsing: Transforms](#parsing-transforms)
- [Validation: Tests](#validation-tests)
Expand Down
15 changes: 10 additions & 5 deletions src/ValidationError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ let strReg = /\$\{\s*(\w+)\s*\}/g;

type Params = Record<string, unknown>;

export default class ValidationError extends Error {
export default class ValidationError implements Error {
name: string;
message: string;
stack?: string | undefined;
value: any;
path?: string;
type?: string;
Expand Down Expand Up @@ -38,9 +41,8 @@ export default class ValidationError extends Error {
value?: any,
field?: string,
type?: string,
disableStack?: boolean,
) {
super();

this.name = 'ValidationError';
this.value = value;
this.path = field;
Expand All @@ -52,7 +54,8 @@ export default class ValidationError extends Error {
toArray(errorOrErrors).forEach((err) => {
if (ValidationError.isError(err)) {
this.errors.push(...err.errors);
this.inner = this.inner.concat(err.inner.length ? err.inner : err);
const innerErrors = err.inner.length ? err.inner : [err];
this.inner.push(...innerErrors);
} else {
this.errors.push(err);
}
Expand All @@ -63,6 +66,8 @@ export default class ValidationError extends Error {
? `${this.errors.length} errors occurred`
: this.errors[0];

if (Error.captureStackTrace) Error.captureStackTrace(this, ValidationError);
if (!disableStack && Error.captureStackTrace)
Error.captureStackTrace(this, ValidationError);
}
[Symbol.toStringTag] = 'Error';
}
32 changes: 29 additions & 3 deletions src/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type SchemaSpec<TDefault> = {
strip?: boolean;
strict?: boolean;
recursive?: boolean;
disableStackTrace?: boolean;
label?: string | undefined;
meta?: SchemaMetadata;
};
Expand Down Expand Up @@ -191,6 +192,7 @@ export default abstract class Schema<
strict: false,
abortEarly: true,
recursive: true,
disableStackTrace: false,
nullable: false,
optional: true,
coerce: true,
Expand Down Expand Up @@ -345,6 +347,8 @@ export default abstract class Schema<
strict: options.strict ?? this.spec.strict,
abortEarly: options.abortEarly ?? this.spec.abortEarly,
recursive: options.recursive ?? this.spec.recursive,
disableStackTrace:
options.disableStackTrace ?? this.spec.disableStackTrace,
};
}

Expand Down Expand Up @@ -499,7 +503,9 @@ export default abstract class Schema<

test(args!, panicOnce, function finishTestRun(err) {
if (err) {
nestedErrors = nestedErrors.concat(err);
Array.isArray(err)
? nestedErrors.push(...err)
: nestedErrors.push(err);
}
if (--count <= 0) {
nextOnce(nestedErrors);
Expand Down Expand Up @@ -553,6 +559,8 @@ export default abstract class Schema<
options?: ValidateOptions<TContext>,
): Promise<this['__outputType']> {
let schema = this.resolve({ ...options, value });
let disableStackTrace =
options?.disableStackTrace ?? schema.spec.disableStackTrace;

return new Promise((resolve, reject) =>
schema._validate(
Expand All @@ -563,7 +571,16 @@ export default abstract class Schema<
reject(error);
},
(errors, validated) => {
if (errors.length) reject(new ValidationError(errors!, validated));
if (errors.length)
reject(
new ValidationError(
errors!,
validated,
undefined,
undefined,
disableStackTrace,
),
);
else resolve(validated as this['__outputType']);
},
),
Expand All @@ -576,6 +593,8 @@ export default abstract class Schema<
): this['__outputType'] {
let schema = this.resolve({ ...options, value });
let result: any;
let disableStackTrace =
options?.disableStackTrace ?? schema.spec.disableStackTrace;

schema._validate(
value,
Expand All @@ -585,7 +604,14 @@ export default abstract class Schema<
throw error;
},
(errors, validated) => {
if (errors.length) throw new ValidationError(errors!, value);
if (errors.length)
throw new ValidationError(
errors!,
value,
undefined,
undefined,
disableStackTrace,
);
result = validated;
},
);
Expand Down
4 changes: 4 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ export interface ValidateOptions<TContext = {}> {
* When false validations will not descend into nested schema (relevant for objects or arrays). Default - true
*/
recursive?: boolean;
/**
* When true ValidationError instance won't include stack trace information. Default - false
*/
disableStackTrace?: boolean;
/**
* Any context needed for validating schema conditions (see: when())
*/
Expand Down
9 changes: 8 additions & 1 deletion src/util/createValidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export type CreateErrorOptions = {
message?: Message<any>;
params?: ExtraParams;
type?: string;
disableStackTrace?: boolean;
};

export type TestContext<TContext = {}> = {
Expand Down Expand Up @@ -79,7 +80,12 @@ export default function createValidation(config: {
next: NextCallback,
) {
const { name, test, params, message, skipAbsent } = config;
let { parent, context, abortEarly = schema.spec.abortEarly } = options;
let {
parent,
context,
abortEarly = schema.spec.abortEarly,
disableStackTrace = schema.spec.disableStackTrace,
} = options;

function resolve<T>(item: T | Reference<T>) {
return Ref.isRef(item) ? item.getValue(value, parent, context) : item;
Expand All @@ -105,6 +111,7 @@ export default function createValidation(config: {
value,
nextParams.path,
overrides.type || name,
overrides.disableStackTrace ?? disableStackTrace,
);
error.params = nextParams;
return error;
Expand Down
13 changes: 12 additions & 1 deletion test/array.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { string, number, object, array, StringSchema, AnySchema } from '../src';
import {
string,
number,
object,
array,
StringSchema,
AnySchema,
ValidationError,
} from '../src';

describe('Array types', () => {
describe('casting', () => {
Expand Down Expand Up @@ -56,6 +64,9 @@ describe('Array types', () => {

let merged = array(number()).concat(array(number().required()));

const ve = new ValidationError('');
// expect(ve.toString()).toBe('[object Error]');
expect(Object.prototype.toString.call(ve)).toBe('[object Error]');
expect((merged.innerType as AnySchema).type).toBe('number');

await expect(merged.validateAt('[0]', undefined)).rejects.toThrowError();
Expand Down

0 comments on commit 4de9ea0

Please sign in to comment.