Skip to content

Commit

Permalink
Fix performance issues and cleanup code (#199)
Browse files Browse the repository at this point in the history
  • Loading branch information
vladfrangu authored Jan 20, 2021
1 parent dbef144 commit 4e3340a
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 82 deletions.
8 changes: 5 additions & 3 deletions source/argument-error.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
import {generateStackTrace} from './utils/generate-stack';

const wrapStackTrace = (error: ArgumentError, stack: string) => `${error.name}: ${error.message}\n${stack}`;

/**
@hidden
*/
export class ArgumentError extends Error {
readonly validationErrors: ReadonlyMap<string, string[]>;
readonly validationErrors: ReadonlyMap<string, Set<string>>;

constructor(message: string, context: Function, stack: string, errors = new Map<string, string[]>()) {
constructor(message: string, context: Function, errors = new Map<string, Set<string>>()) {
super(message);

this.name = 'ArgumentError';

if (Error.captureStackTrace) {
Error.captureStackTrace(this, context);
} else {
this.stack = wrapStackTrace(this, stack);
this.stack = wrapStackTrace(this, generateStackTrace());
}

this.validationErrors = errors;
Expand Down
15 changes: 5 additions & 10 deletions source/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import {BasePredicate, isPredicate} from './predicates/base-predicate';
import modifiers, {Modifiers} from './modifiers';
import predicates, {Predicates} from './predicates';
import test from './test';
import {generateStackTrace} from './utils/generate-stack';

/**
@hidden
*/
export type Main = <T>(value: T, label: string | Function, predicate: BasePredicate<T>, stack: string) => void;
export type Main = <T>(value: T, label: string | Function, predicate: BasePredicate<T>) => void;

// Extends is only necessary for the generated documentation to be cleaner. The loaders below infer the correct type.
export interface Ow extends Modifiers, Predicates {
Expand Down Expand Up @@ -62,8 +61,6 @@ export interface ReusableValidator<T> {
}

const ow = <T>(value: T, labelOrPredicate: unknown, predicate?: BasePredicate<T>) => {
const stack = generateStackTrace();

if (!isPredicate(labelOrPredicate) && typeof labelOrPredicate !== 'string') {
throw new TypeError(`Expected second argument to be a predicate or a string, got \`${typeof labelOrPredicate}\``);
}
Expand All @@ -72,12 +69,12 @@ const ow = <T>(value: T, labelOrPredicate: unknown, predicate?: BasePredicate<T>
// If the second argument is a predicate, infer the label
const stackFrames = callsites();

test(value, () => inferLabel(stackFrames), labelOrPredicate, stack);
test(value, () => inferLabel(stackFrames), labelOrPredicate);

return;
}

test(value, labelOrPredicate, predicate!, stack);
test(value, labelOrPredicate, predicate!);
};

Object.defineProperties(ow, {
Expand All @@ -93,17 +90,15 @@ Object.defineProperties(ow, {
},
create: {
value: <T>(labelOrPredicate: BasePredicate<T> | string | undefined, predicate?: BasePredicate<T>) => (value: T, label?: string) => {
const stack = generateStackTrace();

if (isPredicate(labelOrPredicate)) {
const stackFrames = callsites();

test(value, label ?? (() => inferLabel(stackFrames)), labelOrPredicate, stack);
test(value, label ?? (() => inferLabel(stackFrames)), labelOrPredicate);

return;
}

test(value, label ?? (labelOrPredicate!), predicate!, stack);
test(value, label ?? (labelOrPredicate!), predicate!);
}
}
});
Expand Down
16 changes: 5 additions & 11 deletions source/predicates/any.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ export class AnyPredicate<T = unknown> implements BasePredicate<T> {
private readonly options: PredicateOptions = {}
) {}

[testSymbol](value: T, main: Main, label: string | Function, stack: string): asserts value {
const errors = new Map<string, string[]>();
[testSymbol](value: T, main: Main, label: string | Function): asserts value {
const errors = new Map<string, Set<string>>();

for (const predicate of this.predicates) {
try {
main(value, label, predicate, stack);
main(value, label, predicate);
return;
} catch (error: unknown) {
if (value === undefined && this.options.optional === true) {
Expand All @@ -32,13 +32,8 @@ export class AnyPredicate<T = unknown> implements BasePredicate<T> {
// Get the current errors set, if any.
const alreadyPresent = errors.get(key);

// If they are present already, create a unique set with both current and new values.
if (alreadyPresent) {
errors.set(key, [...new Set([...alreadyPresent, ...value])]);
} else {
// Add the errors found as is to the map.
errors.set(key, value);
}
// Add all errors under the same key
errors.set(key, new Set([...alreadyPresent ?? [], ...value]));
}
}
}
Expand All @@ -51,7 +46,6 @@ export class AnyPredicate<T = unknown> implements BasePredicate<T> {
throw new ArgumentError(
`Any predicate failed with the following errors:\n${message}`,
main,
stack,
errors
);
}
Expand Down
2 changes: 1 addition & 1 deletion source/predicates/base-predicate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ export const isPredicate = (value: unknown): value is BasePredicate => Boolean((
@hidden
*/
export interface BasePredicate<T = unknown> {
[testSymbol](value: T, main: Main, label: string | Function, stack: string): void;
[testSymbol](value: T, main: Main, label: string | Function): void;
}
12 changes: 5 additions & 7 deletions source/predicates/predicate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
/**
@hidden
*/
[testSymbol](value: T, main: Main, label: string | Function, stack: string): asserts value is T {
[testSymbol](value: T, main: Main, label: string | Function): asserts value is T {
// Create a map of labels -> received errors.
const errors = new Map<string, string[]>();
const errors = new Map<string, Set<string>>();

for (const {validator, message} of this.context.validators) {
if (this.options.optional === true && value === undefined) {
Expand Down Expand Up @@ -134,12 +134,10 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
// If we already have any errors for this label.
if (currentErrors) {
// If we don't already have this error logged, add it.
if (!currentErrors.includes(errorMessage)) {
currentErrors.push(errorMessage);
}
currentErrors.add(errorMessage);
} else {
// Set this label and error in the full map.
errors.set(mapKey, [errorMessage]);
errors.set(mapKey, new Set([errorMessage]));
}
}

Expand All @@ -148,7 +146,7 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
// Generate the `error.message` property.
const message = generateArgumentErrorMessage(errors);

throw new ArgumentError(message, main, stack, errors);
throw new ArgumentError(message, main, errors);
}
}

Expand Down
4 changes: 2 additions & 2 deletions source/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ Validate the value against the provided predicate.
@param label - Label which should be used in error messages.
@param predicate - Predicate to test to value against.
*/
export default function test<T>(value: T, label: string | Function, predicate: BasePredicate<T>, stack: string) {
predicate[testSymbol](value, test, label, stack);
export default function test<T>(value: T, label: string | Function, predicate: BasePredicate<T>) {
predicate[testSymbol](value, test, label);
}
19 changes: 10 additions & 9 deletions source/utils/generate-argument-error-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,20 @@ Generates a complete message from all errors generated by predicates.
@param isAny - If this function is called from the any argument.
@hidden
*/
export const generateArgumentErrorMessage = (errors: Map<string, string[]>, isAny = false) => {
export const generateArgumentErrorMessage = (errors: Map<string, Set<string>>, isAny = false) => {
const message = [];

const errorArray = [...errors.values()];
const errorArray = [...errors.entries()];

const anyErrorWithoutOneItemOnly = errorArray.some(array => array.length !== 1);
const anyErrorWithoutOneItemOnly = errorArray.some(([, array]) => array.size !== 1);

// If only one error "key" is present, enumerate all of those errors only.
if (errors.size === 1) {
const returnedErrors = errorArray[0]!;
if (errorArray.length === 1) {
const [, returnedErrors] = errorArray[0]!;

if (!isAny && returnedErrors.length === 1) {
return returnedErrors[0]!;
if (!isAny && returnedErrors.size === 1) {
const [errorMessage] = returnedErrors;
return errorMessage!;
}

for (const entry of returnedErrors) {
Expand All @@ -29,11 +30,11 @@ export const generateArgumentErrorMessage = (errors: Map<string, string[]>, isAn

// If every predicate returns just one error, enumerate them as is.
if (!anyErrorWithoutOneItemOnly) {
return errorArray.map(([item]) => ` - ${item}`).join('\n');
return errorArray.map(([, [item]]) => ` - ${item}`).join('\n');
}

// Else, iterate through all the errors and enumerate them.
for (const [key, value] of errors) {
for (const [key, value] of errorArray) {
message.push(`Errors from the "${key}" predicate:`);
for (const entry of value) {
message.push(` - ${entry}`);
Expand Down
7 changes: 2 additions & 5 deletions source/utils/match-shape.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import is from '@sindresorhus/is';
import test from '../test';
import {isPredicate} from '../predicates/base-predicate';
import {BasePredicate} from '..';
import {generateStackTrace} from './generate-stack';

// eslint-disable-next-line @typescript-eslint/consistent-indexed-object-style
export interface Shape {
Expand Down Expand Up @@ -45,13 +44,12 @@ Test if the `object` matches the `shape` partially.
@param parent - Name of the parent property.
*/
export function partial(object: Record<string, any>, shape: Shape, parent?: string): boolean | string {
const stack = generateStackTrace();
try {
for (const key of Object.keys(shape)) {
const label = parent ? `${parent}.${key}` : key;

if (isPredicate(shape[key])) {
test(object[key], label, shape[key] as BasePredicate, stack);
test(object[key], label, shape[key] as BasePredicate);
} else if (is.plainObject(shape[key])) {
const result = partial(object[key], shape[key] as Shape, label);

Expand All @@ -77,7 +75,6 @@ Test if the `object` matches the `shape` exactly.
@param parent - Name of the parent property.
*/
export function exact(object: Record<string, any>, shape: Shape, parent?: string, isArray?: boolean): boolean | string {
const stack = generateStackTrace();
try {
const objectKeys = new Set<string>(Object.keys(object));

Expand All @@ -87,7 +84,7 @@ export function exact(object: Record<string, any>, shape: Shape, parent?: string
const label = parent ? `${parent}.${key}` : key;

if (isPredicate(shape[key])) {
test(object[key], label, shape[key] as BasePredicate, stack);
test(object[key], label, shape[key] as BasePredicate);
} else if (is.plainObject(shape[key])) {
if (!Object.prototype.hasOwnProperty.call(object, key)) {
return `Expected \`${label}\` to exist`;
Expand Down
38 changes: 19 additions & 19 deletions test/any-multiple-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ test('any predicate', t => {

const reportedError_1_1 = error_1.validationErrors.get('string')!;

t.is(reportedError_1_1.length, 1, 'There should be only one element');
t.deepEqual(reportedError_1_1, [
t.is(reportedError_1_1.size, 1, 'There should be only one element');
t.deepEqual(reportedError_1_1, new Set([
'Expected argument to be of type `string` but received type `number`'
]);
]));

// #endregion

Expand All @@ -43,18 +43,18 @@ test('any predicate', t => {
const reportedError_2_1 = error_2.validationErrors.get('string')!;
const reportedError_2_2 = error_2.validationErrors.get('number')!;

t.is(reportedError_2_1.length, 3, 'There should be three errors reported for the string predicate');
t.is(reportedError_2_2.length, 1, 'There should be one error reported for the number predicate');
t.is(reportedError_2_1.size, 3, 'There should be three errors reported for the string predicate');
t.is(reportedError_2_2.size, 1, 'There should be one error reported for the number predicate');

t.deepEqual(reportedError_2_1, [
t.deepEqual(reportedError_2_1, new Set([
'Expected argument to be of type `string` but received type `number`',
'Expected string to be a URL, got `21`',
'Expected string to have a minimum length of `24`, got `21`'
]);
]));

t.deepEqual(reportedError_2_2, [
t.deepEqual(reportedError_2_2, new Set([
'Expected number to be greater than 42, got 21'
]);
]));

// #endregion

Expand All @@ -74,15 +74,15 @@ test('any predicate', t => {
const reportedError_3_1 = error_3.validationErrors.get('string')!;
const reportedError_3_2 = error_3.validationErrors.get('number')!;

t.is(reportedError_3_1.length, 1, 'There should be one error reported for the string predicate');
t.is(reportedError_3_2.length, 1, 'There should be one error reported for the number predicate');
t.is(reportedError_3_1.size, 1, 'There should be one error reported for the string predicate');
t.is(reportedError_3_2.size, 1, 'There should be one error reported for the number predicate');

t.deepEqual(reportedError_3_1, [
t.deepEqual(reportedError_3_1, new Set([
'Expected argument to be of type `string` but received type `null`'
]);
t.deepEqual(reportedError_3_2, [
]));
t.deepEqual(reportedError_3_2, new Set([
'Expected argument to be of type `number` but received type `null`'
]);
]));

const error_4 = t.throws<ArgumentError>(() => {
ow(21 as any, ow.any(
Expand All @@ -100,19 +100,19 @@ test('any predicate', t => {

const reportedError_4_1 = error_4.validationErrors.get('string')!;

t.is(reportedError_4_1.length, 4, 'There should be four errors reported for the string predicate');
t.is(reportedError_4_1.size, 4, 'There should be four errors reported for the string predicate');

t.deepEqual(reportedError_4_1, [
t.deepEqual(reportedError_4_1, new Set([
'Expected argument to be of type `string` but received type `number`',
'Expected string to be a URL, got `21`',
'Expected string to have a minimum length of `21`, got `21`',
'Expected string to have a minimum length of `42`, got `21`'
]);
]));
// #endregion

// #region Tests line 47,65
class CustomPredicate implements BasePredicate<string> {
[testSymbol](_value: string, _main: Main, _label: string | Function, _stack: string): void {
[testSymbol](_value: string, _main: Main, _label: string | Function): void {
throw new Error('Custom error.');
}
}
Expand Down
6 changes: 3 additions & 3 deletions test/custom-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ test('custom validate message', t => {

const result1_ = error.validationErrors.get('string')!;

t.is(result1_.length, 2, 'There are two reported errors for this input');
t.deepEqual(result1_, [
t.is(result1_.size, 2, 'There are two reported errors for this input');
t.deepEqual(result1_, new Set([
'Expected string, to be have a minimum length of 5, got `1234`',
'This is no url'
], 'There is an error for the string length, and one for invalid URL');
]), 'There is an error for the string length, and one for invalid URL');

t.throws(() => {
ow('12345', ow.string.minLength(5).message((value, label) => `Expected ${label}, to be have a minimum length of 5, got \`${value}\``).url.message('This is no url'));
Expand Down
Loading

0 comments on commit 4e3340a

Please sign in to comment.